RepoMirrors
/
mpv
mirror of https://github.com/mpv-player/mpv
1
0
Fork 0
Code Issues Releases Wiki Activity

vf_vapoursynth: fix crashing memory management mistake 

As pointed out by @olifre in #7016, this line of code was wrong. p->opts
at this point is a struct allocated and managed by m_config. opts->file
is a string, and m_config explicitly frees it on destruction. The line
of code in question replaced the opts->file value, and made both the old
and new value children of the talloc allocation, so they were _also_
freed on destruction.

This crashed due to a double-free. First, talloc auto-freeing freed the
string, then m_config explicitly called talloc_free() on the stale
pointer again.

As @v-fox pointed out, commit 36dd2348a1 seems to have triggered the
crash. I suspect this code merely worked out of coincidence, since it
allowed m_config to free the value first. This removed it from the
auto-free list, and thus did not result in a double-free. The change
in order of calling alloc destructors changed the order of these calls.

There is no strong reason why new behavior (as introduced by commit
36dd2348a1) would be wrong (it feels like cleaner behavior). On the
other hand, what the vf_vapoursynth code did is clearly unclean and
going by the m_config API, you're not at all supposed to do this.
m_config manages all memroy referenced by option structs, the end.

@olifre's suggested fix also would have been correct (not just hiding
the issue), I prefer my fix, as it doesn't mess with the option struct
in tricky ways.

This wouldn't have happened if mpv were written in Haskell.
This commit is contained in:
wm4 2019-10-03 00:41:30 +02:00
parent d2a10fb02e
commit 5f75365f44
1 changed files with 3 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
video/filter/vf_vapoursynth.c
View File

@ -54,6 +54,7 @@ struct vapoursynth_opts {
struct priv {
struct mp_log *log;
struct vapoursynth_opts *opts;
char *script_path;
VSCore *vscore;
const VSAPI *vsapi;
@ -758,8 +759,7 @@ static struct mp_filter *vf_vapoursynth_create(struct mp_filter *parent,
MP_FATAL(p, "'file' parameter must be set.\n");
goto error;
}
talloc_steal(p, p->opts->file);
p->opts->file = mp_get_user_path(p, f->global, p->opts->file);
p->script_path = mp_get_user_path(p, f->global, p->opts->file);
p->max_requests = p->opts->maxrequests;
if (p->max_requests < 0)
@ -841,7 +841,7 @@ static int drv_vss_load(struct priv *p, VSMap *vars)
{
vsscript_setVariable(p->se, vars);
if (vsscript_evaluateFile(&p->se, p->opts->file, 0)) {
if (vsscript_evaluateFile(&p->se, p->script_path, 0)) {
MP_FATAL(p, "Script evaluation failed:\n%s\n", vsscript_getError(p->se));
return -1;
}