Skip to content

Commit c10c819

Browse files
erikderikd
committed
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: #5
1 parent 8c325e3 commit c10c819

File tree

2 files changed

+92
-85
lines changed

2 files changed

+92
-85
lines changed

‎src/src_sinc.c

Lines changed: 78 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -295,12 +295,14 @@ calc_output_single (SINC_FILTER *filter, increment_t increment, increment_t star
295295

296296
left = 0.0 ;
297297
do
298-
{ fraction = fp_to_double (filter_index) ;
299-
indx = fp_to_int (filter_index) ;
298+
{ if (data_index >= 0) /* Avoid underflow access to filter->buffer. */
299+
{ fraction = fp_to_double (filter_index) ;
300+
indx = fp_to_int (filter_index) ;
300301

301-
icoeff = filter->coeffs [indx] + fraction * (filter->coeffs [indx + 1] - filter->coeffs [indx]) ;
302+
icoeff = filter->coeffs [indx] + fraction * (filter->coeffs [indx + 1] - filter->coeffs [indx]) ;
302303

303-
left += icoeff * filter->buffer [data_index] ;
304+
left += icoeff * filter->buffer [data_index] ;
305+
} ;
304306

305307
filter_index -= increment ;
306308
data_index = data_index + 1 ;
@@ -440,13 +442,15 @@ calc_output_stereo (SINC_FILTER *filter, increment_t increment, increment_t star
440442

441443
left [0] = left [1] = 0.0 ;
442444
do
443-
{ fraction = fp_to_double (filter_index) ;
444-
indx = fp_to_int (filter_index) ;
445+
{ if (data_index >= 0) /* Avoid underflow access to filter->buffer. */
446+
{ fraction = fp_to_double (filter_index) ;
447+
indx = fp_to_int (filter_index) ;
445448

446-
icoeff = filter->coeffs [indx] + fraction * (filter->coeffs [indx + 1] - filter->coeffs [indx]) ;
449+
icoeff = filter->coeffs [indx] + fraction * (filter->coeffs [indx + 1] - filter->coeffs [indx]) ;
447450

448-
left [0] += icoeff * filter->buffer [data_index] ;
449-
left [1] += icoeff * filter->buffer [data_index + 1] ;
451+
left [0] += icoeff * filter->buffer [data_index] ;
452+
left [1] += icoeff * filter->buffer [data_index + 1] ;
453+
} ;
450454

451455
filter_index -= increment ;
452456
data_index = data_index + 2 ;
@@ -587,15 +591,17 @@ calc_output_quad (SINC_FILTER *filter, increment_t increment, increment_t start_
587591

588592
left [0] = left [1] = left [2] = left [3] = 0.0 ;
589593
do
590-
{ fraction = fp_to_double (filter_index) ;
591-
indx = fp_to_int (filter_index) ;
594+
{ if (data_index >= 0) /* Avoid underflow access to filter->buffer. */
595+
{ fraction = fp_to_double (filter_index) ;
596+
indx = fp_to_int (filter_index) ;
592597

593-
icoeff = filter->coeffs [indx] + fraction * (filter->coeffs [indx + 1] - filter->coeffs [indx]) ;
598+
icoeff = filter->coeffs [indx] + fraction * (filter->coeffs [indx + 1] - filter->coeffs [indx]) ;
594599

595-
left [0] += icoeff * filter->buffer [data_index] ;
596-
left [1] += icoeff * filter->buffer [data_index + 1] ;
597-
left [2] += icoeff * filter->buffer [data_index + 2] ;
598-
left [3] += icoeff * filter->buffer [data_index + 3] ;
600+
left [0] += icoeff * filter->buffer [data_index] ;
601+
left [1] += icoeff * filter->buffer [data_index + 1] ;
602+
left [2] += icoeff * filter->buffer [data_index + 2] ;
603+
left [3] += icoeff * filter->buffer [data_index + 3] ;
604+
} ;
599605

600606
filter_index -= increment ;
601607
data_index = data_index + 4 ;
@@ -740,17 +746,19 @@ calc_output_hex (SINC_FILTER *filter, increment_t increment, increment_t start_f
740746

741747
left [0] = left [1] = left [2] = left [3] = left [4] = left [5] = 0.0 ;
742748
do
743-
{ fraction = fp_to_double (filter_index) ;
744-
indx = fp_to_int (filter_index) ;
745-
746-
icoeff = filter->coeffs [indx] + fraction * (filter->coeffs [indx + 1] - filter->coeffs [indx]) ;
747-
748-
left [0] += icoeff * filter->buffer [data_index] ;
749-
left [1] += icoeff * filter->buffer [data_index + 1] ;
750-
left [2] += icoeff * filter->buffer [data_index + 2] ;
751-
left [3] += icoeff * filter->buffer [data_index + 3] ;
752-
left [4] += icoeff * filter->buffer [data_index + 4] ;
753-
left [5] += icoeff * filter->buffer [data_index + 5] ;
749+
{ if (data_index >= 0) /* Avoid underflow access to filter->buffer. */
750+
{ fraction = fp_to_double (filter_index) ;
751+
indx = fp_to_int (filter_index) ;
752+
753+
icoeff = filter->coeffs [indx] + fraction * (filter->coeffs [indx + 1] - filter->coeffs [indx]) ;
754+
755+
left [0] += icoeff * filter->buffer [data_index] ;
756+
left [1] += icoeff * filter->buffer [data_index + 1] ;
757+
left [2] += icoeff * filter->buffer [data_index + 2] ;
758+
left [3] += icoeff * filter->buffer [data_index + 3] ;
759+
left [4] += icoeff * filter->buffer [data_index + 4] ;
760+
left [5] += icoeff * filter->buffer [data_index + 5] ;
761+
} ;
754762

755763
filter_index -= increment ;
756764
data_index = data_index + 6 ;
@@ -910,48 +918,49 @@ calc_output_multi (SINC_FILTER *filter, increment_t increment, increment_t start
910918

911919
icoeff = filter->coeffs [indx] + fraction * (filter->coeffs [indx + 1] - filter->coeffs [indx]) ;
912920

913-
/*
914-
** Duff's Device.
915-
** See : http://en.wikipedia.org/wiki/Duff's_device
916-
*/
917-
ch = channels ;
918-
do
919-
{
920-
switch (ch % 8)
921-
{ default :
922-
ch -- ;
923-
left [ch] += icoeff * filter->buffer [data_index + ch] ;
924-
/* Falls through. */
925-
case 7 :
926-
ch -- ;
927-
left [ch] += icoeff * filter->buffer [data_index + ch] ;
928-
/* Falls through. */
929-
case 6 :
930-
ch -- ;
931-
left [ch] += icoeff * filter->buffer [data_index + ch] ;
932-
/* Falls through. */
933-
case 5 :
934-
ch -- ;
935-
left [ch] += icoeff * filter->buffer [data_index + ch] ;
936-
/* Falls through. */
937-
case 4 :
938-
ch -- ;
939-
left [ch] += icoeff * filter->buffer [data_index + ch] ;
940-
/* Falls through. */
941-
case 3 :
942-
ch -- ;
943-
left [ch] += icoeff * filter->buffer [data_index + ch] ;
944-
/* Falls through. */
945-
case 2 :
946-
ch -- ;
947-
left [ch] += icoeff * filter->buffer [data_index + ch] ;
948-
/* Falls through. */
949-
case 1 :
950-
ch -- ;
951-
left [ch] += icoeff * filter->buffer [data_index + ch] ;
952-
} ;
953-
}
954-
while (ch > 0) ;
921+
if (data_index >= 0) /* Avoid underflow access to filter->buffer. */
922+
{ /*
923+
** Duff's Device.
924+
** See : http://en.wikipedia.org/wiki/Duff's_device
925+
*/
926+
ch = channels ;
927+
do
928+
{ switch (ch % 8)
929+
{ default :
930+
ch -- ;
931+
left [ch] += icoeff * filter->buffer [data_index + ch] ;
932+
/* Falls through. */
933+
case 7 :
934+
ch -- ;
935+
left [ch] += icoeff * filter->buffer [data_index + ch] ;
936+
/* Falls through. */
937+
case 6 :
938+
ch -- ;
939+
left [ch] += icoeff * filter->buffer [data_index + ch] ;
940+
/* Falls through. */
941+
case 5 :
942+
ch -- ;
943+
left [ch] += icoeff * filter->buffer [data_index + ch] ;
944+
/* Falls through. */
945+
case 4 :
946+
ch -- ;
947+
left [ch] += icoeff * filter->buffer [data_index + ch] ;
948+
/* Falls through. */
949+
case 3 :
950+
ch -- ;
951+
left [ch] += icoeff * filter->buffer [data_index + ch] ;
952+
/* Falls through. */
953+
case 2 :
954+
ch -- ;
955+
left [ch] += icoeff * filter->buffer [data_index + ch] ;
956+
/* Falls through. */
957+
case 1 :
958+
ch -- ;
959+
left [ch] += icoeff * filter->buffer [data_index + ch] ;
960+
} ;
961+
}
962+
while (ch > 0) ;
963+
} ;
955964

956965
filter_index -= increment ;
957966
data_index = data_index + channels ;

‎tests/varispeed_test.c

Lines changed: 14 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@
2323
#define fftw_cleanup()
2424
#endif
2525

26-
#define BUFFER_LEN (1 << 15)
26+
#define BUFFER_LEN (1 << 14)
2727

2828
static void varispeed_test (int converter, double target_snr) ;
2929
static void varispeed_bounds_test (int converter) ;
@@ -221,22 +221,20 @@ set_ratio_test (int converter, int channels, double initial_ratio, double second
221221
src_data.input_frames = chunk_size ;
222222
src_data.output_frames = total_output_frames ;
223223

224-
/* Process one chunk at initial_ratio. */
225-
if ((error = src_process (src_state, &src_data)))
226-
{ printf ("\n\nLine %d : %s : %s\n\n", __LINE__, details, src_strerror (error)) ;
227-
exit (1) ;
228-
} ;
229-
230-
/* Now hard switch to second_ratio ... */
231-
src_data.src_ratio = second_ratio ;
232-
if ((error = src_process (src_state, &src_data)))
233-
{ printf ("\n\nLine %d : %s : %s\n\n", __LINE__, details, src_strerror (error)) ;
234-
exit (1) ;
235-
} ;
236-
237-
/* ... and process the remaining. */
224+
/* Use a max_loop_count here to enable the detection of infinite loops
225+
** (due to end of input not being detected.
226+
*/
238227
for (k = 0 ; k < max_loop_count ; k ++)
239-
{ if ((error = src_process (src_state, &src_data)) != 0)
228+
{ if (k == 1)
229+
{ /* Hard switch to second_ratio after processing one chunk. */
230+
src_data.src_ratio = second_ratio ;
231+
if ((error = src_set_ratio (src_state, second_ratio)))
232+
{ printf ("\n\nLine %d : %s : %s\n\n", __LINE__, details, src_strerror (error)) ;
233+
exit (1) ;
234+
} ;
235+
} ;
236+
237+
if ((error = src_process (src_state, &src_data)) != 0)
240238
{ printf ("\n\nLine %d : %s : %s\n\n", __LINE__, details, src_strerror (error)) ;
241239
exit (1) ;
242240
} ;

0 commit comments

Comments
 (0)