Messages in this thread | | | Date | Thu, 22 May 2008 12:34:59 +0800 | From | Li Zefan <> | Subject | Re: [LTP] [PATCH] fix sys_prctl() returned uninitialized value |
| |
Andrew Morton wrote: > On Thu, 22 May 2008 11:19:21 +0800 Shi Weihua <shiwh@cn.fujitsu.com> wrote: > >> When we test kernel by the latest LTP(20080430) on ia64, >> the following failure occured: >> ------------------------------------- >> prctl01 1 PASS : Test Passed >> prctl01 0 WARN : prctl() returned 2048 errno = 0 : Success >> prctl01 1 PASS : Test Passed >> prctl01 2 FAIL : Test Failed >> ------------------------------------- >> >> We found commit 3898b1b4ebff8dcfbcf1807e0661585e06c9a91c >> causes this failure by git-bisect. >> And, we found 'error' has not been initialized in the function >> sys_prctl()(kernel/sys.c). When the capability module is not taking >> responsibility for this call, sys_prctl() may return a wrong value. >> >> Now, i fixed it. >> >> Signed-off-by: Shi Weihua <shiwh@cn.fujitsu.com> >> --- >> diff --git a/kernel/sys.c b/kernel/sys.c >> index 895d2d4..26eb0f7 100644 >> --- a/kernel/sys.c >> +++ b/kernel/sys.c >> @@ -1652,7 +1652,7 @@ asmlinkage long sys_umask(int mask) >> asmlinkage long sys_prctl(int option, unsigned long arg2, unsigned long arg3, >> unsigned long arg4, unsigned long arg5) >> { >> - long uninitialized_var(error); >> + long error = 0; >> >> if (security_task_prctl(option, arg2, arg3, arg4, arg5, &error)) >> return error; > > Oh dear, there are so many things wrong with this... > > - if security_task_prctl() is returning "fail" then why on earth > isn't it setting the error code? >
See comments in security.h:
* @task_prctl: ... * @rc_p contains a pointer to communicate back the forced return code * Return 0 if permission is granted, and non-zero if the security module * has taken responsibility (setting *rc_p) for the prctl call.
But I don't know why can't just set *rc_p to 0 before returning 0 (as Shi's previous patch did).
> - cap_task_prctl() _does_ set `error' is if returns non-zero, so it > must be one of the other myriad backend implementations of > security_task_prctl() which is busted. Which one is it? > > - With the above patch applied, sys_prctl() will return zero (ie: > "success") even though it just failed. >
This won't happen. We initialize error to 0, and it will be set to some error value when it should be. The alternative is to set error to -EFXXX or 0 in every switch cases.
> - Can't we remove the sixth argument to security_task_prctl() and > just return the result code like a sane function would do? >
It used to have 5 arguments, but this commit changed it (and caused this ltp failure): 3898b1b4ebff8dcfbcf1807e0661585e06c9a91c (capabilities: implement per-process securebits)
| |