Skip to content

Commit c5ecde0

Browse files
safesintesijktjkt
authored andcommittedJun 5, 2024
fix(DataNode): segfault when unlink unmanaged
When an Unmanaged DataNode is unlinked (e.g. was temporary linked to print a JSON string), the method dereferences a nullptr which is UB. Added tests in unsafe.cpp. Fixes: #21 Change-Id: Ia6a621e0b9d5faad8c64368b3a2e344e52288e87
1 parent d8bf2a8 commit c5ecde0

File tree

2 files changed

+34
-4
lines changed

2 files changed

+34
-4
lines changed
 

‎src/DataNode.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -641,7 +641,7 @@ void DataNode::unlink()
641641
{
642642
handleLyTreeOperation(this, [this] () {
643643
lyd_unlink_tree(m_node);
644-
}, OperationScope::JustThisNode, std::make_shared<internal_refcount>(m_refs->context));
644+
}, OperationScope::JustThisNode, std::make_shared<internal_refcount>(m_refs ? m_refs->context : nullptr));
645645
}
646646

647647
/**
@@ -676,7 +676,7 @@ void DataNode::unlinkWithSiblings()
676676
{
677677
handleLyTreeOperation(this, [this] {
678678
lyd_unlink_siblings(m_node);
679-
}, OperationScope::AffectsFollowingSiblings, std::make_shared<internal_refcount>(m_refs->context));
679+
}, OperationScope::AffectsFollowingSiblings, std::make_shared<internal_refcount>(m_refs ? m_refs->context : nullptr));
680680
}
681681

682682
/**

‎tests/unsafe.cpp

Lines changed: 32 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,13 @@
44
* Written by Václav Kubernát <kubernat@cesnet.cz>
55
*
66
* SPDX-License-Identifier: BSD-3-Clause
7-
*/
7+
*/
88

99
#include <doctest/doctest.h>
1010
#include <libyang-cpp/Context.hpp>
1111
#include <libyang-cpp/Utils.hpp>
1212
#include <libyang/libyang.h>
13+
#include <libyang/tree_data.h>
1314
#include "example_schema.hpp"
1415
#include "test_vars.hpp"
1516
#include "utils/filesystem_path.hpp"
@@ -40,7 +41,6 @@ TEST_CASE("Unsafe methods")
4041
ctx_deleter.release();
4142

4243
auto wrapped = libyang::createUnmanagedContext(ctx, ly_ctx_destroy);
43-
4444
}
4545

4646
DOCTEST_SUBCASE("No custom deleter")
@@ -111,6 +111,20 @@ TEST_CASE("Unsafe methods")
111111
// Both are still unmanaged, both are accessible.
112112
REQUIRE(wrapped.path() == "/example-schema:leafInt32");
113113
REQUIRE(anotherNodeWrapped.path() == "/example-schema:leafInt8");
114+
115+
DOCTEST_SUBCASE("no explicit unlink") { }
116+
117+
DOCTEST_SUBCASE("unlink an unmanaged node from an unmanaged node")
118+
{
119+
REQUIRE(wrapped.findPath("/example-schema:leafInt8"));
120+
REQUIRE(anotherNodeWrapped.findPath("/example-schema:leafInt32"));
121+
122+
// After unlink they are not reachable from each other
123+
anotherNodeWrapped.unlink();
124+
REQUIRE(!wrapped.findPath("/example-schema:leafInt8"));
125+
REQUIRE(!anotherNodeWrapped.findPath("/example-schema:leafInt32"));
126+
lyd_free_all(anotherNode);
127+
}
114128
}
115129

116130
// You have a C++ managed node and you want to insert that into an unmanaged node.
@@ -123,6 +137,22 @@ TEST_CASE("Unsafe methods")
123137
// BOTH are now unmanaged, both are accessible.
124138
REQUIRE(wrapped.path() == "/example-schema:leafInt32");
125139
REQUIRE(anotherNodeWrapped.path() == "/example-schema:leafInt8");
140+
141+
DOCTEST_SUBCASE("no explicit unlink") { }
142+
143+
DOCTEST_SUBCASE("unlink a managed node from an unmanaged node")
144+
{
145+
REQUIRE(wrapped.findPath("/example-schema:leafInt8"));
146+
REQUIRE(anotherNodeWrapped.findPath("/example-schema:leafInt32"));
147+
148+
// After unlink they are not reachable from each other
149+
anotherNodeWrapped.unlink();
150+
REQUIRE(!wrapped.findPath("/example-schema:leafInt8"));
151+
REQUIRE(!anotherNodeWrapped.findPath("/example-schema:leafInt32"));
152+
153+
// this is still unmanaged and we need to delete it
154+
lyd_free_all(anotherNode);
155+
}
126156
}
127157

128158
// You have a C++ managed node and you want to insert an unmanaged node into it.

0 commit comments

Comments
 (0)