You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@qpid.apache.org by "Jiri Daněk (Jira)" <ji...@apache.org> on 2021/11/02 16:08:00 UTC
[jira] [Commented] (DISPATCH-1821) Double-free in
qd_entity_configure_policy on error
[ https://issues.apache.org/jira/browse/DISPATCH-1821?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17437445#comment-17437445 ]
Jiri Daněk commented on DISPATCH-1821:
--------------------------------------
[~kgiusti] The code in question is quite weird and could be probably somewhat improved. (If I remember the previous thinking, the idea is that more correct handling there is to fail startup and don't try to recover.) Other than that, there is probably little to be done about this.
> Double-free in qd_entity_configure_policy on error
> --------------------------------------------------
>
> Key: DISPATCH-1821
> URL: https://issues.apache.org/jira/browse/DISPATCH-1821
> Project: Qpid Dispatch
> Issue Type: Bug
> Components: Policy Engine
> Affects Versions: 1.14.0
> Reporter: Jiri Daněk
> Assignee: Charles E. Rolke
> Priority: Trivial
> Labels: clang-analyzer
> Fix For: 1.18.0
>
>
> As far as I can tell, this double-free is near impossible in normal operation of the router. Below, it is reproduced using a test, which passes in a value for that cannot be converted to a Python bool. There is no way for a legitimate user to make this happen.
> Regarding a fix, I am not sure what the router should do when it detect invalid configuration. Freeing qd->policy is IMO not the right response, because that is normally freed in qd_dispatch_free. Printing an error and carying on (that now happens for {{maxConnections < 0}}) does not seem right to me either (but it's probably needed in case the config is changed through remote management interface),
> {noformat}
> 2020-11-02 09:48:51.887312 +0100 ROUTER_CORE (info) In-process subscription L/$management
> 2020-11-02 09:48:51.939210 +0100 ERROR (error) Python: NotImplementedError:
> 2020-11-02 09:48:51.941666 +0100 ERROR (error) Traceback (most recent call last):
> File "test_module", line 3, in __bool__
> NotImplementedError
> =================================================================
> ==27615==ERROR: AddressSanitizer: attempting double-free on 0x602000001810 in thread T0:
> #0 0x7f876d5191d7 in __interceptor_free (/nix/store/zhbhxp4jgalycwffildy2bwcgmrsmayk-gcc-9.2.0-lib/lib/libasan.so.5+0x10a1d7)
> #1 0x7f876cd33b95 in qd_policy_free ../src/policy.c:136
> #2 0x7f876cd347cf in qd_entity_configure_policy ../src/policy.c:178
> #3 0x4ef6a6 in _DOCTEST_ANON_FUNC_6 ../tests/c_unittests/test_policy.cpp:75
> #4 0x493edd in doctest::Context::run() ../tests/c_unittests/doctest.h:5849
> #5 0x4974e9 in main ../tests/c_unittests/doctest.h:5933
> #6 0x7f876b610d8a in __libc_start_main (/nix/store/xg6ilb9g9zhi2zg1dpi4zcp288rhnvns-glibc-2.30/lib/libc.so.6+0x23d8a)
> #7 0x43cd99 in _start (/home/jdanek/repos/qpid/qpid-dispatch/cmake-build-debug/tests/c_unittests/c_unittests+0x43cd99)
> 0x602000001810 is located 0 bytes inside of 5-byte region [0x602000001810,0x602000001815)
> freed by thread T0 here:
> #0 0x7f876d5191d7 in __interceptor_free (/nix/store/zhbhxp4jgalycwffildy2bwcgmrsmayk-gcc-9.2.0-lib/lib/libasan.so.5+0x10a1d7)
> #1 0x7f876cd347c3 in qd_entity_configure_policy ../src/policy.c:177
> #2 0x4ef6a6 in _DOCTEST_ANON_FUNC_6 ../tests/c_unittests/test_policy.cpp:75
> #3 0x493edd in doctest::Context::run() ../tests/c_unittests/doctest.h:5849
> #4 0x4974e9 in main ../tests/c_unittests/doctest.h:5933
> #5 0x7f876b610d8a in __libc_start_main (/nix/store/xg6ilb9g9zhi2zg1dpi4zcp288rhnvns-glibc-2.30/lib/libc.so.6+0x23d8a)
> previously allocated by thread T0 here:
> #0 0x7f876d4a40b5 in strdup (/nix/store/zhbhxp4jgalycwffildy2bwcgmrsmayk-gcc-9.2.0-lib/lib/libasan.so.5+0x950b5)
> #1 0x7f876cec8c99 in py_string_2_c ../src/python_utils.c:35
> #2 0x7f876cce21e2 in qd_entity_get_string ../src/entity.c:49
> #3 0x7f876cce2b5f in qd_entity_opt_string ../src/entity.c:84
> #4 0x7f876cd341b0 in qd_entity_configure_policy ../src/policy.c:161
> #5 0x4ef6a6 in _DOCTEST_ANON_FUNC_6 ../tests/c_unittests/test_policy.cpp:75
> #6 0x493edd in doctest::Context::run() ../tests/c_unittests/doctest.h:5849
> #7 0x4974e9 in main ../tests/c_unittests/doctest.h:5933
> #8 0x7f876b610d8a in __libc_start_main (/nix/store/xg6ilb9g9zhi2zg1dpi4zcp288rhnvns-glibc-2.30/lib/libc.so.6+0x23d8a)
> SUMMARY: AddressSanitizer: double-free (/nix/store/zhbhxp4jgalycwffildy2bwcgmrsmayk-gcc-9.2.0-lib/lib/libasan.so.5+0x10a1d7) in __interceptor_free
> ==27615==ABORTING
> Process finished with exit code 1
> {noformat}
> The test which follows is currently missing PY decrefs.
> {noformat}
> #include <Python.h>
> #include <thread>
> #include "helpers.hpp"
> #include "qdr_doctest.h"
> extern "C" {
> #include <qpid/dispatch/python_embedded.h>
> #include <qpid/dispatch/router_core.h>
> #include <router_core/router_core_private.h>
> #include <terminus_private.h>
> #include <inttypes.h>
> #include <stdint.h>
> #include <stdio.h>
> }
> TEST_CASE("policy") {
> std::thread([]() {
> WithNoMemoryLeaks leaks{};
> QDR qdr;
> qdr.start();
> qdr.wait();
> {
> auto qd = qdr.qd;
> PyObject *module = Py_CompileString(
> // language: Python
> R"EOT(
> class NotBool:
> def __bool__(self):
> raise NotImplementedError()
> fake_policy = {
> "maxConnections": 4,
> "policyDir": "/tmp",
> "enableVhostPolicy": 4,
> "enableVhostNamePatterns": NotBool(),
> })EOT",
> "test_module", Py_file_input);
> REQUIRE(module != nullptr);
> PyObject *pModuleObj = PyImport_ExecCodeModule("test_module", module);
> CHECK(pModuleObj != nullptr);
> PyErr_Print();
> PyObject *pAttrObj = PyObject_GetAttrString(pModuleObj, "fake_policy");
> REQUIRE(pAttrObj != nullptr);
> auto *entity = reinterpret_cast<qd_entity_t *>(pAttrObj);
> REQUIRE(qd_entity_has(entity, "enableVhostNamePatterns")); // sanity check for the test input
> // TODO Py Decrefs probably needed somewhere
> const qd_python_lock_state_t lock_state = qd_python_lock();
> qd_error_t err = qd_dispatch_configure_policy(qd, entity);
> CHECK(err == QD_ERROR_PYTHON);
> qd_python_unlock(lock_state);
> }
> qdr.stop();
> }).join();
> }
> {noformat}
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org