[FFmpeg-devel] [PATCH] vf_pullup: simplify, fix double free error

Michael Niedermayer michaelni at gmx.at
Tue Mar 25 15:01:02 CET 2014


On Tue, Mar 25, 2014 at 01:59:58PM +0100, wm4 wrote:
> The memory allocation for f->diffs was freed multiple times in some
> corner cases. Simplify the code so that this doesn't happen. In fact,
> the body of free_field_queue restores the original MPlayer code.
> ---
> Sorry for not providing a reproducible test case, but for me this
> happened de to a very weird interaction with my application. I
> suspect this can happen when initializing the filter, but not
> filtering any frames. But I didn't attempt to confirm this.
> ---
>  libavfilter/vf_pullup.c | 21 ++++++++++-----------
>  1 file changed, 10 insertions(+), 11 deletions(-)
> 
> diff --git a/libavfilter/vf_pullup.c b/libavfilter/vf_pullup.c
> index 58d4d7a..9a3fc05 100644
> --- a/libavfilter/vf_pullup.c
> +++ b/libavfilter/vf_pullup.c
> @@ -126,20 +126,18 @@ static int alloc_metrics(PullupContext *s, PullupField *f)
>      return 0;
>  }
>  
> -static void free_field_queue(PullupField *head, PullupField **last)
> +static void free_field_queue(PullupField *head)
>  {
>      PullupField *f = head;
> -    while (f) {
> +    do {
> +        if (!f)
> +            break;
>          av_free(f->diffs);
>          av_free(f->combs);
>          av_free(f->vars);
> -        if (f == *last) {
> -            av_freep(last);
> -            break;
> -        }
>          f = f->next;
> -        av_freep(&f->prev);
> -    };

> +        av_free(f->prev);

this will read freed data
its a ring, theres no way to free the last element by taking a
pointer from inside another element of the ring as there is no
other element for the last one


> +    } while (f != head);
>  }
>  
>  static PullupField *make_field_queue(PullupContext *s, int len)
> @@ -158,14 +156,14 @@ static PullupField *make_field_queue(PullupContext *s, int len)
>      for (; len > 0; len--) {
>          f->next = av_mallocz(sizeof(*f->next));
>          if (!f->next) {
> -            free_field_queue(head, &f);
> +            free_field_queue(head);

this will segfault as f->next is used before checking its null in
free_field_queue

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

The real ebay dictionary, page 2
"100% positive feedback" - "All either got their money back or didnt complain"
"Best seller ever, very honest" - "Seller refunded buyer after failed scam"
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20140325/b1c753bc/attachment.asc>


More information about the ffmpeg-devel mailing list