Re: do_dup patch

On Mon, 25 Nov 2002, Olivier Houchard wrote:

> On Mon, Nov 25, 2002 at 09:32:05PM +1100, Bruce Evans wrote:
> > On Mon, 25 Nov 2002, Olivier Houchard wrote:
> >
> > > This patch makes the "new" and "old" do_dup arguments unsigned int instead of
> > > int. I can't see any reason they would have to be int, and right now a call to
> > > dup or dup2 with an invalid negative fd, such as dup(-999999); will panic a
> > > -CURRENT box (the problem doesn't exist on -STABLE because the file descriptor
> > > validity is checked in dup() and dup2()).
> > > Is there anything wrong with committing this ?
> >
> > Yes; it is sort of backwards.
e >
> > [since file descriptors should be (represented by) (signed) ints]

> So what would be the best ?
> Changing dup and dup2 prototypes in syscall.master to make new and old
> signed instead of unsigned and checking that new >= 0 && old >= 0 in
> do_dup ?

I just added the bounds checks.  Cleaning up the prototypes can wait.
(There are hundreds of other wrong prototypes their anyway, most involving
use of "int" or "u_int" instead of foo_t or not using "const".)

This has not been tested at runtime.

%%%
Index: kern_descrip.c
===================================================================
RCS file: /home/ncvs/src/sys/kern/kern_descrip.c,v
retrieving revision 1.168
diff -u -2 -r1.168 kern_descrip.c
--- kern_descrip.c	27 Oct 2002 18:07:41 -0000	1.168
+++ kern_descrip.c	25 Nov 2002 11:56:27 -0000
@@ -471,6 +475,6 @@
 	 */
 	FILEDESC_LOCK(fdp);
-	if (old >= fdp->fd_nfiles || fdp->fd_ofiles[old] == NULL ||
-	    new >= p->p_rlimit[RLIMIT_NOFILE].rlim_cur ||
+	if (old < 0 || old >= fdp->fd_nfiles || fdp->fd_ofiles[old] == NULL ||
+	    new < 0 || new >= p->p_rlimit[RLIMIT_NOFILE].rlim_cur ||
 	    new >= maxfilesperproc) {
 		FILEDESC_UNLOCK(fdp);
%%%

Bruce


To Unsubscribe: send mail to [email protected]
with "unsubscribe freebsd-audit" in the body of the message