Date: Mon, 25 Nov 2002 23:07:53 +1100 (EST) From: Bruce Evans <bde@zeta.org.au> To: Olivier Houchard <cognet@ci0.org> Cc: freebsd-audit@FreeBSD.ORG Subject: Re: do_dup patch Message-ID: <20021125225927.O56791-100000@gamplex.bde.org> In-Reply-To: <20021125110902.GA10961@ci0.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20021125225927.O56791-100000>