Messages in this thread Patch in this message | | | Subject | Re: [PATCH] Check return from argv_split() in do_coredump(). | From | Tetsuo Handa <> | Date | Fri, 26 Dec 2008 22:20:09 +0900 |
| |
Hello.
Americo Wang wrote: > fail_unlock: > if (helper_argv) > argv_free(helper_argv); > > current->fsuid = fsuid; //<=== goto this line > coredump_finish(mm); > > You need to add a new label, of course. :) > Ah, you were talking about destination of goto statement. I see.
Unfortunately, -mm source uses "revert_creds(old_cred); put_cred(cred);" instead of "current->fsuid = fsuid;". To keep this patch applicable to all afftected versions, I'd like not to introduce a new label.
> if (helper_argv) > argv_free(helper_argv); Well, I think it's better to check "if (helper_argv)" inside argv_free() in case the caller forgets to check.
Regards. -------------------- Subject: Check return from argv_split() in do_coredump().
do_coredump() accesses helper_argv[0] without checking helper_argv != NULL. Though, likely helper_argv != NULL.
Below versions have this problem.
2.6.24.7 2.6.25.20 2.6.26.8 2.6.27.10 2.6.28 mmotm 2008-12-24-01-20
Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> --- fs/exec.c | 2 ++ 1 file changed, 2 insertions(+)
--- linux-2.6.28-rc9-mm1.orig/fs/exec.c +++ linux-2.6.28-rc9-mm1/fs/exec.c @@ -1808,6 +1808,8 @@ int do_coredump(long signr, int exit_cod if (ispipe) { helper_argv = argv_split(GFP_KERNEL, corename+1, &helper_argc); + if (!helper_argv) + goto fail_unlock; /* Terminate the string before the first option */ delimit = strchr(corename, ' '); if (delimit)
| |