Add references to publisher's streams when dealing with forwarders

[atoppi]

We have been notified on the discourse group and on our customers' dedicated platform of a crash occurring in the videoroom plugin when using Janus v1.2.1. After a deep analysis we concluded that the problem might be due to the freeing of a specific mutex during a hangup request while it is being locked by another request.

Backtraces or logs of the crash might mention

g_mutex_clear() called on uninitialised or locked mutex

or

GLib-CRITICAL **: 15:24:46.902: g_hash_table_iter_next: assertion 'ri->version == ri->hash_table->version' failed
#0 0x7fb24b8a3898 in abort (/lib/x86_64-linux-gnu/libc.so.6+0x28898)
#1 0x7fb24c2d3f62 in g_mutex_clear (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0xa4f62)
#2 0x7fb240150bce in janus_videoroom_publisher_stream_free plugins/janus_videoroom.c:2363

This patch tries to address the discovered problem.

If you are affected by this issue, please test and report.

[RSATom]

@atoppi I've backported that patch to v1.2.1 and our team installed updated version on our test servers. Unfortunately we got another crash:

#0 0x7fd8e4267685 in __interceptor_pthread_create ../../../../src/libsanitizer/asan/asan_interceptors.cpp:216
#1 0x7fd8e3edc504  (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0xaa504)
SUMMARY: AddressSanitizer: heap-use-after-free plugins/janus_videoroom.c:8087 in janus_videoroom_incoming_rtp_internal
Shadow bytes around the buggy address:
0x0c2c7fffaa80: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
0x0c2c7fffaa90: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
0x0c2c7fffaaa0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
0x0c2c7fffaab0: fd fd fd fa fa fa fa fa fa fa fa fa fa fa fa fa
0x0c2c7fffaac0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
=>0x0c2c7fffaad0: fd fd fd fd[fd]fd fd fd fd fd fd fd fd fd fd fd
0x0c2c7fffaae0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
0x0c2c7fffaaf0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
0x0c2c7fffab00: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
0x0c2c7fffab10: fd fd fd fa fa fa fa fa fa fa fa fa fa fa fa fa
0x0c2c7fffab20: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
Shadow byte legend (one shadow byte represents 8 application bytes):
Addressable:           00
Partially addressable: 01 02 03 04 05 06 07
Heap left redzone:       fa
Freed heap region:       fd
Stack left redzone:      f1
Stack mid redzone:       f2
Stack right redzone:     f3
Stack after return:      f5
Stack use after scope:   f8
Global redzone:          f9
Global init order:       f6
Poisoned by user:        f7
Container overflow:      fc
Array cookie:            ac
Intra object redzone:    bb
ASan internal:           fe
Left alloca redzone:     ca
Right alloca redzone:    cb
Shadow gap:              cc
==1==ABORTING 

But I don't know if it's related or something different...

[atoppi]

What does line 8087 contain in your branch?

On Fri, Feb 16, 2024, 16:33 Sergey Radionov @.***> wrote:

@atoppi https://github.com/atoppi I've backported that patch to v1.2.1 and our team installed updated version on our test servers. Unfortunately we got another crash:

0 0x7fd8e4267685 in __interceptor_pthread_create ../../../../src/libsanitizer/asan/asan_interceptors.cpp:216

1 0x7fd8e3edc504 (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0xaa504)

SUMMARY: AddressSanitizer: heap-use-after-free plugins/janus_videoroom.c:8087 in janus_videoroom_incoming_rtp_internal Shadow bytes around the buggy address: 0x0c2c7fffaa80: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd 0x0c2c7fffaa90: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd 0x0c2c7fffaaa0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd 0x0c2c7fffaab0: fd fd fd fa fa fa fa fa fa fa fa fa fa fa fa fa 0x0c2c7fffaac0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa =>0x0c2c7fffaad0: fd fd fd fd[fd]fd fd fd fd fd fd fd fd fd fd fd 0x0c2c7fffaae0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd 0x0c2c7fffaaf0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd 0x0c2c7fffab00: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd 0x0c2c7fffab10: fd fd fd fa fa fa fa fa fa fa fa fa fa fa fa fa 0x0c2c7fffab20: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa Shadow byte legend (one shadow byte represents 8 application bytes): Addressable: 00 Partially addressable: 01 02 03 04 05 06 07 Heap left redzone: fa Freed heap region: fd Stack left redzone: f1 Stack mid redzone: f2 Stack right redzone: f3 Stack after return: f5 Stack use after scope: f8 Global redzone: f9 Global init order: f6 Poisoned by user: f7 Container overflow: fc Array cookie: ac Intra object redzone: bb ASan internal: fe Left alloca redzone: ca Right alloca redzone: cb Shadow gap: cc ==1==ABORTING

But I don't know if it's related or something different...

— Reply to this email directly, view it on GitHub https://github.com/meetecho/janus-gateway/pull/3331#issuecomment-1948632677, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAZ7KBKCRACUPNVBFSPRQ73YT533HAVCNFSM6AAAAABDL5JT42VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSNBYGYZTENRXG4 . You are receiving this because you were mentioned.Message ID: @.***>

[lminiero]

@RSATom please test the exact branch of this PR, at least with respect to the VideoRoom, don't apply your patches to whatever fork you have. We can't (and won't) debug modified code.

[RSATom]

@atoppi Sorry, forgot to provide reference to sources. it's here: RSATom/janus-gateway/src/plugins/janus_videoroom.c#L8087

[RSATom]

@lminiero I'm not sure if I can test exact branch since our app relies on other plugins and patches. But there is no changes from our side to Videoroom plugin atm. So, maybe, I can rebase all my patches over this PR and test it that way if it will be acceptable.

[lminiero]

@RSATom please test the new commit.

[RSATom]

Thanks! I will test on Monday

[RSATom]

FYI, we didn't get any crash yesterday, but we plan spend some more days on testing it.

[RSATom]

FYI, didn't get any crash last week...

[lminiero]

Thanks for the feedback, merging then!