[pulseaudio-discuss] [PATCH v8 07/12] bluetooth: Add A2DP aptX and aptX HD codecs support

[Tanu Kaskinen]

On Fri, 2019-04-19 at 12:52 +0200, Pali Rohár wrote:
> On Friday 19 April 2019 11:41:22 Tanu Kaskinen wrote:
> > On Sat, 2019-04-06 at 11:16 +0200, Pali Rohár wrote:
> > > +static size_t decode_buffer(void *codec_info, const uint8_t *input_buffer, size_t input_size, uint8_t *output_buffer, size_t output_size, size_t *processed) {
> > > +    struct aptx_context *aptx_c = (struct aptx_context *) codec_info;
> > > +
> > > +    const uint8_t *p;
> > > +    uint8_t *d;
> > > +    size_t to_write, to_decode;
> > > +
> > > +    p = input_buffer;
> > > +    to_decode = input_size;
> > > +
> > > +    d = output_buffer;
> > > +    to_write = output_size;
> > > +
> > > +    while (PA_LIKELY(to_decode > 0)) {
> > 
> > Is it intentional that encode_buffer() checks both to_encode and
> > to_write in the while loop condition, but decode_buffer() only checks
> > to_decode? If so, why does encode_buffer() need to be extra careful but
> > decode_buffer() doesn't?
> 
> These checks are similar/same as in SBC codec code. And SBC code was
> there prior to my work. So personally I do not know.

Ok, let's find out if this code needs changing.

The output buffer size when decoding is determined by
get_read_block_size(), which returns a value corresponding to the read
MTU (the MTU is in the compressed domain, the block size is in the
uncompressed domain).

The input buffer size is potentially up to twice the read MTU.

It doesn't seem impossible that the socket could have more data queued
than just one MTU. Therefore this doesn't seem safe, because
decode_buffer() may overrun the output buffer. The solution isn't to
check to_write, however, because that would lead to some audio getting
dropped. I think the input buffer size should be limited to roughly one
MTU just like the output buffer size is limited to roughly one MTU. The
exact logic is codec dependent, so the codec implementation should
decide both the input and output buffer sizes, and it should be ensured
that the output buffer is big enough to fully decode a full input
buffer.

-- 
Tanu

https://www.patreon.com/tanuk
https://liberapay.com/tanuk