[1.x] Segmentation fault in AudioBridge

[woutd]

Today, Janus crashed with a segmentation fault.
After checking logs and core file, I noticed it was a call to opus_encoder_ctl by the AudioBridge plugin which caused it.

In src/plugins/janus_audiobridge.c there are 44 calls to that function, but I am focusing on the ones which use participant->encoder.

The following one in janus_audiobridge_handler seems suspicious to me:

if(exploss) {
        participant->expected_loss = expected_loss;
        opus_encoder_ctl(participant->encoder, OPUS_SET_PACKET_LOSS_PERC(participant->expected_loss));
}

There is no check on participant->encoder while on the lines just above there is:

if(participant->encoder)
        opus_encoder_ctl(participant->encoder, OPUS_SET_BITRATE(participant->opus_bitrate ? participant->opus_bitrate : OPUS_AUTO));
if(quality) {
        participant->opus_complexity = complexity;
        if(participant->encoder)
                opus_encoder_ctl(participant->encoder, OPUS_SET_COMPLEXITY(participant->opus_complexity));
}

What version of Janus is this happening on?
1.3.2

Have you tested a more recent version of Janus too?
No, 1.3.2 is latest

Was this working before?
Unknown

Is there a gdb or libasan trace of the issue?
No debug symbols, but there is a core file with following backtrace:

#0  0x00007f436068b4a6 in opus_encoder_ctl () at /usr/lib/x86_64-linux-gnu/libopus.so.0
#1  0x00007f43606ec830 in janus_audiobridge_handler () at /data/global/lib/janus-1.3.2_bullseye_amd64/lib/janus/plugins/libjanus_audiobridge.so
#2  0x00007f43655a905d in  () at /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0
#3  0x00007f4364fd6ea7 in start_thread (arg=<optimized out>) at pthread_create.c:477
#4  0x00007f4364ef6adf in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:95

Additional context
Same second as crash the following log entries:

[Tue Oct  7 09:10:03 2025] [1008219414513122] There's a message for JANUS AudioBridge plugin
[Tue Oct  7 09:10:03 2025] [1008219414513122] Remote SDP:
...
[Tue Oct  7 09:10:03 2025] Detaching handle from JANUS AudioBridge plugin; 0x7f434c026000 0x7f434c02b380 0x7f434c026000 0x7f434c03b750
[Tue Oct  7 09:10:03 2025] [1008219414513122] Handle detached, scheduling destruction
[Tue Oct  7 09:10:03 2025] [7875649576810478] Hanging up PeerConnection because of a Detach
[Tue Oct  7 09:10:03 2025] Detaching handle from JANUS Streaming plugin; 0x7f434c02e450 0x7f434c05c900 0x7f434c02e450 0x7f434c05e2f0
[Tue Oct  7 09:10:03 2025] Preparing JSON event as a reply
[Tue Oct  7 09:10:03 2025] [7875649576810478] Handle detached, scheduling destruction

[lminiero]

Unfortunately there's not much we can do with the info you shared. We'll need line numbers (to know exactly where it crashed) and ideally a libasan trace, which will tell us if it's an attempt to access something that was previously freed. You can find more info in the documentation.

[woutd]

Unfortunately I am unable to reproduce this with debugging symbols. Janus was running fine for months until today.

But as mentioned before, the following code is quite suspicious because there is no NULL check on participant->encoder inside the if(exploss) { and it is a feature I am using:

janus-gateway/src/plugins/janus_audiobridge.c

Lines 7905 to 7915 in a44ebb6

	if(participant->encoder)
	opus_encoder_ctl(participant->encoder, OPUS_SET_BITRATE(participant->opus_bitrate ? participant->opus_bitrate : OPUS_AUTO));
	if(quality) {
	participant->opus_complexity = complexity;
	if(participant->encoder)
	opus_encoder_ctl(participant->encoder, OPUS_SET_COMPLEXITY(participant->opus_complexity));
	}
	if(exploss) {
	participant->expected_loss = expected_loss;
	opus_encoder_ctl(participant->encoder, OPUS_SET_PACKET_LOSS_PERC(participant->expected_loss));
	}

[lminiero]

You can add it for caution, but that would still indicate a problem: if at the end of a changeroom you don't have an encoder for that participant, that user won't be able to talk in the room. It may be a race condition where we access the encoder while it's being closed (e.g., concurrent hangup and changeroom) but again that's something we'd need a libasan dump for.

[woutd]

I understand, and I am not 100% sure that the segmentation fault is caused by this line.

But in your own commit you first check if(participant->encoder) and only 7 lines below you use it without the check. That makes me think it is possible participant->encoder can be NULL at this point.

3a35eca

Or are you telling me the other checks are only there for caution? :)

[lminiero]

We always try to NULL check, so that one should be added for sure. What I meant is that, if that's where it crashed (which is debatable), it would likely misbehave for you anyway, because it would mean you'd have a participant with no encoder. As such, more debugging info as I suggested would be what I'd need to evaluate the actual cause.

[woutd]

I did some more debugging on the core file I have (without debug symbols). I think I can pinpoint the exact position of the crash. And you are right, it is not at the position I first thought.

I know it is at 0x00007f43606ec830:

Program terminated with signal SIGSEGV, Segmentation fault.
#0  0x00007f436068b4a6 in opus_encoder_ctl () from /usr/lib/x86_64-linux-gnu/libopus.so.0
[Current thread is 1 (Thread 0x7f434affd700 (LWP 3893))]
(gdb) bt
#0  0x00007f436068b4a6 in opus_encoder_ctl () at /usr/lib/x86_64-linux-gnu/libopus.so.0
#1  0x00007f43606ec830 in janus_audiobridge_handler () at /data/global/lib/janus-1.3.2_bullseye_amd64/lib/janus/plugins/libjanus_audiobridge.so
#2  0x00007f43655a905d in  () at /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0
#3  0x00007f4364fd6ea7 in start_thread (arg=<optimized out>) at pthread_create.c:477
#4  0x00007f4364ef6adf in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:95

So I checked the assembly code at this point:

disassemble 0x00007f43606ec830
Dump of assembler code for function janus_audiobridge_handler:
...
   0x00007f43606ec7f5 <+10933>:	jle    0x7f43606ec830 <janus_audiobridge_handler+10992>
   0x00007f43606ec7f7 <+10935>:	mov    0x30(%rsp),%rdi
   0x00007f43606ec7fc <+10940>:	lea    0x144fe(%rip),%rsi        # 0x7f4360700d01
   0x00007f43606ec803 <+10947>:	call   0x7f43606b0c20 <strstr@plt>
   0x00007f43606ec808 <+10952>:	test   %rax,%rax
   0x00007f43606ec80b <+10955>:	je     0x7f43606ec830 <janus_audiobridge_handler+10992>
   0x00007f43606ec80d <+10957>:	mov    0x1b0(%r15),%rdi
   0x00007f43606ec814 <+10964>:	mov    $0x1,%edx
   0x00007f43606ec819 <+10969>:	mov    $0xfac,%esi
   0x00007f43606ec81e <+10974>:	xor    %eax,%eax
   0x00007f43606ec820 <+10976>:	movl   $0x1,0x1c0(%r15)
   0x00007f43606ec82b <+10987>:	call   0x7f43606b0840 <opus_encoder_ctl@plt>
=> 0x00007f43606ec830 <+10992>:	mov    0x1d739(%rip),%rax        # 0x7f4360709f70
   0x00007f43606ec837 <+10999>:	cmpl   $0x4,(%rax)
   0x00007f43606ec83a <+11002>:	jle    0x7f43606ec938 <janus_audiobridge_handler+11256>
   0x00007f43606ec840 <+11008>:	mov    0x1d789(%rip),%rax        # 0x7f4360709fd0
   0x00007f43606ec847 <+11015>:	pxor   %xmm0,%xmm0
   0x00007f43606ec84b <+11019>:	lea    0x2d0(%rsp),%r15
   0x00007f43606ec853 <+11027>:	movups %xmm0,0x2d0(%rsp)
   0x00007f43606ec85b <+11035>:	mov    (%rax),%r9d
   0x00007f43606ec85e <+11038>:	movups %xmm0,0x2e0(%rsp)
   0x00007f43606ec866 <+11046>:	movups %xmm0,0x2f0(%rsp)
...

It crashes when calling opus_encoder_ctl just before a call to strstr. Which only occurs once in janus_audiobridge.c.
To double check, I also checked the content of 0x7f4360700d01:

(gdb) x/s 0x7f4360700d01
0x7f4360700d01:	"useinbandfec=1"

Which corresponds with the following code:

8161                         /* What is the Opus payload type? */
8162                         janus_audiobridge_participant *participant = (janus_audiobridge_participant *)session->participant;
8163                         if(sdp != NULL) {
8164                                 participant->opus_pt = janus_sdp_get_codec_pt(sdp, -1, "opus");
8165                                 if(participant->opus_pt > 0 && strstr(msg_sdp, "useinbandfec=1")){
8166                                         /* Opus codec, inband FEC (from Janus to user) set */
8167                                         participant->fec = TRUE;
8168                                         opus_encoder_ctl(participant->encoder, OPUS_SET_INBAND_FEC(participant->fec));
8169                                 }
8170                                 JANUS_LOG(LOG_VERB, "Opus payload type is %d, outgoing FEC %s\n", participant->opus_pt, participant->fec ? "enabled" : "disabled");
8171                         }

participant->encoder is NULL at this point:

(gdb) x $r15 + 0x1b0
0x7f4340042a30:	0x0000000000000000

Sorry I am unable to provide you with better debugging info. The crash was triggered by a random client after Janus was running fine for months. I can compile Janus with debugging symbols but I do not know how to reproduce the issue.

[lminiero]

Interesting analysis, thanks for sharing it! In theory, if we have an SDP (and apparently we do as strstr is processing it to find FEC) then we should have an encoder as well, unless something happened and that went away (maybe the PeerConnection being closed in the meanwhile?). If you can compile Janus with libasan support, the next time it crashes we'll have more info on that too.

[woutd]

I can reproduce the issue by sleeping in that part of the code and then disconnecting within 10 seconds when I see the "Preparing JSON event as a reply" log line.

                                if(participant->opus_pt > 0 && strstr(msg_sdp, "useinbandfec=1")){
                                        /* Opus codec, inband FEC (from Janus to user) set */
                                        participant->fec = TRUE;
+                                       g_usleep(10000000);
                                        opus_encoder_ctl(participant->encoder, OPUS_SET_INBAND_FEC(participant->fec));
                                }

Is it ok to make a libasan dump with this minor modification? It will be really hard to reproduce it without.

[lminiero]

Is it ok to make a libasan dump with this minor modification? It will be really hard to reproduce it without.

Apologies for the delay. I'm currently abroad at a conference, but please do feel free to provide a libasan dump with that change. I'll then check if that leads to a race condition that could happen without the wait too, when I have some time to debug.

[woutd]

I made a libasan dump with the sleep. It is short, hope it is ok I paste it here:

AddressSanitizer:DEADLYSIGNAL
=================================================================
==2692441==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x7fbac56e14a6 bp 0x7fbabe7c6b60 sp 0x7fbabe7b8e90 T14)
==2692441==The signal is caused by a READ memory access.
==2692441==Hint: address points to the zero page.
    #0 0x7fbac56e14a6 in opus_encoder_ctl (/lib/x86_64-linux-gnu/libopus.so.0+0x3c4a6)
    #1 0x7fbabf8ff2d3 in janus_audiobridge_handler plugins/janus_audiobridge.c:8172
    #2 0x7fbac9fe305c  (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x7b05c)
    #3 0x7fbac98ccea6 in start_thread nptl/pthread_create.c:477
    #4 0x7fbac97ecade in __clone (/lib/x86_64-linux-gnu/libc.so.6+0xfbade)

AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV (/lib/x86_64-linux-gnu/libopus.so.0+0x3c4a6) in opus_encoder_ctl
Thread T14 created by T0 here:
    #0 0x7fbaca58c2a2 in __interceptor_pthread_create ../../../../src/libsanitizer/asan/asan_interceptors.cpp:214
    #1 0x7fbaca00c060  (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0xa4060)

==2692441==ABORTING

I investigated the audiobridge code. Please correct me if I am wrong, but I think participant->encoder is used thread-unsafe in many places.

It is created during "join" in janus_audiobridge_handler thread inside sessions_mutex. And destroyed in janus_audiobridge_hangup_media_internal inside sessions_mutex, participant-qmutex, audiobridge->mutex and rooms_mutex.

On top of the mutex, it also uses g_atomic_int_compare_and_exchange on participant->encoding to make sure it is not in use during destroy.

But in most places, it is accessed with only a NULL check (on some places even without) but no mutex or a g_atomic_int_compare_and_exchange to check if it is in use (or destroyed).

Same goes for participant->decoder.

I created an experimental branch (https://github.com/woutd/janus-gateway/tree/audiobridge-multithread-fixes) in an attempt to fix all these thread-unsafe uses. I basically replaced the encoding and decoding gbooleans with mutexes to ensure every access is thread-safe. I am currently using it in a development environment.

What do you think of this approach?

[lminiero]

At the time we thought of using those atomic volatile ints to soft-lock encoders and decoders, as we feared a proper mutex might impact performance too much: besides, encoding and decoding themselves are confined to a single participant thread, and so have very little concurrent accesses. It might make sense to switch to mutex instead, though, especially considering that we use GMutex which defaults to a futex internally anyway, on most systems, but that will need to be carefully evaluated.

Feel free to submit your experimental branch as a PR (a draft PR, if you don't trust it enough at this stage) so that we can review it there and see if it can be improved, in case. I'm still abroad, though, so it will take me a few days before I can look at anything.

[woutd]

I understand this must be carefully reviewed for performance impact and possible deadlocks.
It is only active in development since today, I will keep you updated if I notice any issues.
I will make a draft PR early next week.

[spscream]

we are running this on our production last few days to check if it also fixes #3590, no issues with load or segfaults.