* [FFmpeg-devel] [PATCH 2/7] avcodec/av1dec: initialize ret in av1_receive_frame_internal()
2024-05-02 0:41 [FFmpeg-devel] [PATCH 1/7] avcodec/av1dec: bit_depth cannot be another values than 8, 10, 12 Michael Niedermayer
@ 2024-05-02 0:41 ` Michael Niedermayer
2024-05-02 0:55 ` James Almer
2024-05-02 7:12 ` Andreas Rheinhardt
2024-05-02 0:41 ` [FFmpeg-devel] [PATCH 3/7] avcodec/avfft: Remove dead code Michael Niedermayer
` (5 subsequent siblings)
6 siblings, 2 replies; 24+ messages in thread
From: Michael Niedermayer @ 2024-05-02 0:41 UTC (permalink / raw)
To: FFmpeg development discussions and patches
Fixes: CID1596605 Uninitialized scalar variable
Sponsored-by: Sovereign Tech Fund
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
---
libavcodec/av1dec.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libavcodec/av1dec.c b/libavcodec/av1dec.c
index 4f9222cca27..93ab04eb378 100644
--- a/libavcodec/av1dec.c
+++ b/libavcodec/av1dec.c
@@ -1262,7 +1262,7 @@ static int av1_receive_frame_internal(AVCodecContext *avctx, AVFrame *frame)
{
AV1DecContext *s = avctx->priv_data;
AV1RawTileGroup *raw_tile_group = NULL;
- int i = 0, ret;
+ int i = 0, ret = 0;
for (i = s->nb_unit; i < s->current_obu.nb_units; i++) {
CodedBitstreamUnit *unit = &s->current_obu.units[i];
--
2.43.2
_______________________________________________
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".
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [FFmpeg-devel] [PATCH 2/7] avcodec/av1dec: initialize ret in av1_receive_frame_internal()
2024-05-02 0:41 ` [FFmpeg-devel] [PATCH 2/7] avcodec/av1dec: initialize ret in av1_receive_frame_internal() Michael Niedermayer
@ 2024-05-02 0:55 ` James Almer
2024-05-02 7:12 ` Andreas Rheinhardt
1 sibling, 0 replies; 24+ messages in thread
From: James Almer @ 2024-05-02 0:55 UTC (permalink / raw)
To: ffmpeg-devel
On 5/1/2024 9:41 PM, Michael Niedermayer wrote:
> Fixes: CID1596605 Uninitialized scalar variable
>
> Sponsored-by: Sovereign Tech Fund
> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> ---
> libavcodec/av1dec.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/libavcodec/av1dec.c b/libavcodec/av1dec.c
> index 4f9222cca27..93ab04eb378 100644
> --- a/libavcodec/av1dec.c
> +++ b/libavcodec/av1dec.c
> @@ -1262,7 +1262,7 @@ static int av1_receive_frame_internal(AVCodecContext *avctx, AVFrame *frame)
> {
> AV1DecContext *s = avctx->priv_data;
> AV1RawTileGroup *raw_tile_group = NULL;
> - int i = 0, ret;
> + int i = 0, ret = 0;
>
> for (i = s->nb_unit; i < s->current_obu.nb_units; i++) {
> CodedBitstreamUnit *unit = &s->current_obu.units[i];
Should be ok. Alternatively, we could set ret to AVERROR_BUG here and to
0 in the places where it's missing (and that triggered coverity).
_______________________________________________
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".
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [FFmpeg-devel] [PATCH 2/7] avcodec/av1dec: initialize ret in av1_receive_frame_internal()
2024-05-02 0:41 ` [FFmpeg-devel] [PATCH 2/7] avcodec/av1dec: initialize ret in av1_receive_frame_internal() Michael Niedermayer
2024-05-02 0:55 ` James Almer
@ 2024-05-02 7:12 ` Andreas Rheinhardt
2024-05-02 21:29 ` Michael Niedermayer
1 sibling, 1 reply; 24+ messages in thread
From: Andreas Rheinhardt @ 2024-05-02 7:12 UTC (permalink / raw)
To: ffmpeg-devel
Michael Niedermayer:
> Fixes: CID1596605 Uninitialized scalar variable
>
> Sponsored-by: Sovereign Tech Fund
> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> ---
> libavcodec/av1dec.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/libavcodec/av1dec.c b/libavcodec/av1dec.c
> index 4f9222cca27..93ab04eb378 100644
> --- a/libavcodec/av1dec.c
> +++ b/libavcodec/av1dec.c
> @@ -1262,7 +1262,7 @@ static int av1_receive_frame_internal(AVCodecContext *avctx, AVFrame *frame)
> {
> AV1DecContext *s = avctx->priv_data;
> AV1RawTileGroup *raw_tile_group = NULL;
> - int i = 0, ret;
> + int i = 0, ret = 0;
>
> for (i = s->nb_unit; i < s->current_obu.nb_units; i++) {
> CodedBitstreamUnit *unit = &s->current_obu.units[i];
A better approach is to actually initialize ret before every goto end in
order to ensure that only the actually desired ret is returned and not
some earlier value.
- 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".
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [FFmpeg-devel] [PATCH 2/7] avcodec/av1dec: initialize ret in av1_receive_frame_internal()
2024-05-02 7:12 ` Andreas Rheinhardt
@ 2024-05-02 21:29 ` Michael Niedermayer
2024-05-02 21:34 ` Andreas Rheinhardt
0 siblings, 1 reply; 24+ messages in thread
From: Michael Niedermayer @ 2024-05-02 21:29 UTC (permalink / raw)
To: FFmpeg development discussions and patches
[-- Attachment #1.1: Type: text/plain, Size: 1523 bytes --]
On Thu, May 02, 2024 at 09:12:36AM +0200, Andreas Rheinhardt wrote:
> Michael Niedermayer:
> > Fixes: CID1596605 Uninitialized scalar variable
> >
> > Sponsored-by: Sovereign Tech Fund
> > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> > ---
> > libavcodec/av1dec.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/libavcodec/av1dec.c b/libavcodec/av1dec.c
> > index 4f9222cca27..93ab04eb378 100644
> > --- a/libavcodec/av1dec.c
> > +++ b/libavcodec/av1dec.c
> > @@ -1262,7 +1262,7 @@ static int av1_receive_frame_internal(AVCodecContext *avctx, AVFrame *frame)
> > {
> > AV1DecContext *s = avctx->priv_data;
> > AV1RawTileGroup *raw_tile_group = NULL;
> > - int i = 0, ret;
> > + int i = 0, ret = 0;
> >
> > for (i = s->nb_unit; i < s->current_obu.nb_units; i++) {
> > CodedBitstreamUnit *unit = &s->current_obu.units[i];
>
> A better approach is to actually initialize ret before every goto end in
> order to ensure that only the actually desired ret is returned and not
> some earlier value.
ok, will apply with ret = 0 before the 2 goto end lacking it that i see
thx
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
The real ebay dictionary, page 1
"Used only once" - "Some unspecified defect prevented a second use"
"In good condition" - "Can be repaird by experienced expert"
"As is" - "You wouldnt want it even if you were payed for it, if you knew ..."
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]
[-- Attachment #2: Type: text/plain, Size: 251 bytes --]
_______________________________________________
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".
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [FFmpeg-devel] [PATCH 2/7] avcodec/av1dec: initialize ret in av1_receive_frame_internal()
2024-05-02 21:29 ` Michael Niedermayer
@ 2024-05-02 21:34 ` Andreas Rheinhardt
2024-05-02 21:41 ` Michael Niedermayer
0 siblings, 1 reply; 24+ messages in thread
From: Andreas Rheinhardt @ 2024-05-02 21:34 UTC (permalink / raw)
To: ffmpeg-devel
Michael Niedermayer:
> On Thu, May 02, 2024 at 09:12:36AM +0200, Andreas Rheinhardt wrote:
>> Michael Niedermayer:
>>> Fixes: CID1596605 Uninitialized scalar variable
>>>
>>> Sponsored-by: Sovereign Tech Fund
>>> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
>>> ---
>>> libavcodec/av1dec.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/libavcodec/av1dec.c b/libavcodec/av1dec.c
>>> index 4f9222cca27..93ab04eb378 100644
>>> --- a/libavcodec/av1dec.c
>>> +++ b/libavcodec/av1dec.c
>>> @@ -1262,7 +1262,7 @@ static int av1_receive_frame_internal(AVCodecContext *avctx, AVFrame *frame)
>>> {
>>> AV1DecContext *s = avctx->priv_data;
>>> AV1RawTileGroup *raw_tile_group = NULL;
>>> - int i = 0, ret;
>>> + int i = 0, ret = 0;
>>>
>>> for (i = s->nb_unit; i < s->current_obu.nb_units; i++) {
>>> CodedBitstreamUnit *unit = &s->current_obu.units[i];
>>
>> A better approach is to actually initialize ret before every goto end in
>> order to ensure that only the actually desired ret is returned and not
>> some earlier value.
>
> ok, will apply with ret = 0 before the 2 goto end lacking it that i see
>
I already sent a patch for this.
- 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".
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [FFmpeg-devel] [PATCH 2/7] avcodec/av1dec: initialize ret in av1_receive_frame_internal()
2024-05-02 21:34 ` Andreas Rheinhardt
@ 2024-05-02 21:41 ` Michael Niedermayer
0 siblings, 0 replies; 24+ messages in thread
From: Michael Niedermayer @ 2024-05-02 21:41 UTC (permalink / raw)
To: FFmpeg development discussions and patches
[-- Attachment #1.1: Type: text/plain, Size: 1672 bytes --]
On Thu, May 02, 2024 at 11:34:48PM +0200, Andreas Rheinhardt wrote:
> Michael Niedermayer:
> > On Thu, May 02, 2024 at 09:12:36AM +0200, Andreas Rheinhardt wrote:
> >> Michael Niedermayer:
> >>> Fixes: CID1596605 Uninitialized scalar variable
> >>>
> >>> Sponsored-by: Sovereign Tech Fund
> >>> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> >>> ---
> >>> libavcodec/av1dec.c | 2 +-
> >>> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>>
> >>> diff --git a/libavcodec/av1dec.c b/libavcodec/av1dec.c
> >>> index 4f9222cca27..93ab04eb378 100644
> >>> --- a/libavcodec/av1dec.c
> >>> +++ b/libavcodec/av1dec.c
> >>> @@ -1262,7 +1262,7 @@ static int av1_receive_frame_internal(AVCodecContext *avctx, AVFrame *frame)
> >>> {
> >>> AV1DecContext *s = avctx->priv_data;
> >>> AV1RawTileGroup *raw_tile_group = NULL;
> >>> - int i = 0, ret;
> >>> + int i = 0, ret = 0;
> >>>
> >>> for (i = s->nb_unit; i < s->current_obu.nb_units; i++) {
> >>> CodedBitstreamUnit *unit = &s->current_obu.units[i];
> >>
> >> A better approach is to actually initialize ret before every goto end in
> >> order to ensure that only the actually desired ret is returned and not
> >> some earlier value.
> >
> > ok, will apply with ret = 0 before the 2 goto end lacking it that i see
> >
>
> I already sent a patch for this.
ok, ill drop mine then
thx
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
The bravest are surely those who have the clearest vision
of what is before them, glory and danger alike, and yet
notwithstanding go out to meet it. -- Thucydides
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]
[-- Attachment #2: Type: text/plain, Size: 251 bytes --]
_______________________________________________
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".
^ permalink raw reply [flat|nested] 24+ messages in thread
* [FFmpeg-devel] [PATCH 3/7] avcodec/avfft: Remove dead code
2024-05-02 0:41 [FFmpeg-devel] [PATCH 1/7] avcodec/av1dec: bit_depth cannot be another values than 8, 10, 12 Michael Niedermayer
2024-05-02 0:41 ` [FFmpeg-devel] [PATCH 2/7] avcodec/av1dec: initialize ret in av1_receive_frame_internal() Michael Niedermayer
@ 2024-05-02 0:41 ` Michael Niedermayer
2024-05-02 1:21 ` Lynne
2024-05-02 0:41 ` [FFmpeg-devel] [PATCH 4/7] avcodec/avs2_parser: Assert init_get_bits8() success with const size 15 Michael Niedermayer
` (4 subsequent siblings)
6 siblings, 1 reply; 24+ messages in thread
From: Michael Niedermayer @ 2024-05-02 0:41 UTC (permalink / raw)
To: FFmpeg development discussions and patches
Fixes: CID1543204 Logically dead code
Sponsored-by: Sovereign Tech Fund
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
---
libavcodec/avfft.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libavcodec/avfft.c b/libavcodec/avfft.c
index f6787937f67..0f43f30b776 100644
--- a/libavcodec/avfft.c
+++ b/libavcodec/avfft.c
@@ -158,7 +158,7 @@ RDFTContext *av_rdft_init(int nbits, enum RDFTransformType trans)
return NULL;
}
- s->stride = (trans == DFT_C2R) ? sizeof(AVComplexFloat) : sizeof(float);
+ s->stride = sizeof(float);
s->len = 1 << nbits;
s->inv = trans == IDFT_C2R;
--
2.43.2
_______________________________________________
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".
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [FFmpeg-devel] [PATCH 3/7] avcodec/avfft: Remove dead code
2024-05-02 0:41 ` [FFmpeg-devel] [PATCH 3/7] avcodec/avfft: Remove dead code Michael Niedermayer
@ 2024-05-02 1:21 ` Lynne
2024-05-02 1:28 ` Michael Niedermayer
0 siblings, 1 reply; 24+ messages in thread
From: Lynne @ 2024-05-02 1:21 UTC (permalink / raw)
To: FFmpeg development discussions and patches
May 2, 2024, 02:42 by michael@niedermayer.cc:
> Fixes: CID1543204 Logically dead code
>
> Sponsored-by: Sovereign Tech Fund
> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> ---
> libavcodec/avfft.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/libavcodec/avfft.c b/libavcodec/avfft.c
> index f6787937f67..0f43f30b776 100644
> --- a/libavcodec/avfft.c
> +++ b/libavcodec/avfft.c
> @@ -158,7 +158,7 @@ RDFTContext *av_rdft_init(int nbits, enum RDFTransformType trans)
> return NULL;
> }
>
> - s->stride = (trans == DFT_C2R) ? sizeof(AVComplexFloat) : sizeof(float);
> + s->stride = sizeof(float);
> s->len = 1 << nbits;
> s->inv = trans == IDFT_C2R;
>
That's not right.
While it's true that currently the stride parameter in av_tx_fn
is unused for RDFTs, that may not always be the case, and the
documentation requires that the stride is valid and set to the
value that the current implementation assumes, so that nothing
breaks once that is implemented.
_______________________________________________
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".
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [FFmpeg-devel] [PATCH 3/7] avcodec/avfft: Remove dead code
2024-05-02 1:21 ` Lynne
@ 2024-05-02 1:28 ` Michael Niedermayer
0 siblings, 0 replies; 24+ messages in thread
From: Michael Niedermayer @ 2024-05-02 1:28 UTC (permalink / raw)
To: FFmpeg development discussions and patches
[-- Attachment #1.1: Type: text/plain, Size: 1461 bytes --]
On Thu, May 02, 2024 at 03:21:11AM +0200, Lynne wrote:
> May 2, 2024, 02:42 by michael@niedermayer.cc:
>
> > Fixes: CID1543204 Logically dead code
> >
> > Sponsored-by: Sovereign Tech Fund
> > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> > ---
> > libavcodec/avfft.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/libavcodec/avfft.c b/libavcodec/avfft.c
> > index f6787937f67..0f43f30b776 100644
> > --- a/libavcodec/avfft.c
> > +++ b/libavcodec/avfft.c
> > @@ -158,7 +158,7 @@ RDFTContext *av_rdft_init(int nbits, enum RDFTransformType trans)
> > return NULL;
> > }
> >
> > - s->stride = (trans == DFT_C2R) ? sizeof(AVComplexFloat) : sizeof(float);
> > + s->stride = sizeof(float);
> > s->len = 1 << nbits;
> > s->inv = trans == IDFT_C2R;
> >
>
> That's not right.
> While it's true that currently the stride parameter in av_tx_fn
> is unused for RDFTs, that may not always be the case, and the
> documentation requires that the stride is valid and set to the
> value that the current implementation assumes, so that nothing
> breaks once that is implemented.
i kind of had the same feeling but wanted to post it anyway.
so patch withdrawn, ill mark the issue as "intentional" in coverity
thx
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
Never trust a computer, one day, it may think you are the virus. -- Compn
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]
[-- Attachment #2: Type: text/plain, Size: 251 bytes --]
_______________________________________________
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".
^ permalink raw reply [flat|nested] 24+ messages in thread
* [FFmpeg-devel] [PATCH 4/7] avcodec/avs2_parser: Assert init_get_bits8() success with const size 15
2024-05-02 0:41 [FFmpeg-devel] [PATCH 1/7] avcodec/av1dec: bit_depth cannot be another values than 8, 10, 12 Michael Niedermayer
2024-05-02 0:41 ` [FFmpeg-devel] [PATCH 2/7] avcodec/av1dec: initialize ret in av1_receive_frame_internal() Michael Niedermayer
2024-05-02 0:41 ` [FFmpeg-devel] [PATCH 3/7] avcodec/avfft: Remove dead code Michael Niedermayer
@ 2024-05-02 0:41 ` Michael Niedermayer
2024-05-02 8:25 ` Andreas Rheinhardt
2024-05-02 0:41 ` [FFmpeg-devel] [PATCH 5/7] avcodec/avs3_parser: Check the return value of init_get_bits8() Michael Niedermayer
` (3 subsequent siblings)
6 siblings, 1 reply; 24+ messages in thread
From: Michael Niedermayer @ 2024-05-02 0:41 UTC (permalink / raw)
To: FFmpeg development discussions and patches
Fixes: CID1506708 Unchecked return value
Sponsored-by: Sovereign Tech Fund
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
---
libavcodec/avs2_parser.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/libavcodec/avs2_parser.c b/libavcodec/avs2_parser.c
index 200134f91db..8d4bc3cee0d 100644
--- a/libavcodec/avs2_parser.c
+++ b/libavcodec/avs2_parser.c
@@ -72,13 +72,15 @@ static void parse_avs2_seq_header(AVCodecParserContext *s, const uint8_t *buf,
unsigned aspect_ratio;
unsigned frame_rate_code;
int low_delay;
+ int ret;
// update buf_size_min if parse more deeper
const int buf_size_min = 15;
if (buf_size < buf_size_min)
return;
- init_get_bits8(&gb, buf, buf_size_min);
+ ret = init_get_bits8(&gb, buf, buf_size_min);
+ av_assert2(ret >= 0);
s->key_frame = 1;
s->pict_type = AV_PICTURE_TYPE_I;
--
2.43.2
_______________________________________________
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".
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [FFmpeg-devel] [PATCH 4/7] avcodec/avs2_parser: Assert init_get_bits8() success with const size 15
2024-05-02 0:41 ` [FFmpeg-devel] [PATCH 4/7] avcodec/avs2_parser: Assert init_get_bits8() success with const size 15 Michael Niedermayer
@ 2024-05-02 8:25 ` Andreas Rheinhardt
2024-05-02 21:32 ` Michael Niedermayer
0 siblings, 1 reply; 24+ messages in thread
From: Andreas Rheinhardt @ 2024-05-02 8:25 UTC (permalink / raw)
To: ffmpeg-devel
Michael Niedermayer:
> Fixes: CID1506708 Unchecked return value
>
> Sponsored-by: Sovereign Tech Fund
> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> ---
> libavcodec/avs2_parser.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/libavcodec/avs2_parser.c b/libavcodec/avs2_parser.c
> index 200134f91db..8d4bc3cee0d 100644
> --- a/libavcodec/avs2_parser.c
> +++ b/libavcodec/avs2_parser.c
> @@ -72,13 +72,15 @@ static void parse_avs2_seq_header(AVCodecParserContext *s, const uint8_t *buf,
> unsigned aspect_ratio;
> unsigned frame_rate_code;
> int low_delay;
> + int ret;
> // update buf_size_min if parse more deeper
> const int buf_size_min = 15;
>
> if (buf_size < buf_size_min)
> return;
>
> - init_get_bits8(&gb, buf, buf_size_min);
> + ret = init_get_bits8(&gb, buf, buf_size_min);
> + av_assert2(ret >= 0);
>
> s->key_frame = 1;
> s->pict_type = AV_PICTURE_TYPE_I;
Code like this may trigger set-but-unused variable warnings when
ASSERT_LEVEL is <= 1. Add av_unused to ret to fix this.
Apart from that, it should be av_assert1 (this is not hot).
- 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".
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [FFmpeg-devel] [PATCH 4/7] avcodec/avs2_parser: Assert init_get_bits8() success with const size 15
2024-05-02 8:25 ` Andreas Rheinhardt
@ 2024-05-02 21:32 ` Michael Niedermayer
0 siblings, 0 replies; 24+ messages in thread
From: Michael Niedermayer @ 2024-05-02 21:32 UTC (permalink / raw)
To: FFmpeg development discussions and patches
[-- Attachment #1.1: Type: text/plain, Size: 1542 bytes --]
On Thu, May 02, 2024 at 10:25:24AM +0200, Andreas Rheinhardt wrote:
> Michael Niedermayer:
> > Fixes: CID1506708 Unchecked return value
> >
> > Sponsored-by: Sovereign Tech Fund
> > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> > ---
> > libavcodec/avs2_parser.c | 4 +++-
> > 1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/libavcodec/avs2_parser.c b/libavcodec/avs2_parser.c
> > index 200134f91db..8d4bc3cee0d 100644
> > --- a/libavcodec/avs2_parser.c
> > +++ b/libavcodec/avs2_parser.c
> > @@ -72,13 +72,15 @@ static void parse_avs2_seq_header(AVCodecParserContext *s, const uint8_t *buf,
> > unsigned aspect_ratio;
> > unsigned frame_rate_code;
> > int low_delay;
> > + int ret;
> > // update buf_size_min if parse more deeper
> > const int buf_size_min = 15;
> >
> > if (buf_size < buf_size_min)
> > return;
> >
> > - init_get_bits8(&gb, buf, buf_size_min);
> > + ret = init_get_bits8(&gb, buf, buf_size_min);
> > + av_assert2(ret >= 0);
> >
> > s->key_frame = 1;
> > s->pict_type = AV_PICTURE_TYPE_I;
>
> Code like this may trigger set-but-unused variable warnings when
> ASSERT_LEVEL is <= 1. Add av_unused to ret to fix this.
> Apart from that, it should be av_assert1 (this is not hot).
will apply with these changes
thx
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
Observe your enemies, for they first find out your faults. -- Antisthenes
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]
[-- Attachment #2: Type: text/plain, Size: 251 bytes --]
_______________________________________________
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".
^ permalink raw reply [flat|nested] 24+ messages in thread
* [FFmpeg-devel] [PATCH 5/7] avcodec/avs3_parser: Check the return value of init_get_bits8()
2024-05-02 0:41 [FFmpeg-devel] [PATCH 1/7] avcodec/av1dec: bit_depth cannot be another values than 8, 10, 12 Michael Niedermayer
` (2 preceding siblings ...)
2024-05-02 0:41 ` [FFmpeg-devel] [PATCH 4/7] avcodec/avs2_parser: Assert init_get_bits8() success with const size 15 Michael Niedermayer
@ 2024-05-02 0:41 ` Michael Niedermayer
2024-05-02 8:28 ` Andreas Rheinhardt
2024-05-02 0:41 ` [FFmpeg-devel] [PATCH 6/7] avcodec/cbs_av1: Avoid shift overflow Michael Niedermayer
` (2 subsequent siblings)
6 siblings, 1 reply; 24+ messages in thread
From: Michael Niedermayer @ 2024-05-02 0:41 UTC (permalink / raw)
To: FFmpeg development discussions and patches
Fixes: CID1492867 Unchecked return value
Sponsored-by: Sovereign Tech Fund
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
---
libavcodec/avs3_parser.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/libavcodec/avs3_parser.c b/libavcodec/avs3_parser.c
index a819b5783d6..0f9076befe1 100644
--- a/libavcodec/avs3_parser.c
+++ b/libavcodec/avs3_parser.c
@@ -73,7 +73,9 @@ static void parse_avs3_nal_units(AVCodecParserContext *s, const uint8_t *buf,
GetBitContext gb;
int profile, ratecode, low_delay;
- init_get_bits8(&gb, buf + 4, buf_size - 4);
+ int ret = init_get_bits8(&gb, buf + 4, buf_size - 4);
+ if (ret < 0)
+ return;
s->key_frame = 1;
s->pict_type = AV_PICTURE_TYPE_I;
--
2.43.2
_______________________________________________
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".
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [FFmpeg-devel] [PATCH 5/7] avcodec/avs3_parser: Check the return value of init_get_bits8()
2024-05-02 0:41 ` [FFmpeg-devel] [PATCH 5/7] avcodec/avs3_parser: Check the return value of init_get_bits8() Michael Niedermayer
@ 2024-05-02 8:28 ` Andreas Rheinhardt
2024-05-02 21:53 ` Michael Niedermayer
0 siblings, 1 reply; 24+ messages in thread
From: Andreas Rheinhardt @ 2024-05-02 8:28 UTC (permalink / raw)
To: ffmpeg-devel
Michael Niedermayer:
> Fixes: CID1492867 Unchecked return value
>
> Sponsored-by: Sovereign Tech Fund
> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> ---
> libavcodec/avs3_parser.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/libavcodec/avs3_parser.c b/libavcodec/avs3_parser.c
> index a819b5783d6..0f9076befe1 100644
> --- a/libavcodec/avs3_parser.c
> +++ b/libavcodec/avs3_parser.c
> @@ -73,7 +73,9 @@ static void parse_avs3_nal_units(AVCodecParserContext *s, const uint8_t *buf,
> GetBitContext gb;
> int profile, ratecode, low_delay;
>
> - init_get_bits8(&gb, buf + 4, buf_size - 4);
> + int ret = init_get_bits8(&gb, buf + 4, buf_size - 4);
> + if (ret < 0)
> + return;
>
> s->key_frame = 1;
> s->pict_type = AV_PICTURE_TYPE_I;
This code only reads/skips a few bits here (at most 100 if I counted
correctly), so one could initialize the reader for a shorter length and
assert that the call succeeds.
- 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".
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [FFmpeg-devel] [PATCH 5/7] avcodec/avs3_parser: Check the return value of init_get_bits8()
2024-05-02 8:28 ` Andreas Rheinhardt
@ 2024-05-02 21:53 ` Michael Niedermayer
0 siblings, 0 replies; 24+ messages in thread
From: Michael Niedermayer @ 2024-05-02 21:53 UTC (permalink / raw)
To: FFmpeg development discussions and patches
[-- Attachment #1.1: Type: text/plain, Size: 1583 bytes --]
On Thu, May 02, 2024 at 10:28:36AM +0200, Andreas Rheinhardt wrote:
> Michael Niedermayer:
> > Fixes: CID1492867 Unchecked return value
> >
> > Sponsored-by: Sovereign Tech Fund
> > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> > ---
> > libavcodec/avs3_parser.c | 4 +++-
> > 1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/libavcodec/avs3_parser.c b/libavcodec/avs3_parser.c
> > index a819b5783d6..0f9076befe1 100644
> > --- a/libavcodec/avs3_parser.c
> > +++ b/libavcodec/avs3_parser.c
> > @@ -73,7 +73,9 @@ static void parse_avs3_nal_units(AVCodecParserContext *s, const uint8_t *buf,
> > GetBitContext gb;
> > int profile, ratecode, low_delay;
> >
> > - init_get_bits8(&gb, buf + 4, buf_size - 4);
> > + int ret = init_get_bits8(&gb, buf + 4, buf_size - 4);
> > + if (ret < 0)
> > + return;
> >
> > s->key_frame = 1;
> > s->pict_type = AV_PICTURE_TYPE_I;
>
> This code only reads/skips a few bits here (at most 100 if I counted
> correctly), so one could initialize the reader for a shorter length and
> assert that the call succeeds.
will apply:
- init_get_bits8(&gb, buf + 4, buf_size - 4);
+ av_unused int ret = init_get_bits(&gb, buf + 4, 100);
+ av_assert1(ret >= 0);
thx
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
The educated differ from the uneducated as much as the living from the
dead. -- Aristotle
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]
[-- Attachment #2: Type: text/plain, Size: 251 bytes --]
_______________________________________________
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".
^ permalink raw reply [flat|nested] 24+ messages in thread
* [FFmpeg-devel] [PATCH 6/7] avcodec/cbs_av1: Avoid shift overflow
2024-05-02 0:41 [FFmpeg-devel] [PATCH 1/7] avcodec/av1dec: bit_depth cannot be another values than 8, 10, 12 Michael Niedermayer
` (3 preceding siblings ...)
2024-05-02 0:41 ` [FFmpeg-devel] [PATCH 5/7] avcodec/avs3_parser: Check the return value of init_get_bits8() Michael Niedermayer
@ 2024-05-02 0:41 ` Michael Niedermayer
2024-05-09 1:33 ` Michael Niedermayer
2024-05-02 0:41 ` [FFmpeg-devel] [PATCH 7/7] avcodec/cbs_jpeg: Try to move the read entity to one side in a test Michael Niedermayer
2024-05-02 0:56 ` [FFmpeg-devel] [PATCH 1/7] avcodec/av1dec: bit_depth cannot be another values than 8, 10, 12 James Almer
6 siblings, 1 reply; 24+ messages in thread
From: Michael Niedermayer @ 2024-05-02 0:41 UTC (permalink / raw)
To: FFmpeg development discussions and patches
Fixes: CID1465488 Unintentional integer overflow
Sponsored-by: Sovereign Tech Fund
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
---
libavcodec/cbs_av1.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libavcodec/cbs_av1.c b/libavcodec/cbs_av1.c
index 1d9ac5ab449..fb829960220 100644
--- a/libavcodec/cbs_av1.c
+++ b/libavcodec/cbs_av1.c
@@ -301,7 +301,7 @@ static int cbs_av1_write_increment(CodedBitstreamContext *ctx, PutBitContext *pb
return AVERROR(ENOSPC);
if (len > 0)
- put_bits(pbc, len, (1 << len) - 1 - (value != range_max));
+ put_bits(pbc, len, (1U << len) - 1 - (value != range_max));
CBS_TRACE_WRITE_END_NO_SUBSCRIPTS();
--
2.43.2
_______________________________________________
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".
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [FFmpeg-devel] [PATCH 6/7] avcodec/cbs_av1: Avoid shift overflow
2024-05-02 0:41 ` [FFmpeg-devel] [PATCH 6/7] avcodec/cbs_av1: Avoid shift overflow Michael Niedermayer
@ 2024-05-09 1:33 ` Michael Niedermayer
0 siblings, 0 replies; 24+ messages in thread
From: Michael Niedermayer @ 2024-05-09 1:33 UTC (permalink / raw)
To: FFmpeg development discussions and patches
[-- Attachment #1.1: Type: text/plain, Size: 483 bytes --]
On Thu, May 02, 2024 at 02:41:49AM +0200, Michael Niedermayer wrote:
> Fixes: CID1465488 Unintentional integer overflow
>
> Sponsored-by: Sovereign Tech Fund
> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> ---
> libavcodec/cbs_av1.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
will apply
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
What does censorship reveal? It reveals fear. -- Julian Assange
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]
[-- Attachment #2: Type: text/plain, Size: 251 bytes --]
_______________________________________________
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".
^ permalink raw reply [flat|nested] 24+ messages in thread
* [FFmpeg-devel] [PATCH 7/7] avcodec/cbs_jpeg: Try to move the read entity to one side in a test
2024-05-02 0:41 [FFmpeg-devel] [PATCH 1/7] avcodec/av1dec: bit_depth cannot be another values than 8, 10, 12 Michael Niedermayer
` (4 preceding siblings ...)
2024-05-02 0:41 ` [FFmpeg-devel] [PATCH 6/7] avcodec/cbs_av1: Avoid shift overflow Michael Niedermayer
@ 2024-05-02 0:41 ` Michael Niedermayer
2024-05-02 6:58 ` Andreas Rheinhardt
2024-05-02 0:56 ` [FFmpeg-devel] [PATCH 1/7] avcodec/av1dec: bit_depth cannot be another values than 8, 10, 12 James Almer
6 siblings, 1 reply; 24+ messages in thread
From: Michael Niedermayer @ 2024-05-02 0:41 UTC (permalink / raw)
To: FFmpeg development discussions and patches
Fixes: CID1439654 Untrusted pointer read
Sponsored-by: Sovereign Tech Fund
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
---
libavcodec/cbs_jpeg.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/libavcodec/cbs_jpeg.c b/libavcodec/cbs_jpeg.c
index b1b58dcd65e..406147c082c 100644
--- a/libavcodec/cbs_jpeg.c
+++ b/libavcodec/cbs_jpeg.c
@@ -146,13 +146,13 @@ static int cbs_jpeg_split_fragment(CodedBitstreamContext *ctx,
}
} else {
i = start;
- if (i + 2 > frag->data_size) {
+ if (i > frag->data_size - 2) {
av_log(ctx->log_ctx, AV_LOG_ERROR, "Invalid JPEG image: "
"truncated at %02x marker.\n", marker);
return AVERROR_INVALIDDATA;
}
length = AV_RB16(frag->data + i);
- if (i + length > frag->data_size) {
+ if (length > frag->data_size - i) {
av_log(ctx->log_ctx, AV_LOG_ERROR, "Invalid JPEG image: "
"truncated at %02x marker segment.\n", marker);
return AVERROR_INVALIDDATA;
--
2.43.2
_______________________________________________
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".
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [FFmpeg-devel] [PATCH 7/7] avcodec/cbs_jpeg: Try to move the read entity to one side in a test
2024-05-02 0:41 ` [FFmpeg-devel] [PATCH 7/7] avcodec/cbs_jpeg: Try to move the read entity to one side in a test Michael Niedermayer
@ 2024-05-02 6:58 ` Andreas Rheinhardt
2024-05-02 22:03 ` Michael Niedermayer
0 siblings, 1 reply; 24+ messages in thread
From: Andreas Rheinhardt @ 2024-05-02 6:58 UTC (permalink / raw)
To: ffmpeg-devel
Michael Niedermayer:
> Fixes: CID1439654 Untrusted pointer read
>
> Sponsored-by: Sovereign Tech Fund
> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> ---
> libavcodec/cbs_jpeg.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/libavcodec/cbs_jpeg.c b/libavcodec/cbs_jpeg.c
> index b1b58dcd65e..406147c082c 100644
> --- a/libavcodec/cbs_jpeg.c
> +++ b/libavcodec/cbs_jpeg.c
> @@ -146,13 +146,13 @@ static int cbs_jpeg_split_fragment(CodedBitstreamContext *ctx,
> }
> } else {
> i = start;
> - if (i + 2 > frag->data_size) {
> + if (i > frag->data_size - 2) {
> av_log(ctx->log_ctx, AV_LOG_ERROR, "Invalid JPEG image: "
> "truncated at %02x marker.\n", marker);
> return AVERROR_INVALIDDATA;
> }
> length = AV_RB16(frag->data + i);
> - if (i + length > frag->data_size) {
> + if (length > frag->data_size - i) {
> av_log(ctx->log_ctx, AV_LOG_ERROR, "Invalid JPEG image: "
> "truncated at %02x marker segment.\n", marker);
> return AVERROR_INVALIDDATA;
You should always mention when you are not fixing bugs in our code, but
rather intend to apply workaround for coverity crazyness (i.e. the
requirement that reading values in non-native endianness needs to be
sanitized).
- 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".
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [FFmpeg-devel] [PATCH 7/7] avcodec/cbs_jpeg: Try to move the read entity to one side in a test
2024-05-02 6:58 ` Andreas Rheinhardt
@ 2024-05-02 22:03 ` Michael Niedermayer
2024-07-02 18:29 ` Michael Niedermayer
0 siblings, 1 reply; 24+ messages in thread
From: Michael Niedermayer @ 2024-05-02 22:03 UTC (permalink / raw)
To: FFmpeg development discussions and patches
[-- Attachment #1.1: Type: text/plain, Size: 2119 bytes --]
On Thu, May 02, 2024 at 08:58:17AM +0200, Andreas Rheinhardt wrote:
> Michael Niedermayer:
> > Fixes: CID1439654 Untrusted pointer read
> >
> > Sponsored-by: Sovereign Tech Fund
> > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> > ---
> > libavcodec/cbs_jpeg.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/libavcodec/cbs_jpeg.c b/libavcodec/cbs_jpeg.c
> > index b1b58dcd65e..406147c082c 100644
> > --- a/libavcodec/cbs_jpeg.c
> > +++ b/libavcodec/cbs_jpeg.c
> > @@ -146,13 +146,13 @@ static int cbs_jpeg_split_fragment(CodedBitstreamContext *ctx,
> > }
> > } else {
> > i = start;
> > - if (i + 2 > frag->data_size) {
> > + if (i > frag->data_size - 2) {
> > av_log(ctx->log_ctx, AV_LOG_ERROR, "Invalid JPEG image: "
> > "truncated at %02x marker.\n", marker);
> > return AVERROR_INVALIDDATA;
> > }
> > length = AV_RB16(frag->data + i);
> > - if (i + length > frag->data_size) {
> > + if (length > frag->data_size - i) {
> > av_log(ctx->log_ctx, AV_LOG_ERROR, "Invalid JPEG image: "
> > "truncated at %02x marker segment.\n", marker);
> > return AVERROR_INVALIDDATA;
>
> You should always mention when you are not fixing bugs in our code, but
> rather intend to apply workaround for coverity crazyness (i.e. the
> requirement that reading values in non-native endianness needs to be
> sanitized).
writing code like
if (i + length > frag->data_size)
is IMHO not proper and that has nothing to do with coverity
the reason why this is not proper is that i + length could in
principle overflow. It depends on teh whole loop and calling code
if that is possible or not.
to check length, length should be alone on one side of the check
thx
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
It is what and why we do it that matters, not just one of them.
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]
[-- Attachment #2: Type: text/plain, Size: 251 bytes --]
_______________________________________________
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".
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [FFmpeg-devel] [PATCH 7/7] avcodec/cbs_jpeg: Try to move the read entity to one side in a test
2024-05-02 22:03 ` Michael Niedermayer
@ 2024-07-02 18:29 ` Michael Niedermayer
0 siblings, 0 replies; 24+ messages in thread
From: Michael Niedermayer @ 2024-07-02 18:29 UTC (permalink / raw)
To: FFmpeg development discussions and patches
[-- Attachment #1.1: Type: text/plain, Size: 2595 bytes --]
On Fri, May 03, 2024 at 12:03:57AM +0200, Michael Niedermayer wrote:
> On Thu, May 02, 2024 at 08:58:17AM +0200, Andreas Rheinhardt wrote:
> > Michael Niedermayer:
> > > Fixes: CID1439654 Untrusted pointer read
> > >
> > > Sponsored-by: Sovereign Tech Fund
> > > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> > > ---
> > > libavcodec/cbs_jpeg.c | 4 ++--
> > > 1 file changed, 2 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/libavcodec/cbs_jpeg.c b/libavcodec/cbs_jpeg.c
> > > index b1b58dcd65e..406147c082c 100644
> > > --- a/libavcodec/cbs_jpeg.c
> > > +++ b/libavcodec/cbs_jpeg.c
> > > @@ -146,13 +146,13 @@ static int cbs_jpeg_split_fragment(CodedBitstreamContext *ctx,
> > > }
> > > } else {
> > > i = start;
> > > - if (i + 2 > frag->data_size) {
> > > + if (i > frag->data_size - 2) {
> > > av_log(ctx->log_ctx, AV_LOG_ERROR, "Invalid JPEG image: "
> > > "truncated at %02x marker.\n", marker);
> > > return AVERROR_INVALIDDATA;
> > > }
> > > length = AV_RB16(frag->data + i);
> > > - if (i + length > frag->data_size) {
> > > + if (length > frag->data_size - i) {
> > > av_log(ctx->log_ctx, AV_LOG_ERROR, "Invalid JPEG image: "
> > > "truncated at %02x marker segment.\n", marker);
> > > return AVERROR_INVALIDDATA;
> >
> > You should always mention when you are not fixing bugs in our code, but
> > rather intend to apply workaround for coverity crazyness (i.e. the
> > requirement that reading values in non-native endianness needs to be
> > sanitized).
>
> writing code like
> if (i + length > frag->data_size)
>
> is IMHO not proper and that has nothing to do with coverity
> the reason why this is not proper is that i + length could in
> principle overflow. It depends on teh whole loop and calling code
> if that is possible or not.
>
> to check length, length should be alone on one side of the check
I will add this to the commit message:
" The checked entity should be alone on one side of the check, this avoids
complex considerations of overflows.
This fixes a issue of bad style in our code and a coverity issue.
"
thx
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
Complexity theory is the science of finding the exact solution to an
approximation. Benchmarking OTOH is finding an approximation of the exact
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]
[-- Attachment #2: Type: text/plain, Size: 251 bytes --]
_______________________________________________
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".
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [FFmpeg-devel] [PATCH 1/7] avcodec/av1dec: bit_depth cannot be another values than 8, 10, 12
2024-05-02 0:41 [FFmpeg-devel] [PATCH 1/7] avcodec/av1dec: bit_depth cannot be another values than 8, 10, 12 Michael Niedermayer
` (5 preceding siblings ...)
2024-05-02 0:41 ` [FFmpeg-devel] [PATCH 7/7] avcodec/cbs_jpeg: Try to move the read entity to one side in a test Michael Niedermayer
@ 2024-05-02 0:56 ` James Almer
2024-05-02 22:12 ` Michael Niedermayer
6 siblings, 1 reply; 24+ messages in thread
From: James Almer @ 2024-05-02 0:56 UTC (permalink / raw)
To: ffmpeg-devel
On 5/1/2024 9:41 PM, Michael Niedermayer wrote:
> Fixes: CID1544265 Logically dead code
>
> Sponsored-by: Sovereign Tech Fund
> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> ---
> libavcodec/av1dec.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/libavcodec/av1dec.c b/libavcodec/av1dec.c
> index 79a30a114da..4f9222cca27 100644
> --- a/libavcodec/av1dec.c
> +++ b/libavcodec/av1dec.c
> @@ -493,7 +493,7 @@ static enum AVPixelFormat get_sw_pixel_format(void *logctx,
> else if (bit_depth == 12)
> pix_fmt = AV_PIX_FMT_YUV444P12;
> else
> - av_log(logctx, AV_LOG_WARNING, "Unknown AV1 pixel format.\n");
> + av_assert0(0);
> } else if (seq->color_config.subsampling_x == 1 &&
> seq->color_config.subsampling_y == 0) {
> if (bit_depth == 8)
> @@ -503,7 +503,7 @@ static enum AVPixelFormat get_sw_pixel_format(void *logctx,
> else if (bit_depth == 12)
> pix_fmt = AV_PIX_FMT_YUV422P12;
> else
> - av_log(logctx, AV_LOG_WARNING, "Unknown AV1 pixel format.\n");
> + av_assert0(0);
> } else if (seq->color_config.subsampling_x == 1 &&
> seq->color_config.subsampling_y == 1) {
> if (bit_depth == 8)
> @@ -513,7 +513,7 @@ static enum AVPixelFormat get_sw_pixel_format(void *logctx,
> else if (bit_depth == 12)
> pix_fmt = AV_PIX_FMT_YUV420P12;
> else
> - av_log(logctx, AV_LOG_WARNING, "Unknown AV1 pixel format.\n");
> + av_assert0(0);
> }
> } else {
> if (bit_depth == 8)
> @@ -523,7 +523,7 @@ static enum AVPixelFormat get_sw_pixel_format(void *logctx,
> else if (bit_depth == 12)
> pix_fmt = AV_PIX_FMT_GRAY12;
> else
> - av_log(logctx, AV_LOG_WARNING, "Unknown AV1 pixel format.\n");
> + av_assert0(0);
> }
>
> return pix_fmt;
LGTM.
Can you change bit_depth into an int while at it? no reason for it to be
uint8_t as a scalar.
_______________________________________________
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".
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [FFmpeg-devel] [PATCH 1/7] avcodec/av1dec: bit_depth cannot be another values than 8, 10, 12
2024-05-02 0:56 ` [FFmpeg-devel] [PATCH 1/7] avcodec/av1dec: bit_depth cannot be another values than 8, 10, 12 James Almer
@ 2024-05-02 22:12 ` Michael Niedermayer
0 siblings, 0 replies; 24+ messages in thread
From: Michael Niedermayer @ 2024-05-02 22:12 UTC (permalink / raw)
To: FFmpeg development discussions and patches
[-- Attachment #1.1: Type: text/plain, Size: 2720 bytes --]
On Wed, May 01, 2024 at 09:56:53PM -0300, James Almer wrote:
> On 5/1/2024 9:41 PM, Michael Niedermayer wrote:
> > Fixes: CID1544265 Logically dead code
> >
> > Sponsored-by: Sovereign Tech Fund
> > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> > ---
> > libavcodec/av1dec.c | 8 ++++----
> > 1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/libavcodec/av1dec.c b/libavcodec/av1dec.c
> > index 79a30a114da..4f9222cca27 100644
> > --- a/libavcodec/av1dec.c
> > +++ b/libavcodec/av1dec.c
> > @@ -493,7 +493,7 @@ static enum AVPixelFormat get_sw_pixel_format(void *logctx,
> > else if (bit_depth == 12)
> > pix_fmt = AV_PIX_FMT_YUV444P12;
> > else
> > - av_log(logctx, AV_LOG_WARNING, "Unknown AV1 pixel format.\n");
> > + av_assert0(0);
> > } else if (seq->color_config.subsampling_x == 1 &&
> > seq->color_config.subsampling_y == 0) {
> > if (bit_depth == 8)
> > @@ -503,7 +503,7 @@ static enum AVPixelFormat get_sw_pixel_format(void *logctx,
> > else if (bit_depth == 12)
> > pix_fmt = AV_PIX_FMT_YUV422P12;
> > else
> > - av_log(logctx, AV_LOG_WARNING, "Unknown AV1 pixel format.\n");
> > + av_assert0(0);
> > } else if (seq->color_config.subsampling_x == 1 &&
> > seq->color_config.subsampling_y == 1) {
> > if (bit_depth == 8)
> > @@ -513,7 +513,7 @@ static enum AVPixelFormat get_sw_pixel_format(void *logctx,
> > else if (bit_depth == 12)
> > pix_fmt = AV_PIX_FMT_YUV420P12;
> > else
> > - av_log(logctx, AV_LOG_WARNING, "Unknown AV1 pixel format.\n");
> > + av_assert0(0);
> > }
> > } else {
> > if (bit_depth == 8)
> > @@ -523,7 +523,7 @@ static enum AVPixelFormat get_sw_pixel_format(void *logctx,
> > else if (bit_depth == 12)
> > pix_fmt = AV_PIX_FMT_GRAY12;
> > else
> > - av_log(logctx, AV_LOG_WARNING, "Unknown AV1 pixel format.\n");
> > + av_assert0(0);
> > }
> > return pix_fmt;
>
> LGTM.
will apply
>
> Can you change bit_depth into an int while at it? no reason for it to be
> uint8_t as a scalar.
ok, but its unrelated
thx
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
The bravest are surely those who have the clearest vision
of what is before them, glory and danger alike, and yet
notwithstanding go out to meet it. -- Thucydides
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]
[-- Attachment #2: Type: text/plain, Size: 251 bytes --]
_______________________________________________
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".
^ permalink raw reply [flat|nested] 24+ messages in thread