[FFmpeg-devel] [PATCH 1/2] avcodec/liblc3dec: sanitize channel count in avctx

[FFmpeg-devel] [PATCH 1/2] avcodec/liblc3dec: sanitize channel count in avctx

[James Almer]

Should prevent out of array accesses.

Signed-off-by: James Almer <jamrial@gmail.com>
---
 libavcodec/liblc3dec.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/libavcodec/liblc3dec.c b/libavcodec/liblc3dec.c
index c0a31bc91f..52364859d4 100644
--- a/libavcodec/liblc3dec.c
+++ b/libavcodec/liblc3dec.c
@@ -46,6 +46,8 @@ static av_cold int liblc3_decode_init(AVCodecContext *avctx)
 
     if (avctx->extradata_size < 10)
         return AVERROR_INVALIDDATA;
+    if (channels < 0 || channels > DECODER_MAX_CHANNELS)
+        return AVERROR_INVALIDDATA;
 
     liblc3->frame_us = AV_RL16(avctx->extradata + 0) * 10;
     liblc3->srate_hz = avctx->sample_rate;
-- 
2.44.0

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 2/2] avcodec/liblc3enc: don't force unspec channel layouts

[James Almer]

We only care about channel count. Layout details will be ignored either way.

Signed-off-by: James Almer <jamrial@gmail.com>
---
 libavcodec/liblc3enc.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/libavcodec/liblc3enc.c b/libavcodec/liblc3enc.c
index 63d1645b10..5f8169a0cf 100644
--- a/libavcodec/liblc3enc.c
+++ b/libavcodec/liblc3enc.c
@@ -61,6 +61,11 @@ static av_cold int liblc3_encode_init(AVCodecContext *avctx)
                "Unsupported frame duration %.1f ms.\n", frame_us / 1000.f);
         return AVERROR(EINVAL);
     }
+    if (channels < 0 || channels > ENCODER_MAX_CHANNELS) {
+        av_log(avctx, AV_LOG_ERROR,
+               "Unsupported channel count %d. Should be 1 or 2\n", channels);
+        return AVERROR(EINVAL);
+    }
 
     hr_mode |= srate_hz > 48000;
     hr_mode &= srate_hz >= 48000;
@@ -195,9 +200,6 @@ const FFCodec ff_liblc3_encoder = {
     .p.type         = AVMEDIA_TYPE_AUDIO,
     .p.id           = AV_CODEC_ID_LC3,
     .p.capabilities = AV_CODEC_CAP_DR1 | AV_CODEC_CAP_DELAY,
-    .p.ch_layouts = (const AVChannelLayout[])
-        { { AV_CHANNEL_ORDER_UNSPEC, 1 },
-          { AV_CHANNEL_ORDER_UNSPEC, 2 }, { 0 } },
     .p.supported_samplerates = (const int [])
         { 96000, 48000, 32000, 24000, 16000, 8000, 0 },
     .p.sample_fmts = (const enum AVSampleFormat[])
-- 
2.44.0

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 1/2] avcodec/liblc3dec: sanitize channel count in avctx

[Stefano Sabatini]

On date Thursday 2024-04-04 13:29:35 -0300, James Almer wrote:
> Should prevent out of array accesses.
> 
> Signed-off-by: James Almer <jamrial@gmail.com>
> ---
>  libavcodec/liblc3dec.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/libavcodec/liblc3dec.c b/libavcodec/liblc3dec.c
> index c0a31bc91f..52364859d4 100644
> --- a/libavcodec/liblc3dec.c
> +++ b/libavcodec/liblc3dec.c
> @@ -46,6 +46,8 @@ static av_cold int liblc3_decode_init(AVCodecContext *avctx)
>  
>      if (avctx->extradata_size < 10)
>          return AVERROR_INVALIDDATA;

> +    if (channels < 0 || channels > DECODER_MAX_CHANNELS)
> +        return AVERROR_INVALIDDATA;

add a log:
av_log(avctx, AV_LOG_ERROR,
       "Invalid number of channels %d, max %d decoder channels are accepted\n",
       channels, DECODER_MAX_CHANNES);

>      liblc3->frame_us = AV_RL16(avctx->extradata + 0) * 10;
>      liblc3->srate_hz = avctx->sample_rate;

LGTM otherwise, thanks.
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 2/2] avcodec/liblc3enc: don't force unspec channel layouts

[Stefano Sabatini]

On date Thursday 2024-04-04 13:29:36 -0300, James Almer wrote:
> We only care about channel count. Layout details will be ignored either way.
> 
> Signed-off-by: James Almer <jamrial@gmail.com>
> ---
>  libavcodec/liblc3enc.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/libavcodec/liblc3enc.c b/libavcodec/liblc3enc.c
> index 63d1645b10..5f8169a0cf 100644
> --- a/libavcodec/liblc3enc.c
> +++ b/libavcodec/liblc3enc.c
> @@ -61,6 +61,11 @@ static av_cold int liblc3_encode_init(AVCodecContext *avctx)
>                 "Unsupported frame duration %.1f ms.\n", frame_us / 1000.f);
>          return AVERROR(EINVAL);
>      }
> +    if (channels < 0 || channels > ENCODER_MAX_CHANNELS) {
> +        av_log(avctx, AV_LOG_ERROR,
> +               "Unsupported channel count %d. Should be 1 or 2\n", channels);
> +        return AVERROR(EINVAL);
> +    }
>  
>      hr_mode |= srate_hz > 48000;
>      hr_mode &= srate_hz >= 48000;
> @@ -195,9 +200,6 @@ const FFCodec ff_liblc3_encoder = {
>      .p.type         = AVMEDIA_TYPE_AUDIO,
>      .p.id           = AV_CODEC_ID_LC3,
>      .p.capabilities = AV_CODEC_CAP_DR1 | AV_CODEC_CAP_DELAY,

> -    .p.ch_layouts = (const AVChannelLayout[])
> -        { { AV_CHANNEL_ORDER_UNSPEC, 1 },
> -          { AV_CHANNEL_ORDER_UNSPEC, 2 }, { 0 } },

shouldn't this be equivalent?

Should be good anyway.
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 2/2] avcodec/liblc3enc: don't force unspec channel layouts

[James Almer]

On 4/4/2024 1:45 PM, Stefano Sabatini wrote:
> On date Thursday 2024-04-04 13:29:36 -0300, James Almer wrote:
>> We only care about channel count. Layout details will be ignored either way.
>>
>> Signed-off-by: James Almer <jamrial@gmail.com>
>> ---
>>   libavcodec/liblc3enc.c | 8 +++++---
>>   1 file changed, 5 insertions(+), 3 deletions(-)
>>
>> diff --git a/libavcodec/liblc3enc.c b/libavcodec/liblc3enc.c
>> index 63d1645b10..5f8169a0cf 100644
>> --- a/libavcodec/liblc3enc.c
>> +++ b/libavcodec/liblc3enc.c
>> @@ -61,6 +61,11 @@ static av_cold int liblc3_encode_init(AVCodecContext *avctx)
>>                  "Unsupported frame duration %.1f ms.\n", frame_us / 1000.f);
>>           return AVERROR(EINVAL);
>>       }
>> +    if (channels < 0 || channels > ENCODER_MAX_CHANNELS) {
>> +        av_log(avctx, AV_LOG_ERROR,
>> +               "Unsupported channel count %d. Should be 1 or 2\n", channels);
>> +        return AVERROR(EINVAL);
>> +    }
>>   
>>       hr_mode |= srate_hz > 48000;
>>       hr_mode &= srate_hz >= 48000;
>> @@ -195,9 +200,6 @@ const FFCodec ff_liblc3_encoder = {
>>       .p.type         = AVMEDIA_TYPE_AUDIO,
>>       .p.id           = AV_CODEC_ID_LC3,
>>       .p.capabilities = AV_CODEC_CAP_DR1 | AV_CODEC_CAP_DELAY,
> 
>> -    .p.ch_layouts = (const AVChannelLayout[])
>> -        { { AV_CHANNEL_ORDER_UNSPEC, 1 },
>> -          { AV_CHANNEL_ORDER_UNSPEC, 2 }, { 0 } },
> 
> shouldn't this be equivalent?
> 
> Should be good anyway.

No, because if you pass it a mono or stereo layout, the generic encode 
code will reject it, whereas after this change it will work because the 
encoder will only cares about channel count, not overall layout.
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 2/2] avcodec/liblc3enc: don't force unspec channel layouts

[Andreas Rheinhardt]

James Almer:
> We only care about channel count. Layout details will be ignored either way.
> 
> Signed-off-by: James Almer <jamrial@gmail.com>
> ---
>  libavcodec/liblc3enc.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/libavcodec/liblc3enc.c b/libavcodec/liblc3enc.c
> index 63d1645b10..5f8169a0cf 100644
> --- a/libavcodec/liblc3enc.c
> +++ b/libavcodec/liblc3enc.c
> @@ -61,6 +61,11 @@ static av_cold int liblc3_encode_init(AVCodecContext *avctx)
>                 "Unsupported frame duration %.1f ms.\n", frame_us / 1000.f);
>          return AVERROR(EINVAL);
>      }
> +    if (channels < 0 || channels > ENCODER_MAX_CHANNELS) {
> +        av_log(avctx, AV_LOG_ERROR,
> +               "Unsupported channel count %d. Should be 1 or 2\n", channels);
> +        return AVERROR(EINVAL);
> +    }
>  
>      hr_mode |= srate_hz > 48000;
>      hr_mode &= srate_hz >= 48000;
> @@ -195,9 +200,6 @@ const FFCodec ff_liblc3_encoder = {
>      .p.type         = AVMEDIA_TYPE_AUDIO,
>      .p.id           = AV_CODEC_ID_LC3,
>      .p.capabilities = AV_CODEC_CAP_DR1 | AV_CODEC_CAP_DELAY,
> -    .p.ch_layouts = (const AVChannelLayout[])
> -        { { AV_CHANNEL_ORDER_UNSPEC, 1 },
> -          { AV_CHANNEL_ORDER_UNSPEC, 2 }, { 0 } },
>      .p.supported_samplerates = (const int [])
>          { 96000, 48000, 32000, 24000, 16000, 8000, 0 },
>      .p.sample_fmts = (const enum AVSampleFormat[])

I think we should rather change encode.c to mean that an UNSPEC channel
layout (in AVCodec.ch_layouts) works with every channel layout with the
same number of channels.

- Andreas

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 2/2] avcodec/liblc3enc: don't force unspec channel layouts

[Stefano Sabatini]

On date Thursday 2024-04-04 18:59:09 +0200, Andreas Rheinhardt wrote:
> James Almer:
> > We only care about channel count. Layout details will be ignored either way.
> > 
> > Signed-off-by: James Almer <jamrial@gmail.com>
> > ---
> >  libavcodec/liblc3enc.c | 8 +++++---
> >  1 file changed, 5 insertions(+), 3 deletions(-)
> > 
> > diff --git a/libavcodec/liblc3enc.c b/libavcodec/liblc3enc.c
> > index 63d1645b10..5f8169a0cf 100644
> > --- a/libavcodec/liblc3enc.c
> > +++ b/libavcodec/liblc3enc.c
> > @@ -61,6 +61,11 @@ static av_cold int liblc3_encode_init(AVCodecContext *avctx)
> >                 "Unsupported frame duration %.1f ms.\n", frame_us / 1000.f);
> >          return AVERROR(EINVAL);
> >      }
> > +    if (channels < 0 || channels > ENCODER_MAX_CHANNELS) {
> > +        av_log(avctx, AV_LOG_ERROR,
> > +               "Unsupported channel count %d. Should be 1 or 2\n", channels);
> > +        return AVERROR(EINVAL);
> > +    }
> >  
> >      hr_mode |= srate_hz > 48000;
> >      hr_mode &= srate_hz >= 48000;
> > @@ -195,9 +200,6 @@ const FFCodec ff_liblc3_encoder = {
> >      .p.type         = AVMEDIA_TYPE_AUDIO,
> >      .p.id           = AV_CODEC_ID_LC3,
> >      .p.capabilities = AV_CODEC_CAP_DR1 | AV_CODEC_CAP_DELAY,
> > -    .p.ch_layouts = (const AVChannelLayout[])
> > -        { { AV_CHANNEL_ORDER_UNSPEC, 1 },
> > -          { AV_CHANNEL_ORDER_UNSPEC, 2 }, { 0 } },
> >      .p.supported_samplerates = (const int [])
> >          { 96000, 48000, 32000, 24000, 16000, 8000, 0 },
> >      .p.sample_fmts = (const enum AVSampleFormat[])
> 

> I think we should rather change encode.c to mean that an UNSPEC channel
> layout (in AVCodec.ch_layouts) works with every channel layout with the
> same number of channels.

This was my naive interpretation of the code, without checking the
code. This would probably save some boilerplate (OTOH it should also
be fine to apply the hotfix).
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".