pikvm / ustreamer

µStreamer - Lightweight and fast MJPEG-HTTP streamer
https://pikvm.org
GNU General Public License v3.0
1.74k stars 240 forks source link

Double json_decref call on the same pointer in Janus plugin? #295

Closed RSATom closed 5 days ago

RSATom commented 6 days ago

According to docs json_object_set_new steals reference: https://github.com/pikvm/ustreamer/blob/740e09c70d55c389aa947977bdb6a95b9cd6890c/janus/src/plugin.c#L450-L452 and there m_result will be responsible for freeing x_payload.

But here json_decref will try to unref the same one more time: https://github.com/pikvm/ustreamer/blob/740e09c70d55c389aa947977bdb6a95b9cd6890c/janus/src/plugin.c#L533-L535

Am I miss something?

mdevaev commented 6 days ago

Dunno. Same for https://github.com/pikvm/ustreamer/blob/740e09c70d55c389aa947977bdb6a95b9cd6890c/janus/src/plugin.c#L512

It seems you're right. But I'm not sure why I did that.

RSATom commented 5 days ago

Ok, I'll try it inside debugger to be sure I didn't miss something.

RSATom commented 5 days ago

Checked with address sanitizer:

Nov 27 08:06:32 pikvm kvmd-janus[19596]: kvmd.apps.janus.runner  INFO --- => =================================================================
Nov 27 08:06:32 pikvm kvmd-janus[19596]: kvmd.apps.janus.runner  INFO --- => ==19622==ERROR: AddressSanitizer: heap-use-after-free on address 0xb011c154 at pc 0xb2c62428 bp 0x9eee0454 sp 0x9eee0440
Nov 27 08:06:32 pikvm kvmd-janus[19596]: kvmd.apps.janus.runner  INFO --- => READ of size 4 at 0xb011c154 thread T11
Nov 27 08:06:33 pikvm kvmd-janus[19596]: kvmd.apps.janus.runner  INFO --- => #0 0xb2c62424 in json_decref /usr/include/jansson.h:132
Nov 27 08:06:33 pikvm kvmd-janus[19596]: kvmd.apps.janus.runner  INFO --- => #1 0xb2c62424 in _plugin_handle_message src/plugin.c:535
Nov 27 08:06:33 pikvm kvmd-janus[19596]: kvmd.apps.janus.runner  INFO --- => #2 0x463380 in janus_process_incoming_request (/usr/bin/janus+0x63380) (BuildId: fa0047217e95d5c0cc263c8149c5589cc34cdd58)
Nov 27 08:06:33 pikvm kvmd-janus[19596]: kvmd.apps.janus.runner  INFO --- => #3 0x46bd7c in janus_transport_task (/usr/bin/janus+0x6bd7c) (BuildId: fa0047217e95d5c0cc263c8149c5589cc34cdd58)
Nov 27 08:06:33 pikvm kvmd-janus[19596]: kvmd.apps.janus.runner  INFO --- => #4 0xb68727a8  (/usr/lib/libglib-2.0.so.0+0x947a8) (BuildId: 3eeb3ecebce7edbf08fae4fe910a0f08566fb22f)
Nov 27 08:06:33 pikvm kvmd-janus[19596]: kvmd.apps.janus.runner  INFO --- => #5 0xb6872060  (/usr/lib/libglib-2.0.so.0+0x94060) (BuildId: 3eeb3ecebce7edbf08fae4fe910a0f08566fb22f)
Nov 27 08:06:33 pikvm kvmd-janus[19596]: kvmd.apps.janus.runner  INFO --- => #6 0xb69cd660 in asan_thread_start /build/gcc/src/gcc/libsanitizer/asan/asan_interceptors.cpp:234
Nov 27 08:06:33 pikvm kvmd-janus[19596]: kvmd.apps.janus.runner  INFO --- => #7 0xb5f9f920  (/usr/lib/libc.so.6+0x82920) (BuildId: af72d22a5784a90316910b69091b2106622ef478)
Nov 27 08:06:33 pikvm kvmd-janus[19596]: kvmd.apps.janus.runner  INFO --- => 0xb011c154 is located 4 bytes inside of 36-byte region [0xb011c150,0xb011c174)
Nov 27 08:06:33 pikvm kvmd-janus[19596]: kvmd.apps.janus.runner  INFO --- => freed by thread T11 here:
Nov 27 08:06:33 pikvm kvmd-janus[19596]: kvmd.apps.janus.runner  INFO --- => #0 0xb6a70588 in free /build/gcc/src/gcc/libsanitizer/asan/asan_malloc_linux.cpp:52
Nov 27 08:06:33 pikvm kvmd-janus[19596]: kvmd.apps.janus.runner  INFO --- => #1 0xb6593368  (/usr/lib/libjansson.so.4+0x3368) (BuildId: bb1adf084dfd063d3de16a6dc7f37994f27d85c6)
Nov 27 08:06:33 pikvm kvmd-janus[19596]: kvmd.apps.janus.runner  INFO --- => #2 0xb6593410  (/usr/lib/libjansson.so.4+0x3410) (BuildId: bb1adf084dfd063d3de16a6dc7f37994f27d85c6)
Nov 27 08:06:33 pikvm kvmd-janus[19596]: kvmd.apps.janus.runner  INFO --- => #3 0xb6599404 in json_delete (/usr/lib/libjansson.so.4+0x9404) (BuildId: bb1adf084dfd063d3de16a6dc7f37994f27d85c6)
Nov 27 08:06:33 pikvm kvmd-janus[19596]: kvmd.apps.janus.runner  INFO --- => #4 0xb6593368  (/usr/lib/libjansson.so.4+0x3368) (BuildId: bb1adf084dfd063d3de16a6dc7f37994f27d85c6)
Nov 27 08:06:33 pikvm kvmd-janus[19596]: kvmd.apps.janus.runner  INFO --- => #5 0xb6593410  (/usr/lib/libjansson.so.4+0x3410) (BuildId: bb1adf084dfd063d3de16a6dc7f37994f27d85c6)
Nov 27 08:06:33 pikvm kvmd-janus[19596]: kvmd.apps.janus.runner  INFO --- => #6 0xb6599404 in json_delete (/usr/lib/libjansson.so.4+0x9404) (BuildId: bb1adf084dfd063d3de16a6dc7f37994f27d85c6)
Nov 27 08:06:33 pikvm kvmd-janus[19596]: kvmd.apps.janus.runner  INFO --- => #7 0xb2c62200 in json_decref /usr/include/jansson.h:133
Nov 27 08:06:33 pikvm kvmd-janus[19596]: kvmd.apps.janus.runner  INFO --- => #8 0xb2c62200 in json_decref /usr/include/jansson.h:131
Nov 27 08:06:33 pikvm kvmd-janus[19596]: kvmd.apps.janus.runner  INFO --- => #9 0xb2c62200 in _plugin_handle_message src/plugin.c:534
Nov 27 08:06:33 pikvm kvmd-janus[19596]: kvmd.apps.janus.runner  INFO --- => #10 0x463380 in janus_process_incoming_request (/usr/bin/janus+0x63380) (BuildId: fa0047217e95d5c0cc263c8149c5589cc34cdd58)
Nov 27 08:06:33 pikvm kvmd-janus[19596]: kvmd.apps.janus.runner  INFO --- => #11 0x46bd7c in janus_transport_task (/usr/bin/janus+0x6bd7c) (BuildId: fa0047217e95d5c0cc263c8149c5589cc34cdd58)
Nov 27 08:06:33 pikvm kvmd-janus[19596]: kvmd.apps.janus.runner  INFO --- => #12 0xb68727a8  (/usr/lib/libglib-2.0.so.0+0x947a8) (BuildId: 3eeb3ecebce7edbf08fae4fe910a0f08566fb22f)
Nov 27 08:06:33 pikvm kvmd-janus[19596]: kvmd.apps.janus.runner  INFO --- => #13 0xb6872060  (/usr/lib/libglib-2.0.so.0+0x94060) (BuildId: 3eeb3ecebce7edbf08fae4fe910a0f08566fb22f)
Nov 27 08:06:33 pikvm kvmd-janus[19596]: kvmd.apps.janus.runner  INFO --- => #14 0xb69cd660 in asan_thread_start /build/gcc/src/gcc/libsanitizer/asan/asan_interceptors.cpp:234
Nov 27 08:06:33 pikvm kvmd-janus[19596]: kvmd.apps.janus.runner  INFO --- => #15 0xb5f9f920  (/usr/lib/libc.so.6+0x82920) (BuildId: af72d22a5784a90316910b69091b2106622ef478)
Nov 27 08:06:33 pikvm kvmd-janus[19596]: kvmd.apps.janus.runner  INFO --- => previously allocated by thread T11 here:
Nov 27 08:06:33 pikvm kvmd-janus[19596]: kvmd.apps.janus.runner  INFO --- => #0 0xb6a71a34 in malloc /build/gcc/src/gcc/libsanitizer/asan/asan_malloc_linux.cpp:69
Nov 27 08:06:33 pikvm kvmd-janus[19596]: kvmd.apps.janus.runner  INFO --- => #1 0xb65987c8 in json_object (/usr/lib/libjansson.so.4+0x87c8) (BuildId: bb1adf084dfd063d3de16a6dc7f37994f27d85c6)
Nov 27 08:06:33 pikvm kvmd-janus[19596]: kvmd.apps.janus.runner  INFO --- => #2 0xb65974dc  (/usr/lib/libjansson.so.4+0x74dc) (BuildId: bb1adf084dfd063d3de16a6dc7f37994f27d85c6)
Nov 27 08:06:33 pikvm kvmd-janus[19596]: kvmd.apps.janus.runner  INFO --- => #3 0xb6597a7c in json_vpack_ex (/usr/lib/libjansson.so.4+0x7a7c) (BuildId: bb1adf084dfd063d3de16a6dc7f37994f27d85c6)
Nov 27 08:06:33 pikvm kvmd-janus[19596]: kvmd.apps.janus.runner  INFO --- => #4 0xb6597c64 in json_pack (/usr/lib/libjansson.so.4+0x7c64) (BuildId: bb1adf084dfd063d3de16a6dc7f37994f27d85c6)
Nov 27 08:06:33 pikvm kvmd-janus[19596]: kvmd.apps.janus.runner  INFO --- => #5 0xb2c61eac in _plugin_handle_message src/plugin.c:533
Nov 27 08:06:33 pikvm kvmd-janus[19596]: kvmd.apps.janus.runner  INFO --- => #6 0x463380 in janus_process_incoming_request (/usr/bin/janus+0x63380) (BuildId: fa0047217e95d5c0cc263c8149c5589cc34cdd58)
Nov 27 08:06:33 pikvm kvmd-janus[19596]: kvmd.apps.janus.runner  INFO --- => #7 0x46bd7c in janus_transport_task (/usr/bin/janus+0x6bd7c) (BuildId: fa0047217e95d5c0cc263c8149c5589cc34cdd58)
Nov 27 08:06:33 pikvm kvmd-janus[19596]: kvmd.apps.janus.runner  INFO --- => #8 0xb68727a8  (/usr/lib/libglib-2.0.so.0+0x947a8) (BuildId: 3eeb3ecebce7edbf08fae4fe910a0f08566fb22f)
Nov 27 08:06:33 pikvm kvmd-janus[19596]: kvmd.apps.janus.runner  INFO --- => #9 0xb6872060  (/usr/lib/libglib-2.0.so.0+0x94060) (BuildId: 3eeb3ecebce7edbf08fae4fe910a0f08566fb22f)
Nov 27 08:06:33 pikvm kvmd-janus[19596]: kvmd.apps.janus.runner  INFO --- => #10 0xb69cd660 in asan_thread_start /build/gcc/src/gcc/libsanitizer/asan/asan_interceptors.cpp:234
Nov 27 08:06:33 pikvm kvmd-janus[19596]: kvmd.apps.janus.runner  INFO --- => #11 0xb5f9f920  (/usr/lib/libc.so.6+0x82920) (BuildId: af72d22a5784a90316910b69091b2106622ef478)

where src/plugin.c:535 is https://github.com/pikvm/ustreamer/blob/740e09c70d55c389aa947977bdb6a95b9cd6890c/janus/src/plugin.c#L535

So yes, bug is present. But I still can't understand why it didn't crash...

mdevaev commented 5 days ago

Did you try to comment json_decrefs? Does this fix the problem and not add leaks?

RSATom commented 5 days ago

@mdevaev yes, address sanitizer doesn't stop Janus with json_decref at src/plugin.c:535 commented (and does with not commented). Also on app stop I don't see leaks for that line. But I see some leaks coming from libpython3. I don't know if it's real leaks or not (sometimes it can be false positive).