Messages in this thread | | | From | Lucas De Marchi <> | Date | Tue, 21 May 2013 22:09:57 -0300 | Subject | Re: [PATCH] usermodehelper: kill the sub_info->path[0] check |
| |
Hi Oleg,
On Mon, May 20, 2013 at 1:55 PM, Oleg Nesterov <oleg@redhat.com> wrote: > call_usermodehelper_exec() does nothing but returns success if > path[0] == 0. The only user which needs this strange feature is > request_module(), it can check modprobe_path[0] itself like other > users do if they want to detect the "disabled by admin" case. > > Kill it. Not only it looks strange, it can confuse other callers. > And this allows us to revert 264b83c0 "usermodehelper: check > subprocess_info->path != NULL", do_execve(NULL) is safe. > > Signed-off-by: Oleg Nesterov <oleg@redhat.com>
Acked-By: Lucas De Marchi <lucas.demarchi@intel.com>
But... see below.
> --- > kernel/kmod.c | 11 +++-------- > 1 files changed, 3 insertions(+), 8 deletions(-) > > diff --git a/kernel/kmod.c b/kernel/kmod.c > index 8241906..fb32636 100644 > --- a/kernel/kmod.c > +++ b/kernel/kmod.c > @@ -147,6 +147,9 @@ int __request_module(bool wait, const char *fmt, ...) > */ > WARN_ON_ONCE(wait && current_is_async()); > > + if (!modprobe_path[0]) > + return 0; > +
Any reason to not return -EINVAL here except for maintaining the previous behavior? Checking the callers reveals just a few of them actually check the return value and IMO this is no different than the binary not existing and failing later on exec.
> va_start(args, fmt); > ret = vsnprintf(module_name, MODULE_NAME_LEN, fmt, args); > va_end(args); > @@ -569,14 +572,6 @@ int call_usermodehelper_exec(struct subprocess_info *sub_info, int wait) > int retval = 0; > > helper_lock(); > - if (!sub_info->path) { > - retval = -EINVAL; > - goto out; > - } > - > - if (sub_info->path[0] == '\0') > - goto out; > - > if (!khelper_wq || usermodehelper_disabled) { > retval = -EBUSY; > goto out; > --
Lucas De Marchi
| |