ref: c10c8191f21dd12c1bea2d9b9a68536f7cb0491c
parent: 8c325e35494ba9d2a5578d4a6702691884b85f0f
author: Erik de Castro Lopo <[email protected]>
date: Sat Aug 24 11:13:32 EDT 2019
src/src_sinc.c: Fix a buffer out-of-bounds read error The buffer out-of-bounds read that happens with the 'SRC_SINC_*' converters when the `src_ratio` is dynamically decreased while processing. This is a relatively naive fix for issue that seems to have an up to 3% performance degradation with respect to the unfixed version. It may be possible to come up with a better version of this fix that does not degrade performance. Closes: https://github.com/erikd/libsamplerate/issues/5
--- a/src/src_sinc.c
+++ b/src/src_sinc.c
@@ -295,12 +295,14 @@
left = 0.0 ;
do
- { fraction = fp_to_double (filter_index) ;- indx = fp_to_int (filter_index) ;
+ { if (data_index >= 0) /* Avoid underflow access to filter->buffer. */+ { fraction = fp_to_double (filter_index) ;+ indx = fp_to_int (filter_index) ;
- icoeff = filter->coeffs [indx] + fraction * (filter->coeffs [indx + 1] - filter->coeffs [indx]) ;
+ icoeff = filter->coeffs [indx] + fraction * (filter->coeffs [indx + 1] - filter->coeffs [indx]) ;
- left += icoeff * filter->buffer [data_index] ;
+ left += icoeff * filter->buffer [data_index] ;
+ } ;
filter_index -= increment ;
data_index = data_index + 1 ;
@@ -440,13 +442,15 @@
left [0] = left [1] = 0.0 ;
do
- { fraction = fp_to_double (filter_index) ;- indx = fp_to_int (filter_index) ;
+ { if (data_index >= 0) /* Avoid underflow access to filter->buffer. */+ { fraction = fp_to_double (filter_index) ;+ indx = fp_to_int (filter_index) ;
- icoeff = filter->coeffs [indx] + fraction * (filter->coeffs [indx + 1] - filter->coeffs [indx]) ;
+ icoeff = filter->coeffs [indx] + fraction * (filter->coeffs [indx + 1] - filter->coeffs [indx]) ;
- left [0] += icoeff * filter->buffer [data_index] ;
- left [1] += icoeff * filter->buffer [data_index + 1] ;
+ left [0] += icoeff * filter->buffer [data_index] ;
+ left [1] += icoeff * filter->buffer [data_index + 1] ;
+ } ;
filter_index -= increment ;
data_index = data_index + 2 ;
@@ -587,15 +591,17 @@
left [0] = left [1] = left [2] = left [3] = 0.0 ;
do
- { fraction = fp_to_double (filter_index) ;- indx = fp_to_int (filter_index) ;
+ { if (data_index >= 0) /* Avoid underflow access to filter->buffer. */+ { fraction = fp_to_double (filter_index) ;+ indx = fp_to_int (filter_index) ;
- icoeff = filter->coeffs [indx] + fraction * (filter->coeffs [indx + 1] - filter->coeffs [indx]) ;
+ icoeff = filter->coeffs [indx] + fraction * (filter->coeffs [indx + 1] - filter->coeffs [indx]) ;
- left [0] += icoeff * filter->buffer [data_index] ;
- left [1] += icoeff * filter->buffer [data_index + 1] ;
- left [2] += icoeff * filter->buffer [data_index + 2] ;
- left [3] += icoeff * filter->buffer [data_index + 3] ;
+ left [0] += icoeff * filter->buffer [data_index] ;
+ left [1] += icoeff * filter->buffer [data_index + 1] ;
+ left [2] += icoeff * filter->buffer [data_index + 2] ;
+ left [3] += icoeff * filter->buffer [data_index + 3] ;
+ } ;
filter_index -= increment ;
data_index = data_index + 4 ;
@@ -740,17 +746,19 @@
left [0] = left [1] = left [2] = left [3] = left [4] = left [5] = 0.0 ;
do
- { fraction = fp_to_double (filter_index) ;- indx = fp_to_int (filter_index) ;
+ { if (data_index >= 0) /* Avoid underflow access to filter->buffer. */+ { fraction = fp_to_double (filter_index) ;+ indx = fp_to_int (filter_index) ;
- icoeff = filter->coeffs [indx] + fraction * (filter->coeffs [indx + 1] - filter->coeffs [indx]) ;
+ icoeff = filter->coeffs [indx] + fraction * (filter->coeffs [indx + 1] - filter->coeffs [indx]) ;
- left [0] += icoeff * filter->buffer [data_index] ;
- left [1] += icoeff * filter->buffer [data_index + 1] ;
- left [2] += icoeff * filter->buffer [data_index + 2] ;
- left [3] += icoeff * filter->buffer [data_index + 3] ;
- left [4] += icoeff * filter->buffer [data_index + 4] ;
- left [5] += icoeff * filter->buffer [data_index + 5] ;
+ left [0] += icoeff * filter->buffer [data_index] ;
+ left [1] += icoeff * filter->buffer [data_index + 1] ;
+ left [2] += icoeff * filter->buffer [data_index + 2] ;
+ left [3] += icoeff * filter->buffer [data_index + 3] ;
+ left [4] += icoeff * filter->buffer [data_index + 4] ;
+ left [5] += icoeff * filter->buffer [data_index + 5] ;
+ } ;
filter_index -= increment ;
data_index = data_index + 6 ;
@@ -910,48 +918,49 @@
icoeff = filter->coeffs [indx] + fraction * (filter->coeffs [indx + 1] - filter->coeffs [indx]) ;
- /*
- ** Duff's Device.
- ** See : http://en.wikipedia.org/wiki/Duff's_device
- */
- ch = channels ;
- do
- {- switch (ch % 8)
- { default :- ch -- ;
- left [ch] += icoeff * filter->buffer [data_index + ch] ;
- /* Falls through. */
- case 7 :
- ch -- ;
- left [ch] += icoeff * filter->buffer [data_index + ch] ;
- /* Falls through. */
- case 6 :
- ch -- ;
- left [ch] += icoeff * filter->buffer [data_index + ch] ;
- /* Falls through. */
- case 5 :
- ch -- ;
- left [ch] += icoeff * filter->buffer [data_index + ch] ;
- /* Falls through. */
- case 4 :
- ch -- ;
- left [ch] += icoeff * filter->buffer [data_index + ch] ;
- /* Falls through. */
- case 3 :
- ch -- ;
- left [ch] += icoeff * filter->buffer [data_index + ch] ;
- /* Falls through. */
- case 2 :
- ch -- ;
- left [ch] += icoeff * filter->buffer [data_index + ch] ;
- /* Falls through. */
- case 1 :
- ch -- ;
- left [ch] += icoeff * filter->buffer [data_index + ch] ;
- } ;
- }
- while (ch > 0) ;
+ if (data_index >= 0) /* Avoid underflow access to filter->buffer. */
+ { /*+ ** Duff's Device.
+ ** See : http://en.wikipedia.org/wiki/Duff's_device
+ */
+ ch = channels ;
+ do
+ { switch (ch % 8)+ { default :+ ch -- ;
+ left [ch] += icoeff * filter->buffer [data_index + ch] ;
+ /* Falls through. */
+ case 7 :
+ ch -- ;
+ left [ch] += icoeff * filter->buffer [data_index + ch] ;
+ /* Falls through. */
+ case 6 :
+ ch -- ;
+ left [ch] += icoeff * filter->buffer [data_index + ch] ;
+ /* Falls through. */
+ case 5 :
+ ch -- ;
+ left [ch] += icoeff * filter->buffer [data_index + ch] ;
+ /* Falls through. */
+ case 4 :
+ ch -- ;
+ left [ch] += icoeff * filter->buffer [data_index + ch] ;
+ /* Falls through. */
+ case 3 :
+ ch -- ;
+ left [ch] += icoeff * filter->buffer [data_index + ch] ;
+ /* Falls through. */
+ case 2 :
+ ch -- ;
+ left [ch] += icoeff * filter->buffer [data_index + ch] ;
+ /* Falls through. */
+ case 1 :
+ ch -- ;
+ left [ch] += icoeff * filter->buffer [data_index + ch] ;
+ } ;
+ }
+ while (ch > 0) ;
+ } ;
filter_index -= increment ;
data_index = data_index + channels ;
--- a/tests/varispeed_test.c
+++ b/tests/varispeed_test.c
@@ -23,7 +23,7 @@
#define fftw_cleanup()
#endif
-#define BUFFER_LEN (1 << 15)
+#define BUFFER_LEN (1 << 14)
static void varispeed_test (int converter, double target_snr) ;
static void varispeed_bounds_test (int converter) ;
@@ -221,22 +221,20 @@
src_data.input_frames = chunk_size ;
src_data.output_frames = total_output_frames ;
- /* Process one chunk at initial_ratio. */
- if ((error = src_process (src_state, &src_data)))
- { printf ("\n\nLine %d : %s : %s\n\n", __LINE__, details, src_strerror (error)) ;- exit (1) ;
- } ;
-
- /* Now hard switch to second_ratio ... */
- src_data.src_ratio = second_ratio ;
- if ((error = src_process (src_state, &src_data)))
- { printf ("\n\nLine %d : %s : %s\n\n", __LINE__, details, src_strerror (error)) ;- exit (1) ;
- } ;
-
- /* ... and process the remaining. */
+ /* Use a max_loop_count here to enable the detection of infinite loops
+ ** (due to end of input not being detected.
+ */
for (k = 0 ; k < max_loop_count ; k ++)
- { if ((error = src_process (src_state, &src_data)) != 0)+ { if (k == 1)+ { /* Hard switch to second_ratio after processing one chunk. */+ src_data.src_ratio = second_ratio ;
+ if ((error = src_set_ratio (src_state, second_ratio)))
+ { printf ("\n\nLine %d : %s : %s\n\n", __LINE__, details, src_strerror (error)) ;+ exit (1) ;
+ } ;
+ } ;
+
+ if ((error = src_process (src_state, &src_data)) != 0)
{ printf ("\n\nLine %d : %s : %s\n\n", __LINE__, details, src_strerror (error)) ;exit (1) ;
} ;