GNOME Bugzilla – Bug 137448
mpegaudioparse: crash (bus error)
Last modified: 2004-12-22 21:47:04 UTC
This is with gstreamer/gst-plugins 0.8.0.1: sceptic [tim] - ~ ---> gst-launch-0.8 filesrc location="foo.mp3" ! mp3parse ! fakesink RUNNING pipeline ... Bus error sceptic [tim] - ~ ---> valgrind --num-callers=10 gst-launch-0.8 filesrc location="foo.mp3" ! mp3parse ! fakesink ==23066== Using valgrind-2.1.1, a program supervision framework for x86-linux. (snip) ==23066== warning: Valgrind's pthread_getschedparam is incomplete ==23066== your program may misbehave as a result RUNNING pipeline ... ==23066== ==23066== Process terminating with default action of signal 7 (SIGBUS): dumping core ==23066== Non-existent physical address at address 0x3CE54000 ==23066== at 0x3C8D4685: gst_mp3parse_chain (gstmpegaudioparse.c:333) ==23066== by 0x3C8DD3DF: gst_opt_scheduler_chain_wrapper (gstoptimalscheduler.c:1325) ==23066== by 0x3C0728A7: gst_pad_push (gstpad.c:3034) ==23066== by 0x3C8DC9FA: get_group_schedule_function (gstoptimalscheduler.c:1138) ==23066== by 0x3C8DC4A1: schedule_group (gstoptimalscheduler.c:1007) ==23066== by 0x3C8DC565: gst_opt_scheduler_schedule_run_queue (gstoptimalscheduler.c:1050) ==23066== by 0x3C8DC7A9: schedule_chain (gstoptimalscheduler.c:1092) ==23066== by 0x3C8DFAB4: gst_opt_scheduler_iterate (gstoptimalscheduler.c:2279) ==23066== by 0x3C07D8E8: gst_scheduler_iterate (gstscheduler.c:712) ==23066== by 0x3C052C43: gst_bin_iterate_func (gstbin.c:1109) ==23066== ==23066== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 49 from 2) ==23066== malloc/free: in use at exit: 401744 bytes in 6566 blocks. ==23066== malloc/free: 108386 allocs, 101820 frees, 8176280 bytes allocated. ==23066== For a detailed leak analysis, rerun with: --leak-check=yes ==23066== For counts of detected errors, rerun with: -v Bus error Given the (re-)indenting going on currently, I'll paste the relevant lines from gst-plugins/gst/mpegaudioparse/gstmpegaudioparse.c as well (sorry for the flood): 326 /* while we still have bytes left -4 for the header */ 327 while (offset < size - 4) { 328 int skipped = 0; 329 330 GST_DEBUG ("mp3parse: offset %ld, size %ld ", offset, size); 331 332 /* search for a possible start byte */ 333 for (; ((data[offset] != 0xff) && (offset < size)); offset++) 334 skipped++; 335 if (skipped && !mp3parse->in_flush) { 336 GST_DEBUG ("mp3parse: **** now at %ld skipped %d bytes", offset, skipped); 337 } 338 /* construct the header word */ 339 header = GUINT32_FROM_BE (*((guint32 *) (data + offset))); 340 /* if it's a valid header, go ahead and send off the frame */ 341 if (head_check (header)) { 342 /* calculate the bpf of the frame */ 343 bpf = bpf_from_header (mp3parse, header); The offending mp3 file can be found here: http://www.zen18864.zen.co.uk/tmp/gst-mp3parse-bug-foo.bz2 I'll attach the gstreamer debug output. Cheers -Tim
Created attachment 25708 [details] gstreamer debug output
Line 333 is interesting... for (; ((data[offset] != 0xff) && (offset < size)); offset++) That should be: for (; ((offset < size) && (data[offset] != 0xff)); offset++) Can you please try out this patch? I'll test later tonight or later this week myself (currently debugging other bugs ;) ).
Yep, that's good - now it crashes somewhere else :) The attached patch seems to fix the problem. Cheers -Tim
Created attachment 25712 [details] [review] proposed patch
Doesn't the second part of the patch do the same as the first part?
Yes, it does the same thing. I put it in before I realised that the '-4' is needed in the for loop check as well, and then left it in for clarity :) Feel free to remove the second bit. Cheers -Tim
Created attachment 25817 [details] [review] new patch
Looks like this has been committed to CVS => closing. 2004-03-23 Ronald Bultje <rbultje@ronald.bitfreak.net> * gst/mpegaudioparse/gstmpegaudioparse.c: (gst_mp3parse_chain): Fix buffer overflow read error. Cheers -Tim
Yup. Just couldn't close this because bugzilla was down. Thanks for reporting and thanks for the fix!