crash when generating event

[l00159860]

Hi,
We encountered a crash when using the event handler, and the following is part of the backtrace:
(gdb) bt #0 0x00007fc63871f641 in __strlen_sse2_pminub () at /lib64/libc.so.6 #1 0x00007fc63a0d65de in json_string () at /lib64/libjansson.so.4 #2 0x000000000042d18e in janus_events_notify_handlers (type=type@entry=2, subtype=subtype@entry=0, session_id=<optimized out>) at events.c:186 #3 0x00000000004473b6 in janus_ice_outgoing_traffic_handle (handle=0x7fc5e0004700, pkt=<optimized out>) at ice.c:4155

It looks like that the param handle->token is missed in ice.c:4155 when calling janus_events_notify_handlers()

[l00159860]

I am sorry but the crash is based on 0.10.3, and in master the corresponding line should be ice.c:4170

[lminiero]

I won't look at any report that has not been tested on master.

[ubonass]

we encountered a crash too

[yshuang666]

we encountered the same crash when we updated janus from 0.10.3 to 0.10.4, we installed the janus 0.10.4 to different folder, but forgot to modify the events_folder conf in janus.jcfg : events_folder = "/usr/local/janus/lib/janus/events", modify it, but still has problem, then we checked the code, because not pass token parameter for janus_events_notify_handlers in ice.c (line number:4170), we pass NULL for the token parameter, it is OK now.

[atoppi]

#2315 is a duplicate of this and contains some steps to reproduce and a stack trace from v0.10.3.

[david-goncalves]

Hi all,
The function janus_events_notify_handlers has different parameters based on type.
The type JANUS_EVENT_TYPE_HANDLE is used twice on ice.c. One for "attach" (line 1281) and other for "detach" (line 4170).

The function to attach (line 1281) contains eight parameters.
The function to detach (line 4170) contains seven.

Instead of NULL, what happens if you pass handle->token as the last parameter on line 4170?

[atoppi]

janus_events_notify_handlers is a variadic function.
This means that it will accept a variable number of arguments (like e.g. printf does).
The problem here is that we are not passing to the function any hint about the number of parameters.
Examples of how this can be done:

• first argument is the number of passed arguments
• use a special value to notify the end of arguments (e.g. NULL or /0)

So we are blindly issuing va_arg on a fixed number of arguments.
I suspect that calling va_args when no more arguments are present might lead to undefined behaviors (because the arguments are read from the stack). The effects are dependent on the environment (compiler, architecture, standard lib and so on).

Putting NULL as the last argument in this case is just a workaround, because we are filling all the possibile parameters (thus making useless the variadic function here).

Pinging @lminiero as this might involve a bigger review of how event handlers are used.

[lminiero]

Putting NULL as the last argument in this case is just a workaround, because we are filling all the possibile parameters (thus making useless the variadic function here).

Not a workaround, it's correct. Each event type has its own number of expected variable arguments, because that's what events.c expects. A few months ago in a80106a I added support for token to JANUS_EVENT_TYPE_HANDLE to allow event handlers to be notified about the token used to attach a handle, but I forgot to add it for the detach event, so adding NULL there (or better the token string, which is stored in handle->token) is the correct fix.