Opened 6 years ago

Closed 4 years ago

#7084 closed defect (fixed)

Memory leak in libavfilter/graphparser.c

Reported by: Kira Owned by:
Priority: normal Component: avfilter
Version: unspecified Keywords: leak
Cc: Blocked By:
Blocking: Reproduced by developer: no
Analyzed by developer: no

Description

parse_outputs from libavfilter/graphparser.c

 static int parse_outputs(const char **buf, AVFilterInOut **curr_inputs,
                         AVFilterInOut **open_inputs,
                         AVFilterInOut **open_outputs, void *log_ctx)
{
    int ret, pad = 0;

    while (**buf == '[') {
        char *name = parse_link_name(buf, log_ctx);
        AVFilterInOut *match;

        AVFilterInOut *input = *curr_inputs;

        if (!name)
            return AVERROR(EINVAL);

        if (!input) {
            av_log(log_ctx, AV_LOG_ERROR,
                   "No output pad can be associated to link label '%s'.\n", name);
            av_free(name);
            return AVERROR(EINVAL);
        }
        *curr_inputs = (*curr_inputs)->next;

        /* First check if the label is not in the open_inputs list */
        match = extract_inout(name, open_inputs);

        if (match) {
            if ((ret = link_filter(input->filter_ctx, input->pad_idx,
                                   match->filter_ctx, match->pad_idx, log_ctx)) < 0) {
                av_free(name);
                return ret;
            }
            av_freep(&match->name);
            av_freep(&name);
            av_freep(&match);
            av_freep(&input);
        } else {
            /* Not in the list, so add the first input as an open_output */
            input->name = name;
            insert_inout(open_outputs, input);
        }
        *buf += strspn(*buf, WHITESPACES);
        pad++;
    }

    return pad;
}

line 361:
match = extract_inout(name, open_inputs);
Extract a node from the linked list open_inputs, the origin linked list changed here.
extract_inout from libavfilter/graphparser.c

static AVFilterInOut *extract_inout(const char *label, AVFilterInOut **links)
{
    AVFilterInOut *ret;

    // find the first node whose name is same as label.
    while (*links && (!(*links)->name || strcmp((*links)->name, label)))
        links = &((*links)->next);

    ret = *links;

    if (ret) {  // off the chain
        *links = ret->next;
        ret->next = NULL;
    }

    return ret;
}

If the match found, we enter here.

if (match) {
            if ((ret = link_filter(input->filter_ctx, input->pad_idx,
                                   match->filter_ctx, match->pad_idx, log_ctx)) < 0) {
                av_free(name);
                return ret;
            }
            av_freep(&match->name);
            av_freep(&name);
            av_freep(&match);
            av_freep(&input);
        }

If the link_filter call fails, the variable match will never be freed!

So the memory leak happens here.

Compile the leak_poc.c

Run it and use htop or something else to monitor the memory consumption of the process. You will see the leak.

Attachments (1)

leak_poc.c (2.1 KB ) - added by Kira 6 years ago.
poc of memory leak

Download all attachments as: .zip

Change History (5)

by Kira, 6 years ago

Attachment: leak_poc.c added

poc of memory leak

comment:1 by Carl Eugen Hoyos, 6 years ago

Keywords: memory parse_outputs removed
// reference: doc/examples/filtering_video.c

What is unclear about the first 13 lines of that file?

Do you know why the issue is not reproducible with ffmpeg?

comment:2 by Kira, 6 years ago

Replying to cehoyos:

// reference: doc/examples/filtering_video.c

What is unclear about the first 13 lines of that file?

Do you know why the issue is not reproducible with ffmpeg?

I'm terribly sorry about the copyright problem. I'm not a professional software developer, so I'm not familiar with the LICENSE stuff. Please forgive me about it. If anyone can help me make up for the problem I will be very grateful.
Back to the bug, actually the bug is found during my fuzz. I fuzz the second parameter of avfilter_graph_parse_ptr which is filters_descr. I found the leak when filters_descr is an invalid value "spectrumsynth".
The key point is to trigger the error in function link_filter. I don't know how ffmpeg resolves the input. But I think the problem is still there. If you really need to reproduce it with ffmpeg, maybe I will spend a lot of time on it...

Last edited 6 years ago by Kira (previous) (diff)

in reply to:  1 comment:3 by Kira, 6 years ago

Replying to cehoyos:

// reference: doc/examples/filtering_video.c

What is unclear about the first 13 lines of that file?

Do you know why the issue is not reproducible with ffmpeg?

I found that ffmpeg use avfilter_graph_parse2 rather than avfilter_graph_parse_ptr. I think it's because of the different behavior between avfilter_graph_parse2 and avfilter_graph_parse_ptr.

comment:4 by mkver, 4 years ago

Resolution: fixed
Status: newclosed

Fixed in deb6476fd8bc3a3c2b134704ecb804269843ed89. Thank you very much for your report.

Note: See TracTickets for help on using tickets.