Fix warnings: eval.c: f_termopen(): Use-after-free: MI.

Problem    : Use-after-free @ 15081.
Diagnostic : Multithreading issue.
Rationale  : `get_dict_callback` can return NULL on two different
             cases: 1) when the dict doesn't contain the given key;
             this case is not considered an error. 2) when the key
             exists but there's some problem with its value; this is
             considered an error.
             Then, code calling `get_dict_callback` in
             `common_job_callbacks`, as well as code calling
             `common_job_callbacks`, uses `did_emsg` to distinguish
             between error/non-error cases.
             Suggested error path presumes an error condition within
             `common_job_callbacks`, with `did_emsg` being true, but
             then being false just after returning to calling code in
             `f_termopen`.
             That, clearly, could only happen if another thread run in
             between those points.
Resolution : Refactor `get_dict_callback` and `common_job_callbacks`, so
             that they clearly distinguish between error/non-error
             situations, without recurring to globals.
This commit is contained in:
Eliseo Martínez 2015-03-30 19:11:17 +02:00 committed by Justin M. Keyes
parent 31581bd750
commit bddba93949

View File

@ -5942,18 +5942,23 @@ dictitem_T *dict_find(dict_T *d, char_u *key, int len)
return HI2DI(hi); return HI2DI(hi);
} }
// Get a function from a dictionary /// Get a function from a dictionary
static ufunc_T *get_dict_callback(dict_T *d, char *key) /// @param[out] result The address where a pointer to the wanted callback
/// will be left.
/// @return true/false on success/failure.
static bool get_dict_callback(dict_T *d, char *key, ufunc_T **result)
{ {
dictitem_T *di = dict_find(d, (uint8_t *)key, -1); dictitem_T *di = dict_find(d, (uint8_t *)key, -1);
if (di == NULL) { if (di == NULL) {
return NULL; *result = NULL;
return true;
} }
if (di->di_tv.v_type != VAR_FUNC && di->di_tv.v_type != VAR_STRING) { if (di->di_tv.v_type != VAR_FUNC && di->di_tv.v_type != VAR_STRING) {
EMSG(_("Argument is not a function or function name")); EMSG(_("Argument is not a function or function name"));
return NULL; *result = NULL;
return false;
} }
uint8_t *name = di->di_tv.vval.v_string; uint8_t *name = di->di_tv.vval.v_string;
@ -5970,11 +5975,13 @@ static ufunc_T *get_dict_callback(dict_T *d, char *key)
if (!rv) { if (!rv) {
EMSG2(_("Function %s doesn't exist"), name); EMSG2(_("Function %s doesn't exist"), name);
return NULL; *result = NULL;
return false;
} }
rv->uf_refcount++; rv->uf_refcount++;
return rv; *result = rv;
return true;
} }
/* /*
@ -10822,8 +10829,7 @@ static void f_jobstart(typval_T *argvars, typval_T *rettv)
ufunc_T *on_stdout = NULL, *on_stderr = NULL, *on_exit = NULL; ufunc_T *on_stdout = NULL, *on_stderr = NULL, *on_exit = NULL;
if (argvars[1].v_type == VAR_DICT) { if (argvars[1].v_type == VAR_DICT) {
job_opts = argvars[1].vval.v_dict; job_opts = argvars[1].vval.v_dict;
common_job_callbacks(job_opts, &on_stdout, &on_stderr, &on_exit); if (!common_job_callbacks(job_opts, &on_stdout, &on_stderr, &on_exit)) {
if (did_emsg) {
return; return;
} }
} }
@ -15079,8 +15085,7 @@ static void f_termopen(typval_T *argvars, typval_T *rettv)
dict_T *job_opts = NULL; dict_T *job_opts = NULL;
if (argvars[1].v_type == VAR_DICT) { if (argvars[1].v_type == VAR_DICT) {
job_opts = argvars[1].vval.v_dict; job_opts = argvars[1].vval.v_dict;
common_job_callbacks(job_opts, &on_stdout, &on_stderr, &on_exit); if (!common_job_callbacks(job_opts, &on_stdout, &on_stderr, &on_exit)) {
if (did_emsg) {
return; return;
} }
} }
@ -20053,27 +20058,27 @@ static inline JobOptions common_job_options(char **argv, ufunc_T *on_stdout,
return opts; return opts;
} }
static inline void common_job_callbacks(dict_T *vopts, ufunc_T **on_stdout, /// Return true/false on success/failure.
ufunc_T **on_stderr, ufunc_T **on_exit) static inline bool common_job_callbacks(dict_T *vopts, ufunc_T **on_stdout,
ufunc_T **on_stderr, ufunc_T **on_exit)
{ {
*on_stdout = get_dict_callback(vopts, "on_stdout"); if (get_dict_callback(vopts, "on_stdout", on_stdout)
*on_stderr = get_dict_callback(vopts, "on_stderr"); && get_dict_callback(vopts, "on_stderr", on_stderr)
*on_exit = get_dict_callback(vopts, "on_exit"); && get_dict_callback(vopts, "on_exit", on_exit)) {
if (did_emsg) { vopts->internal_refcount++;
if (*on_stdout) { vopts->dv_refcount++;
user_func_unref(*on_stdout); return true;
}
if (*on_stderr) {
user_func_unref(*on_stderr);
}
if (*on_exit) {
user_func_unref(*on_exit);
}
return;
} }
if (*on_stdout) {
vopts->internal_refcount++; user_func_unref(*on_stdout);
vopts->dv_refcount++; }
if (*on_stderr) {
user_func_unref(*on_stderr);
}
if (*on_exit) {
user_func_unref(*on_exit);
}
return false;
} }
static inline Job *common_job_start(JobOptions opts, typval_T *rettv) static inline Job *common_job_start(JobOptions opts, typval_T *rettv)