[FFmpeg-devel] [PATCH 1/2] avfilter/signature_lookup: dont leave uncleared pointers in sll_free()

[FFmpeg-devel] [PATCH 1/2] avfilter/signature_lookup: dont leave uncleared pointers in sll_free()

[Michael Niedermayer]

Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
---
 libavfilter/signature_lookup.c | 21 ++++++++++-----------
 1 file changed, 10 insertions(+), 11 deletions(-)

diff --git a/libavfilter/signature_lookup.c b/libavfilter/signature_lookup.c
index 86dd0c66754..52a97e1bc7e 100644
--- a/libavfilter/signature_lookup.c
+++ b/libavfilter/signature_lookup.c
@@ -37,6 +37,15 @@
 #define STATUS_END_REACHED 1
 #define STATUS_BEGIN_REACHED 2
 
+static void sll_free(MatchingInfo **sll)
+{
+    while (*sll) {
+        MatchingInfo *tmp = *sll;
+        *sll = (*sll)->next;
+        av_free(tmp);
+    }
+}
+
 static void fill_l1distlut(uint8_t lut[])
 {
     int i, j, tmp_i, tmp_j,count;
@@ -520,16 +529,6 @@ static MatchingInfo evaluate_parameters(AVFilterContext *ctx, SignatureContext *
     return bestmatch;
 }
 
-static void sll_free(MatchingInfo *sll)
-{
-    void *tmp;
-    while (sll) {
-        tmp = sll;
-        sll = sll->next;
-        av_freep(&tmp);
-    }
-}
-
 static MatchingInfo lookup_signatures(AVFilterContext *ctx, SignatureContext *sc, StreamContext *first, StreamContext *second, int mode)
 {
     CoarseSignature *cs, *cs2;
@@ -572,7 +571,7 @@ static MatchingInfo lookup_signatures(AVFilterContext *ctx, SignatureContext *sc
                    "ratio %f, offset %d, score %d, %d frames matching\n",
                    bestmatch.first->index, bestmatch.second->index,
                    bestmatch.framerateratio, bestmatch.offset, bestmatch.score, bestmatch.matchframes);
-            sll_free(infos);
+            sll_free(&infos);
         }
     } while (find_next_coarsecandidate(sc, second->coarsesiglist, &cs, &cs2, 0) && !bestmatch.whole);
     return bestmatch;
-- 
2.17.1

_______________________________________________
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] avfilter/signature_lookup: Do not dereference NULL pointers after malloc failure

[Michael Niedermayer]

Fixes: CID 1403229 Dereference after null check

Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
---
 libavfilter/signature_lookup.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/libavfilter/signature_lookup.c b/libavfilter/signature_lookup.c
index 52a97e1bc7e..0c724456e24 100644
--- a/libavfilter/signature_lookup.c
+++ b/libavfilter/signature_lookup.c
@@ -298,6 +298,11 @@ static MatchingInfo* get_matching_parameters(AVFilterContext *ctx, SignatureCont
                         if (!c->next)
                             av_log(ctx, AV_LOG_FATAL, "Could not allocate memory");
                         c = c->next;
+
+                    }
+                    if (!c) {
+                        sll_free(&cands);
+                        goto error;
                     }
                     c->framerateratio = (i+1.0) / 30;
                     c->score = hspace[i][j].score;
@@ -314,6 +319,7 @@ static MatchingInfo* get_matching_parameters(AVFilterContext *ctx, SignatureCont
             }
         }
     }
+    error:
     for (i = 0; i < MAX_FRAMERATE; i++) {
         av_freep(&hspace[i]);
     }
-- 
2.17.1

_______________________________________________
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] avfilter/signature_lookup: Do not dereference NULL pointers after malloc failure

[Michael Niedermayer]

[-- Attachment #1.1: Type: text/plain, Size: 524 bytes --]

On Mon, Feb 05, 2024 at 12:44:59PM +0100, Michael Niedermayer wrote:
> Fixes: CID 1403229 Dereference after null check
> 
> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> ---
>  libavfilter/signature_lookup.c | 6 ++++++
>  1 file changed, 6 insertions(+)

will apply

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Dictatorship naturally arises out of democracy, and the most aggravated
form of tyranny and slavery out of the most extreme liberty. -- Plato

[-- 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".

Re: [FFmpeg-devel] [PATCH 1/2] avfilter/signature_lookup: dont leave uncleared pointers in sll_free()

[Andreas Rheinhardt]

Michael Niedermayer:
> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> ---
>  libavfilter/signature_lookup.c | 21 ++++++++++-----------
>  1 file changed, 10 insertions(+), 11 deletions(-)
> 
> diff --git a/libavfilter/signature_lookup.c b/libavfilter/signature_lookup.c
> index 86dd0c66754..52a97e1bc7e 100644
> --- a/libavfilter/signature_lookup.c
> +++ b/libavfilter/signature_lookup.c
> @@ -37,6 +37,15 @@
>  #define STATUS_END_REACHED 1
>  #define STATUS_BEGIN_REACHED 2
>  
> +static void sll_free(MatchingInfo **sll)
> +{
> +    while (*sll) {
> +        MatchingInfo *tmp = *sll;
> +        *sll = (*sll)->next;
> +        av_free(tmp);
> +    }

This does not clear the pointers at all. This does (and avoids
indirections).

static void sll_free(MatchingInfo **sllp)
{
    MatchingInfo *sll = *sllp;

    *sllp = NULL;
    while (sll) {
        MatchingInfo *tmp = sll;
        sll = sll->next;
        av_free(tmp);
    }
}

> +}
> +
>  static void fill_l1distlut(uint8_t lut[])
>  {
>      int i, j, tmp_i, tmp_j,count;
> @@ -520,16 +529,6 @@ static MatchingInfo evaluate_parameters(AVFilterContext *ctx, SignatureContext *
>      return bestmatch;
>  }
>  
> -static void sll_free(MatchingInfo *sll)
> -{
> -    void *tmp;
> -    while (sll) {
> -        tmp = sll;
> -        sll = sll->next;
> -        av_freep(&tmp);
> -    }
> -}
> -
>  static MatchingInfo lookup_signatures(AVFilterContext *ctx, SignatureContext *sc, StreamContext *first, StreamContext *second, int mode)
>  {
>      CoarseSignature *cs, *cs2;
> @@ -572,7 +571,7 @@ static MatchingInfo lookup_signatures(AVFilterContext *ctx, SignatureContext *sc
>                     "ratio %f, offset %d, score %d, %d frames matching\n",
>                     bestmatch.first->index, bestmatch.second->index,
>                     bestmatch.framerateratio, bestmatch.offset, bestmatch.score, bestmatch.matchframes);
> -            sll_free(infos);
> +            sll_free(&infos);
>          }
>      } while (find_next_coarsecandidate(sc, second->coarsesiglist, &cs, &cs2, 0) && !bestmatch.whole);
>      return bestmatch;

_______________________________________________
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] avfilter/signature_lookup: dont leave uncleared pointers in sll_free()

[Michael Niedermayer]

[-- Attachment #1.1: Type: text/plain, Size: 3495 bytes --]

On Mon, Feb 05, 2024 at 12:51:57PM +0100, Andreas Rheinhardt wrote:
> Michael Niedermayer:
> > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> > ---
> >  libavfilter/signature_lookup.c | 21 ++++++++++-----------
> >  1 file changed, 10 insertions(+), 11 deletions(-)
> > 
> > diff --git a/libavfilter/signature_lookup.c b/libavfilter/signature_lookup.c
> > index 86dd0c66754..52a97e1bc7e 100644
> > --- a/libavfilter/signature_lookup.c
> > +++ b/libavfilter/signature_lookup.c
> > @@ -37,6 +37,15 @@
> >  #define STATUS_END_REACHED 1
> >  #define STATUS_BEGIN_REACHED 2
> >  
> > +static void sll_free(MatchingInfo **sll)
> > +{
> > +    while (*sll) {
> > +        MatchingInfo *tmp = *sll;
> > +        *sll = (*sll)->next;
> > +        av_free(tmp);
> > +    }
> 
> This does not clear the pointers at all. This does (and avoids
> indirections).
> 
> static void sll_free(MatchingInfo **sllp)
> {
>     MatchingInfo *sll = *sllp;
> 
>     *sllp = NULL;
>     while (sll) {
>         MatchingInfo *tmp = sll;
>         sll = sll->next;
>         av_free(tmp);
>     }
> }

I tried it with code below, but your code is not different from mine in behavior just more complex

output:
(nil) 0x560e8daad2c0 (nil)
vs.
(nil) 0x557ae6e472c0 (nil)

sll_free_n2() is simpler and will clear all, the reason i did not
propose it, is its recursive and can hit stack space limits in principle
sll_free_n3() and sll_free_n4() are other options that will clear all
but maybe every choice contains bugs, i didnt really test them with more than one testcase

-----------

#include <stdio.h>
#include <stdlib.h>
#include <string.h>

#define FFSWAP(type,a,b) do{type SWAP_tmp= b; b= a; a= SWAP_tmp;}while(0)

static void av_free(void *ptr)
{
    free(ptr);
}

static void av_freep(void *arg)
{
    void *val;

    memcpy(&val, arg, sizeof(val));
    memcpy(arg, &(void *){ NULL }, sizeof(val));
    av_free(val);
}


typedef struct MatchingInfo {
    struct MatchingInfo *next;
} MatchingInfo;


static void sll_free_n(MatchingInfo **sll)
{
    while (*sll) {
        MatchingInfo *tmp = *sll;
        *sll = (*sll)->next;
        av_free(tmp);
    }
}

static void sll_free_n2(MatchingInfo **sll)
{
    if (*sll)
        sll_free_n(&(*sll)->next);
    av_freep(sll);
}

static void sll_free_n3(MatchingInfo **sll)
{
    while (*sll) {
        MatchingInfo *tmp = *sll;
        *sll = tmp->next;
        tmp->next = NULL;
        av_free(tmp);
    }
}

static void sll_free_n4(MatchingInfo **sll)
{
    MatchingInfo *tmp = NULL;
    while (*sll) {
        FFSWAP(MatchingInfo *, tmp, (*sll)->next);
        av_freep(sll);
        FFSWAP(MatchingInfo *, tmp, *sll);
    }
}

static void sll_free_r(MatchingInfo **sllp)
{
    MatchingInfo *sll = *sllp;

    *sllp = NULL;
    while (sll) {
        MatchingInfo *tmp = sll;
        sll = sll->next;
        av_free(tmp);
    }
}

main() {
    MatchingInfo *mi, *m1, *m2;

    m1 = mi = malloc(sizeof(MatchingInfo));
    m2 = mi->next = malloc(sizeof(MatchingInfo));
    m2->next= NULL;

    sll_free_r(&mi);

    printf("%p %p %p\n", mi, m1->next, m2->next);

}

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

In fact, the RIAA has been known to suggest that students drop out
of college or go to community college in order to be able to afford
settlements. -- The RIAA

[-- 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".

Re: [FFmpeg-devel] [PATCH 1/2] avfilter/signature_lookup: dont leave uncleared pointers in sll_free()

[Andreas Rheinhardt]

Michael Niedermayer:
> On Mon, Feb 05, 2024 at 12:51:57PM +0100, Andreas Rheinhardt wrote:
>> Michael Niedermayer:
>>> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
>>> ---
>>>  libavfilter/signature_lookup.c | 21 ++++++++++-----------
>>>  1 file changed, 10 insertions(+), 11 deletions(-)
>>>
>>> diff --git a/libavfilter/signature_lookup.c b/libavfilter/signature_lookup.c
>>> index 86dd0c66754..52a97e1bc7e 100644
>>> --- a/libavfilter/signature_lookup.c
>>> +++ b/libavfilter/signature_lookup.c
>>> @@ -37,6 +37,15 @@
>>>  #define STATUS_END_REACHED 1
>>>  #define STATUS_BEGIN_REACHED 2
>>>  
>>> +static void sll_free(MatchingInfo **sll)
>>> +{
>>> +    while (*sll) {
>>> +        MatchingInfo *tmp = *sll;
>>> +        *sll = (*sll)->next;
>>> +        av_free(tmp);
>>> +    }
>>
>> This does not clear the pointers at all. This does (and avoids
>> indirections).
>>
>> static void sll_free(MatchingInfo **sllp)
>> {
>>     MatchingInfo *sll = *sllp;
>>
>>     *sllp = NULL;
>>     while (sll) {
>>         MatchingInfo *tmp = sll;
>>         sll = sll->next;
>>         av_free(tmp);
>>     }
>> }
> 
> I tried it with code below, but your code is not different from mine in behavior just more complex
> 

Your code indeed resets the pointer; it overwrites the pointer once per
loop iteration and so sets it to NULL in the last iteration. I somehow
overlooked that.
I actually consider your code more complex (my code resets the original
pointer and directly traverses the list, your code does the same, but in
between it overwrites the original pointer to store the next pointer
instead of using a simple stack variable for this purpose).
Apply as you wish.

> output:
> (nil) 0x560e8daad2c0 (nil)
> vs.
> (nil) 0x557ae6e472c0 (nil)
> 
> sll_free_n2() is simpler and will clear all, the reason i did not
> propose it, is its recursive and can hit stack space limits in principle
> sll_free_n3() and sll_free_n4() are other options that will clear all
> but maybe every choice contains bugs, i didnt really test them with more than one testcase

sll_free_n2() is not recursive.

> 
> -----------
> 
> #include <stdio.h>
> #include <stdlib.h>
> #include <string.h>
> 
> #define FFSWAP(type,a,b) do{type SWAP_tmp= b; b= a; a= SWAP_tmp;}while(0)
> 
> static void av_free(void *ptr)
> {
>     free(ptr);
> }
> 
> static void av_freep(void *arg)
> {
>     void *val;
> 
>     memcpy(&val, arg, sizeof(val));
>     memcpy(arg, &(void *){ NULL }, sizeof(val));
>     av_free(val);
> }
> 
> 
> typedef struct MatchingInfo {
>     struct MatchingInfo *next;
> } MatchingInfo;
> 
> 
> static void sll_free_n(MatchingInfo **sll)
> {
>     while (*sll) {
>         MatchingInfo *tmp = *sll;
>         *sll = (*sll)->next;
>         av_free(tmp);
>     }
> }
> 
> static void sll_free_n2(MatchingInfo **sll)
> {
>     if (*sll)
>         sll_free_n(&(*sll)->next);
>     av_freep(sll);
> }
> 
> static void sll_free_n3(MatchingInfo **sll)
> {
>     while (*sll) {
>         MatchingInfo *tmp = *sll;
>         *sll = tmp->next;
>         tmp->next = NULL;
>         av_free(tmp);
>     }
> }
> 
> static void sll_free_n4(MatchingInfo **sll)
> {
>     MatchingInfo *tmp = NULL;
>     while (*sll) {
>         FFSWAP(MatchingInfo *, tmp, (*sll)->next);
>         av_freep(sll);
>         FFSWAP(MatchingInfo *, tmp, *sll);
>     }
> }
> 
> static void sll_free_r(MatchingInfo **sllp)
> {
>     MatchingInfo *sll = *sllp;
> 
>     *sllp = NULL;
>     while (sll) {
>         MatchingInfo *tmp = sll;
>         sll = sll->next;
>         av_free(tmp);
>     }
> }
> 
> main() {
>     MatchingInfo *mi, *m1, *m2;
> 
>     m1 = mi = malloc(sizeof(MatchingInfo));
>     m2 = mi->next = malloc(sizeof(MatchingInfo));
>     m2->next= NULL;
> 
>     sll_free_r(&mi);
> 
>     printf("%p %p %p\n", mi, m1->next, m2->next);
> 
> }
> 

_______________________________________________
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] avfilter/signature_lookup: dont leave uncleared pointers in sll_free()

[Michael Niedermayer]

[-- Attachment #1.1: Type: text/plain, Size: 2188 bytes --]

On Tue, Feb 06, 2024 at 11:36:13AM +0100, Andreas Rheinhardt wrote:
> Michael Niedermayer:
> > On Mon, Feb 05, 2024 at 12:51:57PM +0100, Andreas Rheinhardt wrote:
> >> Michael Niedermayer:
> >>> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> >>> ---
> >>>  libavfilter/signature_lookup.c | 21 ++++++++++-----------
> >>>  1 file changed, 10 insertions(+), 11 deletions(-)
> >>>
> >>> diff --git a/libavfilter/signature_lookup.c b/libavfilter/signature_lookup.c
> >>> index 86dd0c66754..52a97e1bc7e 100644
> >>> --- a/libavfilter/signature_lookup.c
> >>> +++ b/libavfilter/signature_lookup.c
> >>> @@ -37,6 +37,15 @@
> >>>  #define STATUS_END_REACHED 1
> >>>  #define STATUS_BEGIN_REACHED 2
> >>>  
> >>> +static void sll_free(MatchingInfo **sll)
> >>> +{
> >>> +    while (*sll) {
> >>> +        MatchingInfo *tmp = *sll;
> >>> +        *sll = (*sll)->next;
> >>> +        av_free(tmp);
> >>> +    }
> >>
> >> This does not clear the pointers at all. This does (and avoids
> >> indirections).
> >>
> >> static void sll_free(MatchingInfo **sllp)
> >> {
> >>     MatchingInfo *sll = *sllp;
> >>
> >>     *sllp = NULL;
> >>     while (sll) {
> >>         MatchingInfo *tmp = sll;
> >>         sll = sll->next;
> >>         av_free(tmp);
> >>     }
> >> }
> > 
> > I tried it with code below, but your code is not different from mine in behavior just more complex
> > 
> 
> Your code indeed resets the pointer; it overwrites the pointer once per
> loop iteration and so sets it to NULL in the last iteration. I somehow
> overlooked that.
> I actually consider your code more complex (my code resets the original
> pointer and directly traverses the list, your code does the same, but in
> between it overwrites the original pointer to store the next pointer
> instead of using a simple stack variable for this purpose).

> Apply as you wish.

ok


[...]
> sll_free_n2() is not recursive.

the function is cursed, noone can implement it without bugs

thx

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

He who knows, does not speak. He who speaks, does not know. -- Lao Tsu

[-- 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".

[FFmpeg-devel] [PATCH 1/2] avcodec/ac3enc_template: add fbw_channels assert

[Michael Niedermayer]

fbw_channels must be > 0 as teh code is only run if cpl_enabled is set and that requires mode >= AC3_CHMODE_STEREO

CID 718138 Uninitialized scalar variable
	assumes this assert to be false

Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
---
 libavcodec/ac3enc_template.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/libavcodec/ac3enc_template.c b/libavcodec/ac3enc_template.c
index ce9ef58a330..34d07cc9e5b 100644
--- a/libavcodec/ac3enc_template.c
+++ b/libavcodec/ac3enc_template.c
@@ -223,6 +223,8 @@ static void apply_channel_coupling(AC3EncodeContext *s)
         }
     }
 
+    av_assert1(s->fbw_channels > 0);
+
     /* calculate final coupling coordinates, taking into account reusing of
        coordinates in successive blocks */
     for (bnd = 0; bnd < s->num_cpl_bands; bnd++) {
-- 
2.17.1

_______________________________________________
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] avfilter/signature_lookup: Do not dereference NULL pointers after malloc failure

[Michael Niedermayer]

Fixes: CID 1403229 Dereference after null check

Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
---
 libavfilter/signature_lookup.c | 25 ++++++++++++++-----------
 1 file changed, 14 insertions(+), 11 deletions(-)

diff --git a/libavfilter/signature_lookup.c b/libavfilter/signature_lookup.c
index 86dd0c66754..6e45fde1b5a 100644
--- a/libavfilter/signature_lookup.c
+++ b/libavfilter/signature_lookup.c
@@ -37,6 +37,14 @@
 #define STATUS_END_REACHED 1
 #define STATUS_BEGIN_REACHED 2
 
+static void sll_free(MatchingInfo **sll)
+{
+    while (*sll) {
+        sll = &(*sll)->next;
+        av_freep(sll);
+    }
+}
+
 static void fill_l1distlut(uint8_t lut[])
 {
     int i, j, tmp_i, tmp_j,count;
@@ -290,6 +298,10 @@ static MatchingInfo* get_matching_parameters(AVFilterContext *ctx, SignatureCont
                             av_log(ctx, AV_LOG_FATAL, "Could not allocate memory");
                         c = c->next;
                     }
+                    if (!c) {
+                        sll_free(&cands);
+                        goto error;
+                    }
                     c->framerateratio = (i+1.0) / 30;
                     c->score = hspace[i][j].score;
                     c->offset = j-90;
@@ -305,6 +317,7 @@ static MatchingInfo* get_matching_parameters(AVFilterContext *ctx, SignatureCont
             }
         }
     }
+    error:
     for (i = 0; i < MAX_FRAMERATE; i++) {
         av_freep(&hspace[i]);
     }
@@ -520,16 +533,6 @@ static MatchingInfo evaluate_parameters(AVFilterContext *ctx, SignatureContext *
     return bestmatch;
 }
 
-static void sll_free(MatchingInfo *sll)
-{
-    void *tmp;
-    while (sll) {
-        tmp = sll;
-        sll = sll->next;
-        av_freep(&tmp);
-    }
-}
-
 static MatchingInfo lookup_signatures(AVFilterContext *ctx, SignatureContext *sc, StreamContext *first, StreamContext *second, int mode)
 {
     CoarseSignature *cs, *cs2;
@@ -572,7 +575,7 @@ static MatchingInfo lookup_signatures(AVFilterContext *ctx, SignatureContext *sc
                    "ratio %f, offset %d, score %d, %d frames matching\n",
                    bestmatch.first->index, bestmatch.second->index,
                    bestmatch.framerateratio, bestmatch.offset, bestmatch.score, bestmatch.matchframes);
-            sll_free(infos);
+            sll_free(&infos);
         }
     } while (find_next_coarsecandidate(sc, second->coarsesiglist, &cs, &cs2, 0) && !bestmatch.whole);
     return bestmatch;
-- 
2.17.1

_______________________________________________
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] avfilter/signature_lookup: Do not dereference NULL pointers after malloc failure

[Andreas Rheinhardt]

Michael Niedermayer:
> Fixes: CID 1403229 Dereference after null check
> 
> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> ---
>  libavfilter/signature_lookup.c | 25 ++++++++++++++-----------
>  1 file changed, 14 insertions(+), 11 deletions(-)
> 
> diff --git a/libavfilter/signature_lookup.c b/libavfilter/signature_lookup.c
> index 86dd0c66754..6e45fde1b5a 100644
> --- a/libavfilter/signature_lookup.c
> +++ b/libavfilter/signature_lookup.c
> @@ -37,6 +37,14 @@
>  #define STATUS_END_REACHED 1
>  #define STATUS_BEGIN_REACHED 2
>  
> +static void sll_free(MatchingInfo **sll)
> +{
> +    while (*sll) {
> +        sll = &(*sll)->next;
> +        av_freep(sll);
> +    }
> +}
> +

This will leak every element except the second (if existing) of the
linked list.

>  static void fill_l1distlut(uint8_t lut[])
>  {
>      int i, j, tmp_i, tmp_j,count;
> @@ -290,6 +298,10 @@ static MatchingInfo* get_matching_parameters(AVFilterContext *ctx, SignatureCont
>                              av_log(ctx, AV_LOG_FATAL, "Could not allocate memory");
>                          c = c->next;
>                      }
> +                    if (!c) {
> +                        sll_free(&cands);
> +                        goto error;
> +                    }
>                      c->framerateratio = (i+1.0) / 30;
>                      c->score = hspace[i][j].score;
>                      c->offset = j-90;
> @@ -305,6 +317,7 @@ static MatchingInfo* get_matching_parameters(AVFilterContext *ctx, SignatureCont
>              }
>          }
>      }
> +    error:
>      for (i = 0; i < MAX_FRAMERATE; i++) {
>          av_freep(&hspace[i]);
>      }
> @@ -520,16 +533,6 @@ static MatchingInfo evaluate_parameters(AVFilterContext *ctx, SignatureContext *
>      return bestmatch;
>  }
>  
> -static void sll_free(MatchingInfo *sll)
> -{
> -    void *tmp;
> -    while (sll) {
> -        tmp = sll;
> -        sll = sll->next;
> -        av_freep(&tmp);
> -    }
> -}
> -
>  static MatchingInfo lookup_signatures(AVFilterContext *ctx, SignatureContext *sc, StreamContext *first, StreamContext *second, int mode)
>  {
>      CoarseSignature *cs, *cs2;
> @@ -572,7 +575,7 @@ static MatchingInfo lookup_signatures(AVFilterContext *ctx, SignatureContext *sc
>                     "ratio %f, offset %d, score %d, %d frames matching\n",
>                     bestmatch.first->index, bestmatch.second->index,
>                     bestmatch.framerateratio, bestmatch.offset, bestmatch.score, bestmatch.matchframes);
> -            sll_free(infos);
> +            sll_free(&infos);
>          }
>      } while (find_next_coarsecandidate(sc, second->coarsesiglist, &cs, &cs2, 0) && !bestmatch.whole);
>      return bestmatch;

_______________________________________________
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] avfilter/signature_lookup: Do not dereference NULL pointers after malloc failure

[Michael Niedermayer]

[-- Attachment #1.1: Type: text/plain, Size: 1269 bytes --]

On Mon, Feb 05, 2024 at 11:35:54AM +0100, Andreas Rheinhardt wrote:
> Michael Niedermayer:
> > Fixes: CID 1403229 Dereference after null check
> > 
> > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> > ---
> >  libavfilter/signature_lookup.c | 25 ++++++++++++++-----------
> >  1 file changed, 14 insertions(+), 11 deletions(-)
> > 
> > diff --git a/libavfilter/signature_lookup.c b/libavfilter/signature_lookup.c
> > index 86dd0c66754..6e45fde1b5a 100644
> > --- a/libavfilter/signature_lookup.c
> > +++ b/libavfilter/signature_lookup.c
> > @@ -37,6 +37,14 @@
> >  #define STATUS_END_REACHED 1
> >  #define STATUS_BEGIN_REACHED 2
> >  
> > +static void sll_free(MatchingInfo **sll)
> > +{
> > +    while (*sll) {
> > +        sll = &(*sll)->next;
> > +        av_freep(sll);
> > +    }
> > +}
> > +
> 
> This will leak every element except the second (if existing) of the
> linked list.

ill post a better and split patch

thx

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

If you fake or manipulate statistics in a paper in physics you will never
get a job again.
If you fake or manipulate statistics in a paper in medicin you will get
a job for life at the pharma industry.

[-- 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".