Re: [PATCH] exec: do not leave bprm->interp on stack

From: Tetsuo Handa
Date: Fri Nov 23 2012 - 18:13:01 EST


P J P wrote:
>
> Hello Kees, all,
>
> Please have a look at a *NEW* patch at the end of this mail. It seems to fix
> both the issues, stack disclosure + undue recursions.
>
> It uses modprobe "--first-time" option which returns an error code when trying
> to load a module which is already present or unload one which is not present.

It might fix both "stack disclosure" + "undue recursions" issues, but it
introduces a regression "only one of concurrent requesters succeeds" which
ruins the value of automatic module loading feature.

> @@ -1423,7 +1423,14 @@ int search_binary_handler(struct linux_binprm *bprm,struct pt_regs *regs)
> break; /* -ENOEXEC */
> if (try)
> break; /* -ENOEXEC */
> - request_module("binfmt-%04x", *(unsigned short *)(&bprm->buf[2]));

What happens if more than one processes are requesting execve() of a program
which needs to call request_module("binfmt-%04x") to succeed, and more than two
of them concurrently reached at this line? Only one process will succeed. This
means that execve() of a program which needs to call request_module() will fail
if concurrently reached here.

> + if (request_module("binfmt-%04x",
> + *(unsigned short *)(&bprm->buf[2])))
> + {
> + printk(KERN_WARNING
> + "request_module: failed to load: binfmt-%04x",
> + *(unsigned short *)(&bprm->buf[2]));
> + break;
> + }
> }
> #else
> break;
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/