On 02/04, Linus Torvalds wrote:
>
> --- a/kernel/kmod.c
> +++ b/kernel/kmod.c
> @@ -239,7 +239,7 @@ static int ____call_usermodehelper(void *data)
>
> commit_creds(new);
>
> - retval = do_execve(sub_info->path,
> + retval = do_execve(getname_kernel(sub_info->path),
> (const char __user *const __user *)sub_info->argv,
> (const char __user *const __user *)sub_info->envp);
Great, this naturally duplicates filename unconditionally, and we can
kill bprm->tcomm[].
But,
> --- a/include/linux/binfmts.h
> +++ b/include/linux/binfmts.h
> @@ -37,7 +37,7 @@ struct linux_binprm {
> int unsafe; /* how unsafe this exec is (mask of
> LSM_UNSAFE_*) */
> unsigned int per_clear; /* bits to clear in current->personality */
> int argc, envc;
> - const char * filename; /* Name of binary as seen by procps */
> + struct filename *filename; /* Name of binary as seen by procps */
Do we really need this change? If not (afaics), the patch can be
much simpler, see below...
> -void free_bprm(struct linux_binprm *bprm)
> +static void free_bprm(struct linux_binprm *bprm)
> {
> free_arg_pages(bprm);
> if (bprm->cred) {
> @@ -1174,15 +1179,17 @@ void free_bprm(struct linux_binprm *bprm)
> fput(bprm->file);
> }
> /* If a binfmt changed the interp, free it. */
> - if (bprm->interp != bprm->filename)
> + if (bprm->interp != bprm->filename->name)
> kfree(bprm->interp);
> + if (bprm->filename)
> + putname(bprm->filename);
Even if we actually need to turn bprm->filename into "struct filename"
this free_bprm()->putname() only complicates the code, unless I missed
something. The caller, do_execve(), can do putname() unconditionally and
avoid if/NULL games.
IOW, doesn't the change below (on top of your patch) obviously makes
sense or I am totally confused?
Oleg.
--- x/fs/exec.c
+++ x/fs/exec.c
@@ -1183,8 +1183,6 @@ static void free_bprm(struct linux_binpr
/* If a binfmt changed the interp, free it. */
if (bprm->interp != bprm->filename->name)
kfree(bprm->interp);
- if (bprm->filename)
- putname(bprm->filename);
kfree(bprm);
}
@@ -1478,9 +1476,6 @@ static int do_execve_common(struct filen
if (!bprm)
goto out_files;
- bprm->filename = filename;
- bprm->interp = filename->name;
-
retval = prepare_bprm_creds(bprm);
if (retval)
goto out_free;
@@ -1496,6 +1491,8 @@ static int do_execve_common(struct filen
sched_exec();
bprm->file = file;
+ bprm->filename = filename;
+ bprm->interp = filename->name;
retval = bprm_mm_init(bprm);
if (retval)
@@ -1538,7 +1535,7 @@ static int do_execve_common(struct filen
free_bprm(bprm);
if (displaced)
put_files_struct(displaced);
- return retval;
+ goto out_ret;
out:
if (bprm->mm) {
@@ -1552,14 +1549,12 @@ out_unmark:
out_free:
free_bprm(bprm);
- filename = NULL;
out_files:
if (displaced)
reset_files_struct(displaced);
out_ret:
- if (filename)
- putname(filename);
+ putname(filename);
return retval;
}
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
