TL;DR: for an 8khz input stream captured on a 48khz device, the format conversion temp buffer was allocated on the basis of reading in x *device* frames per chunk (<x converted frames), but the actual copy was done using x *converted* stream frames (>x unconverted) per chunk, causing a buffer overflow.

--

While working on USB-C loopback tests, I encountered a crash in cras when capturing audio at 8000Hz. The crash is 100% reproducible on chell, using a USB-C -> 3.5mm dongle, with the following command:

/usr/bin/cras_test_client --capture_file /dev/null --duration 1 --num_channels 1 --rate 8000 

The device's supported params:
...
FORMAT:  S24_3LE
SUBFORMAT:  STD
SAMPLE_BITS: 24
FRAME_BITS: 48
CHANNELS: 2
RATE: 48000
...

Debugging with valgrind (take care to run as 'cras' user, otherwise cras can't register dbus interface, and therefore does not switch active input node to the USB device) shows invalid read and writes in format conversion:

==3945== Thread 2:
==3945== Invalid write of size 2
==3945==    at 0x128E73: convert_s243le_to_s16le (string3.h:65)
==3945==    by 0x12A5F7: cras_fmt_conv_convert_frames (cras_fmt_conv.c:844)
==3945==    by 0x137750: dev_stream_capture (dev_stream.c:260)
==3945==    by 0x135F3E: dev_io_capture (dev_io.c:374)
==3945==    by 0x13683A: dev_io_run (dev_io.c:721)
==3945==    by 0x122437: audio_io_thread (audio_thread.c:845)
==3945==    by 0x4A2E2B7: start_thread (pthread_create.c:333)
==3945==    by 0x544CFAC: clone (clone.S:109)
==3945==  Address 0x722c190 is 0 bytes after a block of size 640 alloc'd
==3945==    at 0x402B9E5: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==3945==    by 0x1286D6: cras_fmt_conv_create (cras_fmt_conv.c:585)
==3945==    by 0x12A87D: config_format_converter (cras_fmt_conv.c:947)
==3945==    by 0x136EB1: dev_stream_create (dev_stream.c:0)
==3945==    by 0x121BC7: audio_io_thread (audio_thread.c:274)
==3945==    by 0x4A2E2B7: start_thread (pthread_create.c:333)
==3945==    by 0x544CFAC: clone (clone.S:109)

==3945== Invalid read of size 2
==3945==    at 0x40AEB90: speex_resampler_process_int (in /usr/lib64/libspeexdsp.so.1.5.0)
==3945==    by 0x40AF5D5: speex_resampler_process_interleaved_int (in /usr/lib64/libspeexdsp.so.1.5.0)
==3945==    by 0x12A6C9: cras_fmt_conv_convert_frames (cras_fmt_conv.c:878)
.....
==3945==  Address 0x722c190 is 0 bytes after a block of size 640 alloc'd
==3945==    at 0x402B9E5: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==3945==    by 0x1286D6: cras_fmt_conv_create (cras_fmt_conv.c:585)
......

The buffer in question is allocated here:
https://chromium.googlesource.com/chromiumos/third_party/adhd/+/master/cras/src/server/cras_fmt_conv.c#583

The size of the buffer is defined here:
https://chromium.googlesource.com/chromiumos/third_party/adhd/+/master/cras/src/server/dev_stream.c#63

The converter's input is the buffer returned by the ALSA mmap call:
https://chromium.googlesource.com/chromiumos/third_party/adhd/+/master/cras/src/server/cras_alsa_helpers.c#793

The number of frames requested from/returned by ALSA is dependent on all streams attached to the device:
https://chromium.googlesource.com/chromiumos/third_party/adhd/+/master/cras/src/server/dev_io.c#229

The per-stream calculation of available space is here:
https://chromium.googlesource.com/chromiumos/third_party/adhd/+/master/cras/src/server/dev_stream.c#424

The sample rate conversion is taken into account in both the allocation of the buffer, and the calculation of how much data to request from ALSA.

In this instance, the device rate is 48khz, and the stream rate is 8khz.

When the buffer is allocated, the sample rate conversion is done here:
https://chromium.googlesource.com/chromiumos/third_party/adhd/+/master/cras/src/server/dev_stream.c#64

The conversion converts stream->buffer_frames (here, 80) at 48khz, to the equivalent number of frames at 8khz, which is 14.

When the buffer is populated, the sample rate conversion in the number of frames to copy is done here:
https://chromium.googlesource.com/chromiumos/third_party/adhd/+/master/cras/src/server/dev_stream.c#460

This conversion takes a parameter of 80 frames (supposed to represent 80 frames of converted audio at 8khz), and returns the number of input frames (at 48khz) required to produce this output: 480.

So: the buffer was allocated on the basis of reading in 80 *device* frames per chunk (14 converted frames), but the actual copy was done using 80 *converted* stream frames (480 unconverted) per chunk.

It seems that the allocation is incorrect, and the conversion is as expected; stream->buffer_frames (80 in this example) is defined (indirectly, via block_size) by the client, so is referring to converted output frames:
https://chromium.googlesource.com/chromiumos/third_party/adhd/+/master/cras/src/libcras/cras_client.c#2310
https://chromium.googlesource.com/chromiumos/third_party/adhd/+/master/cras/src/server/cras_rclient.c#65
https://chromium.googlesource.com/chromiumos/third_party/adhd/+/master/cras/src/server/cras_rstream.c#233

The appropriate fix appears to be to update the size calculation during allocation, so that the buffer size is large enough to hold the requested number of converted output frames (rather than that many input frames).

It seems like this was a latent bug, perhaps partly hidden by the allocation of 4 bytes per sample (to fit the largest supported size), regardless of input:
https://chromium.googlesource.com/chromiumos/third_party/adhd/+/master/cras/src/server/cras_fmt_conv.c#585

The bug likely showed up in this instance due to the large difference in sample rates, and 24 bit source samples.

Related issues (will update bug after more investigation):

1) Need to verify end to end, but given the size calculation in allocation for output streams is the inverse of that for input streams, it's likely the format conversion buffers are being over-allocated, and could be reduced to save memory.

2) The allocation uses cras_frames_at_rate(..) to do frame count conversion, whereas the copy uses cras_fmt_conv_out_frames_to_in(..). These are usually different by 1, due to the call to linear_resampler_out_frames_to_in in the latter. This discrepancy needs to be resolved.

3) The copy uses takes the BULK_AUDIO_OK stream flag into account, and may use stream->buffer_frames or cb_threshold:
https://chromium.googlesource.com/chromiumos/third_party/adhd/+/master/cras/src/server/cras_rstream.h#159
..whereas the allocate always used buffer_frames:
https://chromium.googlesource.com/chromiumos/third_party/adhd/+/master/cras/src/server/dev_stream.c#63
Need to check if/how a client could create a stream with cb_threshold > buffer_frames, and if that would cause an overflow.

4) The output of the format conversion is stored in another temporary buffer, which uses the correct sample rate conversion numbers, but allocates 2x this, would be good to understand why:
https://chromium.googlesource.com/chromiumos/third_party/adhd/+/master/cras/src/server/dev_stream.c#86