openssh unix dev - Mar 2001 - Bug in bsd-waitpid.c and bsd-nextstep.c

Hi!

The handling of the "status" information in bsd-waitpid.c and
bsd-nextstep.c
seems to be bit odd. Patch attached.

Best regards,
	Lutz
-- 
Lutz Jaenicke                             Lutz.Jaenicke at aet.TU-Cottbus.DE
BTU Cottbus               http://www.aet.TU-Cottbus.DE/personen/jaenicke/
Lehrstuhl Allgemeine Elektrotechnik                  Tel. +49 355 69-4129
Universitaetsplatz 3-4, D-03044 Cottbus              Fax. +49 355 69-4153
-------------- next part --------------
--- bsd-nexstep.c.org	Sun Mar 25 15:08:05 2001
+++ bsd-nextstep.c	Sun Mar 25 15:09:42 2001
@@ -37,7 +37,8 @@
 
 	#undef wait			/* Use NeXT's wait() function */
 	wait_pid = wait(&statusp);
-	status = (int *) statusp.w_status;
+	if (status)
+		*status = (int)statusp.w_status;
 
 	return wait_pid;
 }
--- bsd-waitpid.c.org	Sun Mar 25 15:06:20 2001
+++ bsd-waitpid.c	Sun Mar 25 15:07:36 2001
@@ -43,7 +43,8 @@
 		pid = 0;   /* wait4() wants pid=0 for indiscriminate wait. */
 	}
         wait_pid = wait4(pid, &statusp, options, NULL);
-        stat_loc = (int *)statusp.w_status;            
+	if (stat_loc)
+        	*stat_loc = (int)statusp.w_status;            
         return wait_pid;                               
 }

On Sun, 25 Mar 2001, Lutz Jaenicke wrote:
> Hi!
>
> The handling of the "status" information in bsd-waitpid.c and
bsd-nextstep.c
> seems to be bit odd. Patch attached.
>
Thanks, I'll apply it as soon as the CVS tree is back up.  Hey, Damien
are you having problems?

The Usenet post that I used to base the code on never checked if
'status'
was NULL so I never thought about it at that time.  Since I've never
really had a problem with either function on NeXTStep I never have gone
back to review any of the code.

- Ben

On Sun, Mar 25, 2001 at 11:40:03AM -0600, mouring at etoh.eviladmin.org
wrote:
> On Sun, 25 Mar 2001, Lutz Jaenicke wrote:
> > The handling of the "status" information in bsd-waitpid.c
and bsd-nextstep.c
> > seems to be bit odd. Patch attached.
> 
> Thanks, I'll apply it as soon as the CVS tree is back up.  Hey, Damien
> are you having problems?
> 
> The Usenet post that I used to base the code on never checked if
'status'
> was NULL so I never thought about it at that time.  Since I've never
> really had a problem with either function on NeXTStep I never have gone
> back to review any of the code.
Since NeXTStep on HPPA seems to have problems with its own BSD extensions,
Jacques Distler recommended to use openbsd-compat for use with PRNGD on
that platform. While examining problems he just reported, I found these
bugs. The major part is not that NULL is not checked, but that the code
was simply wrong:
If stat_loc is pointer to an int passed to waitpid(), the code
  stat_loc = (int *)statusp.w_status;
will write the pointer to statusp.w_status to the local copy of stat_loc.
After returning from waitpid(), the calling function did not get back the
information requested. (Since only the local copy is affected, it also
did not hurt to pass the NULL pointer.)

As you write that you got the code from a Usenet post, I would recommend
to check out other portions of the code for correct use of pointer arguments.

Best regards,
	Lutz
-- 
Lutz Jaenicke                             Lutz.Jaenicke at aet.TU-Cottbus.DE
BTU Cottbus               http://www.aet.TU-Cottbus.DE/personen/jaenicke/
Lehrstuhl Allgemeine Elektrotechnik                  Tel. +49 355 69-4129
Universitaetsplatz 3-4, D-03044 Cottbus              Fax. +49 355 69-4153