rtpjitterbuffer: drop-messages-interval parameter size doesn't always match GstClockTime size
Back with another time64 issue (64-bit time_t
on 32-bit systems), this time in gst-plugins-good.
The test suite elements_rtpjitterbuffer
segfaults on test test_drop_messages_interval
on systems configured this way (easier to trigger on big endian systems but likely failing on all):
#0 __strchrnul (s=s@entry=0xa <error: Cannot access memory at address 0xa>, c=c@entry=58) at src/string/strchrnul.c:19
#1 0xf78b8bd4 in strchr (s=s@entry=0xa <error: Cannot access memory at address 0xa>, c=c@entry=58) at src/string/strchr.c:5
#2 0xf75ffcb8 in g_param_spec_pool_lookup (pool=0xf733e520,
param_name=param_name@entry=0xa <error: Cannot access memory at address 0xa>, owner_type=4145021488,
walk_ancestors=walk_ancestors@entry=1) at ../gobject/gparam.c:1104
#3 0xf75f8b10 in g_object_set_valist (object=object@entry=0xf71c62d8,
first_property_name=first_property_name@entry=0xa73268 "drop-messages-interval", var_args=var_args@entry=0xffed8a50)
at ../gobject/gobject.c:2531
#4 0xf75f9610 in g_object_set (_object=0xf71c62d8,
first_property_name=first_property_name@entry=0xa73268 "drop-messages-interval") at ../gobject/gobject.c:2717
#5 0x00a651f8 in test_drop_messages_interval (__i__=<optimized out>) at ../tests/check/elements/rtpjitterbuffer.c:3100
#6 0xf741dc4c in srunner_run_tagged () from /usr/lib/libgstcheck-1.0.so.0
#7 0xf741e13c in srunner_run () from /usr/lib/libgstcheck-1.0.so.0
#8 0xf741e190 in srunner_run_all () from /usr/lib/libgstcheck-1.0.so.0
#9 0xf740d608 in gst_check_run_suite () from /usr/lib/libgstcheck-1.0.so.0
#10 0x00a6317c in main (argc=<optimized out>, argv=<optimized out>) at ../tests/check/elements/rtpjitterbuffer.c:3450
The reason we are looking up a parameter with a name of *0xa is because of stack junk. And that's because it is interpreting this...
g_object_set (h->element, "drop-messages-interval", interval, NULL);
... as a varargs list, and since that property is defined at rtpmanager/gstrtpjitterbuffer.c:636 as ...
g_object_class_install_property (gobject_class, PROP_DROP_MESSAGES_INTERVAL,
g_param_spec_uint ("drop-messages-interval",
"Drop message interval",
"Minimal time between posting dropped packet messages", 0,
G_MAXUINT, DEFAULT_DROP_MESSAGES_INTERVAL_MS,
G_PARAM_READWRITE | G_PARAM_STATIC_STRINGS));
... a uint
, it's only reading 32 bits of the 64-bit interval
. Casting interval
to guint
does fix the issue:
Running suite(s): rtpjitterbuffer
100%: Checks: 66, Failures: 0, Errors: 0
Check suite rtpjitterbuffer ran in 17.266s (tests failed: 0)
... but I don't think that's right, because then all downstream consumers need to perform that cast to use GstClockTime
safely as a parameter. That said, I also don't know if there are any downstreams that use guint
s.
I'm not sure if the best way forward is:
- Define the property depending on the size of
GstClockTime
. That would mean it depends on the size oftime_t
. - Define the property unconditionally as a 64-bit value. Wastes space on 32-bit
time_t
systems, but should be safer. - Define the property unconditionally as a system word. This would mean the cast is necessary for at least the test suite, and also anything that uses
GstClockTime
instead ofguint
as the property's value type.
Perhaps the true issue is that interval
should have been defined as a guint
in the test instead of as a GstClockTime
. Not entirely sure.