Bug#778529: lame: fill_buffer_resample segmentation fault
Control: tags -1 + patch Am Mittwoch, den 18.02.2015, 10:53 +0100 schrieb Fabian Greffrath: But this is still not the cause of the crash, sigh! Patching the sample to report 1 channel, it still crashes at the same location. Phew, got it. This time, it was a simple logical error in the lame sources: The fake sample rate of the fuzzed input file is 1631 kHz which lame tries to sample down to 48 kHz in the process of encoding. The ratio between input and output samplerates is thus 1631/48000=339.79 which is very close, but only close, to the integer value 340. In libmp3lame/util.c:fill_buffer_resample(), lame checks if the ratio between input and output sample rate is an integer by the following calculation (l. 547): intratio = (fabs(resample_ratio - floor(.5 + resample_ratio)) .0001); Please note that the value of .0001, which the fabs() of the difference is compared against here, is a rather arbitrary value and is *not* sufficient to tell the difference between an integer and a floating point ratio in the case at hand (where it is actually about 0.2)! The value of intratio is added to another variable filter_l a few lines later, which in turn is used in the calculation of the value of the offset variable, which triggers the assertion (l. 594f): offset = (time0 - esv-itime[ch] - (j + .5 * (filter_l % 2))); assert(fabs(offset) = .501); In the case at hand, filter_l has got an even value by addition of intratio, which in turn was set to 1 in good faith that the sample rate ratio is integer, whereas in reality it is not. Thus, the latter part of the equation above is not substracted from the offset variable, so its value is higher than it should. In the following line, offset is used to calculate the value of joff, which is used to dereference esv-blackfilt, where it causes an overflow and finally a segfault (l. 608): xvalue += y * esv-blackfilt[joff][i]; The trivial fix for this would be to decrease the arbitrary value of .0001 by another factor 10 and compare against 0.1, but this would only suffice until the next fuzzed sample with an even higher sample rate is provided. I thus suggest to compare against the smallest number of type double (resample_ratio is of type double) that can still be distinguished from 0: DBL_EPSILON. The attached patch does exact that. Cheers, Fabian -- Dr.-Ing. Fabian Greffrath, Dipl.-Phys. RWTH Aachen University Institute of Mineral Engineering (GHI) Mauerstr. 5, D-52064 Aachen Phone: +49-241-8094979, Fax: +49-241-8092226 Subject: Fix decision if sample rate ratio is an integer value or not If the sample rate of the input file is sufficiently close to an integer multiple of the output sample rate, the value of the intratio variable is calculated incorrectly. This leads to further values being miscalculated up to the joff variable which is used as an index to dereference the esv-blackfilt array. This leads top an overflow and causes a segmentation fault. Author: Fabian Greffrath fabian+deb...@greffrath.com Bug-Debian: https://bugs.debian.org/778529 --- a/libmp3lame/util.c +++ b/libmp3lame/util.c @@ -26,6 +26,7 @@ # include config.h #endif +#include float.h #include lame.h #include machine.h #include encoder.h @@ -544,7 +545,7 @@ fill_buffer_resample(lame_internal_flags if (bpc BPC) bpc = BPC; -intratio = (fabs(resample_ratio - floor(.5 + resample_ratio)) .0001); +intratio = (fabs(resample_ratio - floor(.5 + resample_ratio)) = DBL_EPSILON); fcn = 1.00 / resample_ratio; if (fcn 1.00) fcn = 1.00;
Bug#778529: lame: fill_buffer_resample segmentation fault
Am Dienstag, den 17.02.2015, 11:19 +0100 schrieb Fabian Greffrath: But, the sample at hand reports -251 channels. Adding ... || gfp-num_channels 0) to Maks' patch actually fixes the crash. But this is still not the cause of the crash, sigh! Patching the sample to report 1 channel, it still crashes at the same location. - Fabian -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#778529: lame: fill_buffer_resample segmentation fault
On Wed, Feb 18, 2015 at 12:11:35PM +0100, Fabian Greffrath wrote: Phew, got it. Thank you for your comprehensive analysis. I have verified that the patch fixes this issue. Should I report this to upstream bug tracker or does package maintainer handle that? Bug tracker in sourceforge.net does not seem to be very active. -- Henri Salo -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#778529: lame: fill_buffer_resample segmentation fault
Am Mittwoch, den 18.02.2015, 12:11 +0100 schrieb Fabian Greffrath: This time, it was a simple logical error in the lame sources: The fake sample rate of the fuzzed input file is 1631 kHz which lame tries to sample down to 48 kHz in the process of encoding. The ratio between input and output samplerates is thus 1631/48000=339.79 which is very close, but only close, to the integer value 340. Actually, the bug is easy to reproduce even without a fuzzed sample. Take a random valid WAV file and convert it to another sample rate, e.g. sox /path/to/dummy.wav --rate 95999 crash.wav This time, I set the input sample frequency to 95999 Hz, which is 2*48kHz-1Hz. The intratio variable will be set to 1 again, although fabs(resample_ratio - floor(.5 + resample_ratio)) == 0.2 0.0001 != 0. The crash.wav sample will thus crash LAME. - Fabian -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#778529: lame: fill_buffer_resample segmentation fault
Am Dienstag, den 17.02.2015, 09:11 +0100 schrieb Fabian Greffrath: Does anyone know the highest reasonable input samplerate supported by lame? Na, this was a red herring. I just patched a (valid) WAV file to report a samplerate of some 10^6kHz and lame still successfully processed on it. But, the sample at hand reports -251 channels. Adding ... || gfp-num_channels 0) to Maks' patch actually fixes the crash. - Fabian -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#778529: lame: fill_buffer_resample segmentation fault
Hi Henri, thank you for submitting this bug report! Am Montag, den 16.02.2015, 12:45 +0200 schrieb Henri Salo: Error reading input file lame: util.c:595: fill_buffer_resample: Assertion `fabs(offset) = .501' failed. This seems to be the inverse problem this time: The samplerate in the fuzzed pseudo-WAV header is simply much too high. If I extend Maks' patch to also check for if (gfp-samplerate_in MAX_SAMP_FREQ) then it fails gracefully instead of crashing. However, MAX_SAMP_FREQ is currently defined to 48000 and I am not sure if this means maximum input or output samplerate supported by lame. Shouldn't lame be able to encode files with 96kHz samplerate into MP3 by reducing the samplerate to 48kHz in the process? Does anyone know the highest reasonable input samplerate supported by lame? Cheers, Fabian -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
