lkml.org 
[lkml]   [2013]   [Aug]   [16]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v5 2/3] fs/binfmts: Better handling of binfmt loops
On 08/15, Zach Levis wrote:
>
> +static bool update_prev_binfmts(struct linux_binprm *bprm,
> + struct linux_binfmt *cur_fmt)
> +{
> +
> + if (!try_module_get(cur_fmt->module))
> + return false;
> + if (bprm->previous_binfmts[1])
> + put_binfmt(bprm->previous_binfmts[1]);
> + bprm->previous_binfmts[1] = bprm->previous_binfmts[0];
> + bprm->previous_binfmts[0] = cur_fmt;
> + return true;
> +}

Still can't understand the logic behind this function and its usage.
IOW, what ->previous_binfmts[] actually means? previous_binfmts[1]
could be a caller or the previous fmt which was called at the same
depth.

> @@ -1393,15 +1498,38 @@ int search_binary_handler(struct linux_binprm *bprm)
> list_for_each_entry(fmt, &formats, lh) {
> if (!try_module_get(fmt->module))
> continue;
> +
> + if (!update_prev_binfmts(bprm, fmt))
> + continue;
> +
> read_unlock(&binfmt_lock);
> +
> bprm->recursion_depth++;
> retval = fmt->load_binary(bprm);
> bprm->recursion_depth--;
> - if (retval >= 0 || retval != -ENOEXEC ||
> - bprm->mm == NULL || bprm->file == NULL) {
> + if (retval == -ELOOP
> + && bprm->recursion_depth == 0) { /* cur, previous */
> + pr_err("Too much recursion with binfmts (0:%s, -1:%s) in file %s, skipping (base %s).\n",
> + binfmt_name(bprm->previous_binfmts[0]),
> + binfmt_name(bprm->previous_binfmts[1]),
> + bprm->filename,
> + fmt->name);
> +
> + free_arg_pages(bprm);
> + if (bprm->interp != bprm->filename)
> + kfree(bprm->interp);

this doesn't look safe too, kfree(interp) can be called twice.

and once again, we should not lose -ELOOP as an error code if the
next fmt returns ENOEXEC.

But the main problem (in my opinion) is that this doesn't worth the
trouble, sorry.

Oleg.



\
 
 \ /
  Last update: 2013-08-16 17:01    [W:0.210 / U:0.656 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site