[ 
https://issues.apache.org/jira/browse/DISPATCH-2055?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17334969#comment-17334969
 ] 

ASF GitHub Bot commented on DISPATCH-2055:
------------------------------------------

kgiusti commented on a change in pull request #1146:
URL: https://github.com/apache/qpid-dispatch/pull/1146#discussion_r622485407



##########
File path: src/log.c
##########
@@ -271,7 +280,7 @@ static const char* level_name(int level) {
 static const char *SEPARATORS=", ;:";
 
 /// Calculate the bit mask for a log enable string. Return -1 and set qd_error 
on error.
-static int enable_mask(const char *enable_) {
+static int enable_mask(const char *enable_, bool log_error) {

Review comment:
       This also appears to only be called with log_error=false, which makes 
the log_error parameter & logic not necessary.

##########
File path: src/log.c
##########
@@ -525,71 +538,143 @@ void qd_log_finalize(void) {
         log_sink_free_lh(DEQ_HEAD(sink_list));
 }
 
-qd_error_t qd_log_entity(qd_entity_t *entity) {
-
+qd_error_t qd_log_entity(qd_entity_t *entity)
+{
     qd_error_clear();
 
-    //Obtain the log_source_lock global lock
-    sys_mutex_lock(log_source_lock);
-
     char* module = 0;
-    char *output = 0;
+    char *outputFile = 0;
     char *enable = 0;
+    int include_timestamp = 0;
+    int include_source = 0;
+
+    bool has_enable = false;
+    bool has_output_file = false;
+    bool has_include_timestamp = false;
+    bool has_include_source = false;
+    bool is_sink_syslog = false;
     bool trace_enabled = false;
+    bool error_in_output = false;
+    bool error_in_enable = false;
 
     do {
-
+        //
+        // A module attribute MUST be specified for a log entity.
+        // Every other attribute is optional.
+        //
         module = qd_entity_get_string(entity, "module");
+
+        //
+        // If the module is not specified, there is nothing to do, just log an
+        // error and break out of this do loop.
+        //
         QD_ERROR_BREAK();

Review comment:
       IIRC these QD_ERROR_BREAK's force a jump to the end of the while (0) 
loop.
   At the end of the while (0) loop there is a mutex unlock call, which will be 
unlocking a mutex which is not locked.

##########
File path: src/log.c
##########
@@ -222,27 +222,36 @@ static level_t levels[] = {
     LEVEL("critical", QD_LOG_CRITICAL, LOG_CRIT)
 };
 
+static const level_t invalid_level = {"invalid", -2, -2, 0};
+
 static char level_names[TEXT_MAX] = {0}; /* Set up in qd_log_initialize */
 
 /// Return NULL and set qd_error if not a valid bit.
-static const level_t* level_for_bit(int bit) {
+static const level_t* level_for_bit(int bit, bool log_error) {

Review comment:
       IIUC this level_for_bit is only called by write_log(), which passes 
log_error=false.
   So why add the bool at all?  Simply remove the qd_error since it will never 
be called.

##########
File path: src/log.c
##########
@@ -222,27 +222,36 @@ static level_t levels[] = {
     LEVEL("critical", QD_LOG_CRITICAL, LOG_CRIT)
 };
 
+static const level_t invalid_level = {"invalid", -2, -2, 0};
+
 static char level_names[TEXT_MAX] = {0}; /* Set up in qd_log_initialize */
 
 /// Return NULL and set qd_error if not a valid bit.
-static const level_t* level_for_bit(int bit) {
+static const level_t* level_for_bit(int bit, bool log_error) {
     level_index_t i = 0;
     while (i < N_LEVELS && levels[i].bit != bit) ++i;
     if (i == N_LEVELS) {
-        qd_error(QD_ERROR_CONFIG, "'%d' is not a valid log level bit.", bit);
-        return NULL;
+        if (log_error)
+            qd_error(QD_ERROR_CONFIG, "'%d' is not a valid log level bit.", 
bit);
+        return &invalid_level;
     }
     return &levels[i];
 }
 
+static bool is_log_level_invalid(const level_t *level)
+{
+    return ! strcmp(level->name, "invalid");
+}
+
 /// Return NULL and set qd_error if not a valid level.
-static const level_t* level_for_name(const char *name, int len) {
+static const level_t* level_for_name(const char *name, int len, bool 
log_error) {

Review comment:
       same here, log_error is essentially unused.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[email protected]


> AddressSanitizer: heap-use-after-free in write_log during 
> system_tests_qdmanage
> -------------------------------------------------------------------------------
>
>                 Key: DISPATCH-2055
>                 URL: https://issues.apache.org/jira/browse/DISPATCH-2055
>             Project: Qpid Dispatch
>          Issue Type: Bug
>    Affects Versions: 1.16.0
>            Reporter: Jiri Daněk
>            Assignee: Ganesh Murthy
>            Priority: Major
>             Fix For: 1.17.0
>
>
> https://travis-ci.com/github/apache/qpid-dispatch/jobs/498853046#L5728
> {noformat}
> 29: Process 13857 error: exit code 1, expected -1
> 29: qdrouterd -c test_router_1.conf -I 
> /home/travis/build/apache/qpid-dispatch/python
> 29: 
> /home/travis/build/apache/qpid-dispatch/build/tests/system_test.dir/system_tests_qdmanage/QdmanageTest/setUpClass/test_router_1-2.cmd
> 29: >>>>
> 29: =================================================================
> 29: ==13857==ERROR: AddressSanitizer: heap-use-after-free on address 
> 0xffffaca04ba8 at pc 0xffffb1f23f34 bp 0xffffa5cf2770 sp 0xffffa5cf2768
> 29: READ of size 8 at 0xffffaca04ba8 thread T2
> 29:     #0 0xffffb1f23f30 in write_log 
> /home/travis/build/apache/qpid-dispatch/src/log.c:343:22
> 29:     #1 0xffffb1f23f30 in qd_vlog_impl 
> /home/travis/build/apache/qpid-dispatch/src/log.c:437:5
> 29:     #2 0xffffb1f24d44 in qd_log_impl 
> /home/travis/build/apache/qpid-dispatch/src/log.c:456:3
> 29:     #3 0xffffb1b482c8 in pni_tracer_to_log_sink 
> /home/travis/build/apache/qpid-dispatch/qpid-proton/c/src/core/transport.c:2874:3
> 29: 
> 29: 0xffffaca04ba8 is located 24 bytes inside of 48-byte region 
> [0xffffaca04b90,0xffffaca04bc0)
> 29: freed by thread T3 here:
> 29:     #0 0x48f30c in free 
> (/home/travis/build/apache/qpid-dispatch/build/router/qdrouterd+0x48f30c)
> 29:     #1 0xffffb1f27c64 in qd_log_entity 
> /home/travis/build/apache/qpid-dispatch/src/log.c:555:17
> 29:     #2 0xffffacf09ff4  (/lib/aarch64-linux-gnu/libffi.so.7+0x5ff4)
> 29:     #3 0xffffacf097c8  (/lib/aarch64-linux-gnu/libffi.so.7+0x57c8)
> 29:     #4 0xffffacddfd20 in _ctypes_callproc 
> (/usr/lib/python3.8/lib-dynload/_ctypes.cpython-38-aarch64-linux-gnu.so+0xfd20)
> 29:     #5 0xffffacde0334  
> (/usr/lib/python3.8/lib-dynload/_ctypes.cpython-38-aarch64-linux-gnu.so+0x10334)
> 29:     #6 0xffffb17dc604 in _PyObject_MakeTpCall 
> (/lib/aarch64-linux-gnu/libpython3.8.so.1.0+0x299604)
> 29:     #7 0xffffb15b6698  
> (/lib/aarch64-linux-gnu/libpython3.8.so.1.0+0x73698)
> 29:     #8 0xffffb15bd888 in _PyEval_EvalFrameDefault 
> (/lib/aarch64-linux-gnu/libpython3.8.so.1.0+0x7a888)
> 29:     #9 0xffffb15c1698  
> (/lib/aarch64-linux-gnu/libpython3.8.so.1.0+0x7e698)
> 29:     #10 0xffffb17dc8a0  
> (/lib/aarch64-linux-gnu/libpython3.8.so.1.0+0x2998a0)
> 29:     #11 0xffffb15b6624  
> (/lib/aarch64-linux-gnu/libpython3.8.so.1.0+0x73624)
> 29:     #12 0xffffb15bd888 in _PyEval_EvalFrameDefault 
> (/lib/aarch64-linux-gnu/libpython3.8.so.1.0+0x7a888)
> 29:     #13 0xffffb15c1698  
> (/lib/aarch64-linux-gnu/libpython3.8.so.1.0+0x7e698)
> 29:     #14 0xffffb17dc8a0  
> (/lib/aarch64-linux-gnu/libpython3.8.so.1.0+0x2998a0)
> 29:     #15 0xffffb15b6624  
> (/lib/aarch64-linux-gnu/libpython3.8.so.1.0+0x73624)
> 29:     #16 0xffffb15bb3bc in _PyEval_EvalFrameDefault 
> (/lib/aarch64-linux-gnu/libpython3.8.so.1.0+0x783bc)
> 29:     #17 0xffffb15c1698  
> (/lib/aarch64-linux-gnu/libpython3.8.so.1.0+0x7e698)
> 29:     #18 0xffffb15b6624  
> (/lib/aarch64-linux-gnu/libpython3.8.so.1.0+0x73624)
> 29:     #19 0xffffb15b75a0 in _PyEval_EvalFrameDefault 
> (/lib/aarch64-linux-gnu/libpython3.8.so.1.0+0x745a0)
> 29:     #20 0xffffb1702958 in _PyEval_EvalCodeWithName 
> (/lib/aarch64-linux-gnu/libpython3.8.so.1.0+0x1bf958)
> 29:     #21 0xffffb17dbbbc in _PyFunction_Vectorcall 
> (/lib/aarch64-linux-gnu/libpython3.8.so.1.0+0x298bbc)
> 29:     #22 0xffffb17dc808  
> (/lib/aarch64-linux-gnu/libpython3.8.so.1.0+0x299808)
> 29:     #23 0xffffb17dcf34  
> (/lib/aarch64-linux-gnu/libpython3.8.so.1.0+0x299f34)
> 29:     #24 0xffffb17dd8c0 in PyObject_CallFunction 
> (/lib/aarch64-linux-gnu/libpython3.8.so.1.0+0x29a8c0)
> 29:     #25 0xffffb1f7e4d0 in qd_io_rx_handler 
> /home/travis/build/apache/qpid-dispatch/src/python_embedded.c:662:23
> 29:     #26 0xffffb200eb54 in qdr_forward_on_message 
> /home/travis/build/apache/qpid-dispatch/src/router_core/forwarder.c:341:28
> 29:     #27 0xffffb2031bb4 in qdr_general_handler 
> /home/travis/build/apache/qpid-dispatch/src/router_core/router_core.c:903:9
> 29:     #28 0xffffb20ee754 in qd_timer_visit 
> /home/travis/build/apache/qpid-dispatch/src/timer.c:205:9
> 29:     #29 0xffffb20e1754 in handle 
> /home/travis/build/apache/qpid-dispatch/src/server.c:1008:9
> 29: 
> 29: previously allocated by thread T0 here:
> 29:     #0 0x48f578 in malloc 
> (/home/travis/build/apache/qpid-dispatch/build/router/qdrouterd+0x48f578)
> 29:     #1 0xffffb1f262ac in qd_malloc 
> /home/travis/build/apache/qpid-dispatch/include/qpid/dispatch/ctools.h:229:17
> 29:     #2 0xffffb1f262ac in log_sink_lh 
> /home/travis/build/apache/qpid-dispatch/src/log.c:165:16
> 29:     #3 0xffffb1f27a48 in qd_log_entity 
> /home/travis/build/apache/qpid-dispatch/src/log.c:550:32
> 29:     #4 0xffffacf09ff4  (/lib/aarch64-linux-gnu/libffi.so.7+0x5ff4)
> 29:     #5 0xffffacf097c8  (/lib/aarch64-linux-gnu/libffi.so.7+0x57c8)
> 29:     #6 0xffffacddfd20 in _ctypes_callproc 
> (/usr/lib/python3.8/lib-dynload/_ctypes.cpython-38-aarch64-linux-gnu.so+0xfd20)
> 29:     #7 0xffffacde0334  
> (/usr/lib/python3.8/lib-dynload/_ctypes.cpython-38-aarch64-linux-gnu.so+0x10334)
> 29:     #8 0xffffb17dc604 in _PyObject_MakeTpCall 
> (/lib/aarch64-linux-gnu/libpython3.8.so.1.0+0x299604)
> 29:     #9 0xffffb15b6698  
> (/lib/aarch64-linux-gnu/libpython3.8.so.1.0+0x73698)
> 29:     #10 0xffffb15bd888 in _PyEval_EvalFrameDefault 
> (/lib/aarch64-linux-gnu/libpython3.8.so.1.0+0x7a888)
> 29:     #11 0xffffb15c1698  
> (/lib/aarch64-linux-gnu/libpython3.8.so.1.0+0x7e698)
> 29:     #12 0xffffb17dc8a0  
> (/lib/aarch64-linux-gnu/libpython3.8.so.1.0+0x2998a0)
> 29:     #13 0xffffb15b6624  
> (/lib/aarch64-linux-gnu/libpython3.8.so.1.0+0x73624)
> 29:     #14 0xffffb15bd888 in _PyEval_EvalFrameDefault 
> (/lib/aarch64-linux-gnu/libpython3.8.so.1.0+0x7a888)
> 29:     #15 0xffffb15c1698  
> (/lib/aarch64-linux-gnu/libpython3.8.so.1.0+0x7e698)
> 29:     #16 0xffffb15b6624  
> (/lib/aarch64-linux-gnu/libpython3.8.so.1.0+0x73624)
> 29:     #17 0xffffb15b75a0 in _PyEval_EvalFrameDefault 
> (/lib/aarch64-linux-gnu/libpython3.8.so.1.0+0x745a0)
> 29:     #18 0xffffb15c1698  
> (/lib/aarch64-linux-gnu/libpython3.8.so.1.0+0x7e698)
> 29:     #19 0xffffb15b6624  
> (/lib/aarch64-linux-gnu/libpython3.8.so.1.0+0x73624)
> 29:     #20 0xffffb15b75a0 in _PyEval_EvalFrameDefault 
> (/lib/aarch64-linux-gnu/libpython3.8.so.1.0+0x745a0)
> 29:     #21 0xffffb1702958 in _PyEval_EvalCodeWithName 
> (/lib/aarch64-linux-gnu/libpython3.8.so.1.0+0x1bf958)
> 29:     #22 0xffffb17dbbbc in _PyFunction_Vectorcall 
> (/lib/aarch64-linux-gnu/libpython3.8.so.1.0+0x298bbc)
> 29:     #23 0xffffb15b6624  
> (/lib/aarch64-linux-gnu/libpython3.8.so.1.0+0x73624)
> 29:     #24 0xffffb15bb3bc in _PyEval_EvalFrameDefault 
> (/lib/aarch64-linux-gnu/libpython3.8.so.1.0+0x783bc)
> 29:     #25 0xffffb1702958 in _PyEval_EvalCodeWithName 
> (/lib/aarch64-linux-gnu/libpython3.8.so.1.0+0x1bf958)
> 29:     #26 0xffffb17dbbbc in _PyFunction_Vectorcall 
> (/lib/aarch64-linux-gnu/libpython3.8.so.1.0+0x298bbc)
> 29:     #27 0xffffb17dcf34  
> (/lib/aarch64-linux-gnu/libpython3.8.so.1.0+0x299f34)
> 29:     #28 0xffffb17dd8c0 in PyObject_CallFunction 
> (/lib/aarch64-linux-gnu/libpython3.8.so.1.0+0x29a8c0)
> 29:     #29 0xffffb1f057f8 in qd_dispatch_load_config 
> /home/travis/build/apache/qpid-dispatch/src/dispatch.c:133:45
> 29:     #30 0x4bd030 in main_process 
> /home/travis/build/apache/qpid-dispatch/router/src/main.c:97:5
> 29: 
> 29: Thread T2 created by T0 here:
> 29:     #0 0x47a794 in pthread_create 
> (/home/travis/build/apache/qpid-dispatch/build/router/qdrouterd+0x47a794)
> 29:     #1 0xffffb1f79084 in sys_thread 
> /home/travis/build/apache/qpid-dispatch/src/posix/threading.c:183:5
> 29:     #2 0xffffb20dac9c in qd_server_run 
> /home/travis/build/apache/qpid-dispatch/src/server.c:1485:22
> 29: 
> 29: Thread T3 created by T0 here:
> 29:     #0 0x47a794 in pthread_create 
> (/home/travis/build/apache/qpid-dispatch/build/router/qdrouterd+0x47a794)
> 29:     #1 0xffffb1f79084 in sys_thread 
> /home/travis/build/apache/qpid-dispatch/src/posix/threading.c:183:5
> 29:     #2 0xffffb20dac9c in qd_server_run 
> /home/travis/build/apache/qpid-dispatch/src/server.c:1485:22
> 29: 
> 29: SUMMARY: AddressSanitizer: heap-use-after-free 
> /home/travis/build/apache/qpid-dispatch/src/log.c:343:22 in write_log
> 29: Shadow bytes around the buggy address:
> 29:   0x200ff5940920: fa fa fd fd fd fd fd fa fa fa fd fd fd fd fd fa
> 29:   0x200ff5940930: fa fa fd fd fd fd fd fa fa fa fd fd fd fd fd fa
> 29:   0x200ff5940940: fa fa fd fd fd fd fd fa fa fa fd fd fd fd fd fa
> 29:   0x200ff5940950: fa fa fd fd fd fd fd fa fa fa fd fd fd fd fd fa
> 29:   0x200ff5940960: fa fa fd fd fd fd fd fa fa fa 00 00 00 00 00 fa
> 29: =>0x200ff5940970: fa fa fd fd fd[fd]fd fd fa fa fd fd fd fd fd fa
> 29:   0x200ff5940980: fa fa fd fd fd fd fd fa fa fa fd fd fd fd fd fa
> 29:   0x200ff5940990: fa fa fd fd fd fd fd fa fa fa 00 00 00 00 03 fa
> 29:   0x200ff59409a0: fa fa 00 00 00 00 03 fa fa fa fd fd fd fd fd fa
> 29:   0x200ff59409b0: fa fa fd fd fd fd fd fd fa fa 00 00 00 00 00 fa
> 29:   0x200ff59409c0: fa fa 00 00 00 00 00 fa fa fa 00 00 00 00 00 00
> 29: Shadow byte legend (one shadow byte represents 8 application bytes):
> 29:   Addressable:           00
> 29:   Partially addressable: 01 02 03 04 05 06 07 
> 29:   Heap left redzone:       fa
> 29:   Freed heap region:       fd
> 29:   Stack left redzone:      f1
> 29:   Stack mid redzone:       f2
> 29:   Stack right redzone:     f3
> 29:   Stack after return:      f5
> 29:   Stack use after scope:   f8
> 29:   Global redzone:          f9
> 29:   Global init order:       f6
> 29:   Poisoned by user:        f7
> 29:   Container overflow:      fc
> 29:   Array cookie:            ac
> 29:   Intra object redzone:    bb
> 29:   ASan internal:           fe
> 29:   Left alloca redzone:     ca
> 29:   Right alloca redzone:    cb
> 29:   Shadow gap:              cc
> 29: ==13857==ABORTING
> 29: <<<<
> 29: 
> 29: ----------------------------------------------------------------------
> 29: Ran 34 tests in 82.088s
> 29: 
> 29: FAILED (errors=6)
> 29/74 Test #29: system_tests_qdmanage .............................***Failed  
>  82.36 sec
> {noformat}



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to