crashed Janus with error heap-use-after-free in videoroom plugin

[Sensemakers-Nick]

Yesterday one of our servers crashed with the error: heap-use-after-free using the videorooms plugin.
Couldn't find updates to this particular part of code in GitHub, nor issues so decided to open a ticket.

gdb pastebin: https://pastebin.com/6GZFShE0
libasan pastebin: https://pastebin.com/YRX2j2j8

The load of the server ramped up pretty badly before crashing.

Roughly 40 videorooms connected.

[atoppi]

Hi @Sensemakers-Nick, thanks for the detailed report.
It seems that a room has been destroyed while the plugin was handling a negotiation.

More specifically the videoroom->h264_profile string attribute is being read by g_ascii_strdown in janus_sdp_get_codec_pt_full in order to craft a SDP answer, but since the room has been freed that string is not valid anymore, hence the libasan trap.

Maybe we need and extra reference on the videoroom while handling the negotiation.

[Sensemakers-Nick]

Hi @atoppi,

Thanks for the reply, you're welcome.

[atoppi]

@Sensemakers-Nick I'm attaching a patch that tries to address the issue, could you please apply it to your sources and test again?

git apply fix-increase-vroom-ref.txt

then rebuild janus

[Sensemakers-Nick]

Thanks @atoppi
Will apply the patch next week during a maintenance window and keep you posted!

[tmatth]

@atoppi we haven't seen this crash but will test the patch and report any issues.

[atoppi]

Thank you @tmatth !

[Sensemakers-Nick]

We've succesfully applied the patch, waiting for results, will let you know @atoppi

[Sensemakers-Nick]

@atoppi The same error heap-use-after-free in videoroom plugin happened today, but this time during the alteration of allowed participants in a destroyed room. Our API tried to edit the list of allowed participants in the room, but during this time, the room got destroyed.

Should I open a new ticket?

Pastebin: https://pastebin.com/yQDtQVFm

[atoppi]

Yes please open a new issue and try to add some more info.

1. enable verbose logging (5)
2. the freed/allocated call stacks in libasan are too short, check your CFLAGS (I suggest -Og -g3 -ggdb3 -fno-omit-frame-pointer -fsanitize=address)
3. try to capture a refcount debug dump (uncomment #define REFCOUNT_DEBUG in refcount.h and recompile)

[Sensemakers-Nick]

@atoppi will do, was compiled with

CFLAGS="-O1 -g3 -ggdb3 -fno-omit-frame-pointer -fsanitize=address -fno-sanitize-recover=all -fsanitize-address-use-after-scope" LDFLAGS="-fsanitize=address"

• Changing O1 to Og
• Enabling define REFCOUNT_DEBUG in refcount.h

[Sensemakers-Nick]

Unfortunately there is no crash log available at this moment, hopefully next time I can also provide you a backtrace.

[atoppi]

@Sensemakers-Nick do you think the original issue has been solved with the patch ?

[Sensemakers-Nick]

u think the original issue has been solved with th

I cannot say for sure, will need to wait it out a bit, if all goes well the next two weeks it has likely worked.
Am getting a lot more feedback now from Janus.

Information like this:
Jan 20 14:31:28 senprtc01.senp.local janus[29922]: [plugins/janus_videoroom.c:janus_videoroom_destroy_session:2649:increase] 0x607000743ba0 (2) Jan 20 14:31:28 senprtc01.senp.local janus[29922]: [plugins/janus_videoroom.c:janus_videoroom_session_destroy:1732:decrease] 0x607000743ba0 (1)

[Sensemakers-Nick]

Hi @atoppi,

We've trapped the same error yesterday, this time with the log information you've requested:
https://pastebin.com/55RYJPM8

[atoppi]

Same error, different issue.
This time a participant session has been freed before iterating over room participants list to notify them of a room destruction.
We need the whole log that includes the refcount lines to understand how that session has been freed.
I suspect the massive usage you're doing of the admin api to forward requests to the plugin is triggering many race conditions we were unaware of.

[Sensemakers-Nick]

I'll drop the entire log, it's going to be a big one ;-)

[teveelnicks]

@atoppi couldn't paste it in the pastebin, so I've shared them here.

portion of the log
entire log

Hope this helps

[atoppi]

thanks it helps, this is the freed reference of the last crash (to match lines one need to apply the patch that I attached in this issue).

Jan 21 19:04:48 senprtc01 janus[29922]: [plugins/janus_videoroom.c:janus_videoroom_create_session:2521:init] 0x607000b12810 (1)
Jan 21 19:05:00 senprtc01 janus[29922]: [plugins/janus_videoroom.c:janus_videoroom_handle_message:4676:increase] 0x607000b12810 (2)
Jan 21 19:05:00 senprtc01 janus[29922]: [plugins/janus_videoroom.c:janus_videoroom_hangup_media:5448:increase] 0x607000b12810 (3)
Jan 21 19:05:00 senprtc01 janus[29922]: [plugins/janus_videoroom.c:janus_videoroom_hangup_media:5451:decrease] 0x607000b12810 (2)
Jan 21 19:05:00 senprtc01 janus[29922]: [plugins/janus_videoroom.c:janus_videoroom_destroy_session:2649:increase] 0x607000b12810 (3)
Jan 21 19:05:00 senprtc01 janus[29922]: [plugins/janus_videoroom.c:janus_videoroom_session_destroy:1732:decrease] 0x607000b12810 (2)
Jan 21 19:05:00 senprtc01 janus[29922]: [plugins/janus_videoroom.c:janus_videoroom_destroy_session:2677:decrease] 0x607000b12810 (1)
Jan 21 19:05:00 senprtc01 janus[29922]: [plugins/janus_videoroom.c:janus_videoroom_message_free:1775:decrease] 0x607000b12810 (0)

[atoppi]

This seems a race condition happening when a user session gets destroyed while the plugin is still handling the setup.
The last unref that makes the count go to 0 is due to janus_videoroom_message_free , called when the plugin has handled the message.

[teveelnicks]

You're welcome!
I did apply the patch tough.

https://pastebin.com/hFSuk329

[atoppi]

@teveelnicks I have updated the patch with another fix for the last crash. Please apply on master and test again.
fixes_vroom.diff.txt

We'll wait for more info to debug the allowed list crash.

[Sensemakers-Nick]

Thanks @atoppi!
Will apply the patch soon.

[Sensemakers-Nick]

We haven't seen this issue since the last patch.
Thank you @atoppi !