linux-kernel - Re: [PATCH] fix sys_prctl() returned uninitialized value

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <48357017.5060206@kernel.org>
Date:	Thu, 22 May 2008 06:07:35 -0700
From:	"Andrew G. Morgan" <morgan@...nel.org>
To:	Andrew Morton <akpm@...ux-foundation.org>
CC:	Shi Weihua <shiwh@...fujitsu.com>,
	"Serge E. Hallyn" <serue@...ibm.com>,
	linux-security-module@...r.kernel.org,
	LKML <linux-kernel@...r.kernel.org>, jmorris@...ei.org,
	ltp-list@...ts.sourceforge.net
Subject: Re: [PATCH] fix sys_prctl() returned uninitialized value

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Acked-by: Andrew G. Morgan <morgan@...nel.org>

Cheers

Andrew

Andrew Morton wrote:
| On Wed, 21 May 2008 22:01:17 -0700 "Andrew G. Morgan"
<morgan@...nel.org> wrote:
|
|> this is the default expected by the subsequent switch ().
|>
|> Signed-off-by: Andrew G. Morgan <morgan@...nel.org>
|> ---
|>  kernel/sys.c |    2 ++
|>  1 files changed, 2 insertions(+), 0 deletions(-)
|>
|> diff --git a/kernel/sys.c b/kernel/sys.c
|> index 895d2d4..cb25a64 100644
|> --- a/kernel/sys.c
|> +++ b/kernel/sys.c
|> @@ -1657,6 +1657,8 @@ asmlinkage long sys_prctl(int option, unsigned
long arg2, unsigned long arg3,
|>  	if (security_task_prctl(option, arg2, arg3, arg4, arg5, &error))
|>  		return error;
|>
|> +	error = 0;
|> +
|>  	switch (option) {
|>  		case PR_SET_PDEATHSIG:
|>  			if (!valid_signal(arg2)) {
|
| Looking at it some more there are two cases which don't initialise
| `error': PR_SET_PDEATHSIG and PR_SET_DUMPABLE.  (let's set aside the
| silliness of having sys_prctl() perform set_dumpable()'s argument
| checking for it).
|
| So I would propose this fix, mainly because it removes that nasty
| uninitialized_var().  Please review carefully.
|
|
|
| From: Shi Weihua <shiwh@...fujitsu.com>
|
| If none of the switch cases match, the PR_SET_PDEATHSIG and
| PR_SET_DUMPABLE cases of the switch statement will never write to local
| variable `error'.
|
| Signed-off-by: Shi Weihua <shiwh@...fujitsu.com>
| Cc: Andrew G. Morgan <morgan@...nel.org>
| Cc: "Serge E. Hallyn" <serue@...ibm.com>
| Signed-off-by: Andrew Morton <akpm@...ux-foundation.org>
| ---
|
|  kernel/sys.c |    6 ++----
|  1 file changed, 2 insertions(+), 4 deletions(-)
|
| diff -puN kernel/sys.c~sys_prctl-fix-return-of-uninitialized-value
kernel/sys.c
| --- a/kernel/sys.c~sys_prctl-fix-return-of-uninitialized-value
| +++ a/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;
| @@ -1701,9 +1701,7 @@ asmlinkage long sys_prctl(int option, un
|  			error = PR_TIMING_STATISTICAL;
|  			break;
|  		case PR_SET_TIMING:
| -			if (arg2 == PR_TIMING_STATISTICAL)
| -				error = 0;
| -			else
| +			if (arg2 != PR_TIMING_STATISTICAL)
|  				error = -EINVAL;
|  			break;
|
| _
|
| --
| To unsubscribe from this list: send the line "unsubscribe
linux-security-module" in
| the body of a message to majordomo@...r.kernel.org
| More majordomo info at  http://vger.kernel.org/majordomo-info.html
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.6 (GNU/Linux)

iD8DBQFINXAX+bHCR3gb8jsRAl4JAJ0djex041bKhq2gvcwUdJpfjSt9+gCeI4g0
uh1GZlx9WxoeU4ztcOV2kHI=
=c1Dw
-----END PGP SIGNATURE-----
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/