From owner-freebsd-audit Mon Nov 25 3:54:38 2002 Delivered-To: freebsd-audit@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 73B8B37B401 for ; Mon, 25 Nov 2002 03:54:37 -0800 (PST) Received: from mailman.zeta.org.au (mailman.zeta.org.au [203.26.10.16]) by mx1.FreeBSD.org (Postfix) with ESMTP id 69CE243E3B for ; Mon, 25 Nov 2002 03:54:36 -0800 (PST) (envelope-from bde@zeta.org.au) Received: from bde.zeta.org.au (bde.zeta.org.au [203.2.228.102]) by mailman.zeta.org.au (8.9.3/8.8.7) with ESMTP id WAA07307; Mon, 25 Nov 2002 22:54:29 +1100 Date: Mon, 25 Nov 2002 23:07:53 +1100 (EST) From: Bruce Evans X-X-Sender: bde@gamplex.bde.org To: Olivier Houchard Cc: freebsd-audit@FreeBSD.ORG Subject: Re: do_dup patch In-Reply-To: <20021125110902.GA10961@ci0.org> Message-ID: <20021125225927.O56791-100000@gamplex.bde.org> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: owner-freebsd-audit@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG 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 majordomo@FreeBSD.org with "unsubscribe freebsd-audit" in the body of the message