From owner-freebsd-hackers@FreeBSD.ORG Thu Nov 9 19:34:10 2006 Return-Path: X-Original-To: freebsd-hackers@freebsd.org Delivered-To: freebsd-hackers@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 89DBD16A4AB for ; Thu, 9 Nov 2006 19:34:10 +0000 (UTC) (envelope-from ed@hoeg.nl) Received: from palm.hoeg.nl (palm.hoeg.nl [83.98.131.212]) by mx1.FreeBSD.org (Postfix) with ESMTP id BD9D943D7C for ; Thu, 9 Nov 2006 19:33:44 +0000 (GMT) (envelope-from ed@hoeg.nl) Received: by palm.hoeg.nl (Postfix, from userid 1000) id 92A051CEB3; Thu, 9 Nov 2006 20:33:42 +0100 (CET) Date: Thu, 9 Nov 2006 20:33:42 +0100 From: Ed Schouten To: FreeBSD Hackers Message-ID: <20061109193342.GC16100@hoeg.nl> References: <20061109124119.GB16100@hoeg.nl> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="Y5rl02BVI9TCfPar" Content-Disposition: inline In-Reply-To: <20061109124119.GB16100@hoeg.nl> User-Agent: Mutt/1.5.13 (2006-08-11) X-Mailman-Approved-At: Thu, 09 Nov 2006 20:02:54 +0000 Subject: Re: [Patch] sys/kern/kern_descrip.c: remove double limit check in fcntl() X-BeenThere: freebsd-hackers@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Technical Discussions relating to FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 09 Nov 2006 19:34:10 -0000 --Y5rl02BVI9TCfPar Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable * Ed Schouten wrote: > The patch below prevents this by performing this check by do_dup(). It > will prevent fcntl() from PROC_LOCK()'ing twice. It also fixes the > return value of fcntl(). The manual page states that it should return > EMFILE when it exceeds its limit, though the actual code sets EINVAL. Woops - It looks like I wasn't awake when I was reading the fcntl() manual page. fcntl() should return EINVAL when the minimum value is higher than the limit and EMFILE when it can't find a free descriptor in the range from the minimum value to the maximum. dup() generalizes this to EMFILE. It cannot return EINVAL. I decided to change the patch that do_dup() can return EINVAL for the fcntl() scenario and that the dup()/dup2() system calls readjust the errno right before returning. Please use the patch below. %%% --- src/sys/kern/kern_descrip.c Thu Nov 9 20:18:41 2006 +++ src/sys/kern/kern_descrip.c Thu Nov 9 20:26:25 2006 @@ -283,8 +283,14 @@ dup2(struct thread *td, struct dup2_args *uap) { =20 - return (do_dup(td, DUP_FIXED, (int)uap->from, (int)uap->to, - td->td_retval)); + int error; + + error =3D do_dup(td, DUP_FIXED, (int)uap->from, (int)uap->to, + td->td_retval); + /* dup2() should only return EMFILE when exceeding limits */ + if (error =3D=3D EINVAL) + error =3D EMFILE; + return (error); } =20 /* @@ -302,8 +308,13 @@ int dup(struct thread *td, struct dup_args *uap) { + int error; =20 - return (do_dup(td, DUP_VARIABLE, (int)uap->fd, 0, td->td_retval)); + error =3D do_dup(td, DUP_VARIABLE, (int)uap->fd, 0, td->td_retval); + /* dup() should only return EMFILE when exceeding limits */ + if (error =3D=3D EINVAL) + error =3D EMFILE; + return (error); } =20 /* @@ -358,7 +369,6 @@ struct proc *p; char *pop; struct vnode *vp; - u_int newmin; int error, flg, tmp; int giant_locked; =20 @@ -396,16 +406,7 @@ case F_DUPFD: /* mtx_assert(&Giant, MA_NOTOWNED); */ FILEDESC_UNLOCK(fdp); - newmin =3D arg; - PROC_LOCK(p); - if (newmin >=3D lim_cur(p, RLIMIT_NOFILE) || - newmin >=3D maxfilesperproc) { - PROC_UNLOCK(p); - error =3D EINVAL; - break; - } - PROC_UNLOCK(p); - error =3D do_dup(td, DUP_VARIABLE, fd, newmin, td->td_retval); + error =3D do_dup(td, DUP_VARIABLE, fd, arg, td->td_retval); break; =20 case F_GETFD: @@ -629,7 +630,7 @@ maxfd =3D min((int)lim_cur(p, RLIMIT_NOFILE), maxfilesperproc); PROC_UNLOCK(p); if (new >=3D maxfd) - return (EMFILE); + return (EINVAL); =20 FILEDESC_LOCK(fdp); if (old >=3D fdp->fd_nfiles || fdp->fd_ofiles[old] =3D=3D NULL) { %%% --=20 Ed Schouten WWW: http://g-rave.nl/ --Y5rl02BVI9TCfPar Content-Type: application/pgp-signature Content-Disposition: inline -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.5 (FreeBSD) iD8DBQFFU4KW52SDGA2eCwURAmJgAJ9UpJDaGjCsgHLkzrz6rApKS1PplwCfaqNA trhr/xIwPaVAZE2BydCP9BE= =NEMo -----END PGP SIGNATURE----- --Y5rl02BVI9TCfPar--