NetBSD Problem Report #40717

From dholland@netbsd.org  Sun Feb 22 21:44:37 2009
Return-Path: <dholland@netbsd.org>
Received: from mail.netbsd.org (mail.netbsd.org [204.152.190.11])
	by www.NetBSD.org (Postfix) with ESMTP id 474FB63B8C3
	for <gnats-bugs@gnats.NetBSD.org>; Sun, 22 Feb 2009 21:44:37 +0000 (UTC)
Message-Id: <20090222214437.2C00163B1C0@mail.netbsd.org>
Date: Sun, 22 Feb 2009 21:44:37 +0000 (UTC)
From: dholland@netbsd.org
Reply-To: dholland@netbsd.org
To: gnats-bugs@gnats.NetBSD.org
Subject: kernel data leak in wait4()
X-Send-Pr-Version: 3.95

>Number:         40717
>Category:       kern
>Synopsis:       kernel data leak in wait4()
>Confidential:   no
>Severity:       serious
>Priority:       high
>Responsible:    kern-bug-people
>State:          closed
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Sun Feb 22 21:45:04 +0000 2009
>Closed-Date:    Sun Nov 01 21:09:47 +0000 2009
>Last-Modified:  Sat Nov 14 18:20:02 +0000 2009
>Originator:     David A. Holland
>Release:        NetBSD 5.99.7 (20080221)
>Organization:
>Environment:
(irrelevant)

>Description:

The rusage parameter of wait4() returns a copy of an uninitialized
chunk of kernel stack for stopped processes.

>How-To-Repeat:

code reading

>Fix:

Can't currently test this (or much of anything) because of the
premature removal of softdep.

Index: kern_exit.c
===================================================================
RCS file: /cvsroot/src/sys/kern/kern_exit.c,v
retrieving revision 1.218
diff -u -p -r1.218 kern_exit.c
--- kern_exit.c	22 Jan 2009 14:38:35 -0000	1.218
+++ kern_exit.c	22 Feb 2009 21:38:42 -0000
@@ -688,9 +688,10 @@ do_sys_wait(struct lwp *l, int *pid, int
 	if (child->p_stat == SZOMB) {
 		/* proc_free() will release the proc_lock. */
 		*was_zombie = 1;
-		if (options & WNOWAIT)
+		if (options & WNOWAIT) {
 			mutex_exit(proc_lock);
-		else {
+			memset(ru, 0, sizeof(*ru));
+		} else {
 			proc_free(child, ru);
 		}
 	} else {
@@ -698,6 +699,7 @@ do_sys_wait(struct lwp *l, int *pid, int
 		*was_zombie = 0;
 		mutex_exit(proc_lock);
 		*status = W_STOPCODE(*status);
+		memset(ru, 0, sizeof(*ru));
 	}

 	return 0;

>Release-Note:

>Audit-Trail:

State-Changed-From-To: open->closed
State-Changed-By: rmind@NetBSD.org
State-Changed-When: Sun, 01 Nov 2009 21:09:47 +0000
State-Changed-Why:
Applied, thanks.


From: Mindaugas Rasiukevicius <rmind@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/40717 CVS commit: src/sys/kern
Date: Sun, 1 Nov 2009 21:05:30 +0000

 Module Name:	src
 Committed By:	rmind
 Date:		Sun Nov  1 21:05:30 UTC 2009

 Modified Files:
 	src/sys/kern: kern_exit.c

 Log Message:
 do_sys_wait: clear rusage, instead of returning garbage.  Patch from
 dholland@ via PR/40717, with minor change by me.


 To generate a diff of this commit:
 cvs rdiff -u -r1.223 -r1.224 src/sys/kern/kern_exit.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

From: Onno van der Linden <o.vd.linden@quicknet.nl>
To: gnats-bugs@netbsd.org
Cc: 
Subject: Re: kern/40717
Date: Wed, 4 Nov 2009 20:42:17 +0100

 > diff -u -p -r1.218 kern_exit.c
 > --- kern_exit.c	22 Jan 2009 14:38:35 -0000	1.218
 > +++ kern_exit.c	22 Feb 2009 21:38:42 -0000
 > @@ -688,9 +688,10 @@ do_sys_wait(struct lwp *l, int *pid, int
 >  	if (child->p_stat == SZOMB) {
 >  		/* proc_free() will release the proc_lock. */
 >  		*was_zombie = 1;
 > -		if (options & WNOWAIT)
 > +		if (options & WNOWAIT) {
 >  			mutex_exit(proc_lock);
 > -		else {
 > +			memset(ru, 0, sizeof(*ru));
 > +		} else {
 >  			proc_free(child, ru);
 >  		}
 >  	} else {
 > @@ -698,6 +699,7 @@ do_sys_wait(struct lwp *l, int *pid, int
 >  		*was_zombie = 0;
 >  		mutex_exit(proc_lock);
 >  		*status = W_STOPCODE(*status);
 > +		memset(ru, 0, sizeof(*ru));
 >  	}

 The "unprotected" memsets won't like a null pointer being
 passed to them, the call to do_sys_wait in sys___wait450 says:

 error = do_sys_wait(l, &pid, &status, SCARG(uap, options),
 	    SCARG(uap, rusage) != NULL ? &ru : NULL, &was_zombie)

 which means ru can be null.

 With ^Z as my susp character I tried at the shell prompt:
 cat
 ^Z

 and poof .....

 if (ru)
 in front of those memsets will fix that.

 And what's up with that was_zombie variable in sys___wait450 ?
 It gets set in do_sys_wait but is never referenced again.

 Onno

From: David Holland <dholland-bugs@netbsd.org>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: kern/40717
Date: Sat, 14 Nov 2009 18:18:08 +0000

 On Wed, Nov 04, 2009 at 08:50:03PM +0000, Onno van der Linden wrote:
  >  The "unprotected" memsets won't like a null pointer being
  >  passed to them, the call to do_sys_wait in sys___wait450 says:
  > [...]
  >  And what's up with that was_zombie variable in sys___wait450 ?
  >  It gets set in do_sys_wait but is never referenced again.

 For the gnats record: rmind fixed all this.

 -- 
 David A. Holland
 dholland@netbsd.org

>Unformatted:

Home	PR Database Search