Date: Thu, 9 Nov 2006 20:33:42 +0100 From: Ed Schouten <ed@fxq.nl> To: FreeBSD Hackers <freebsd-hackers@freebsd.org> Subject: Re: [Patch] sys/kern/kern_descrip.c: remove double limit check in fcntl() Message-ID: <20061109193342.GC16100@hoeg.nl> In-Reply-To: <20061109124119.GB16100@hoeg.nl> References: <20061109124119.GB16100@hoeg.nl>
next in thread | previous in thread | raw e-mail | index | archive | help
--Y5rl02BVI9TCfPar
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable
* Ed Schouten <ed@fxq.nl> 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 <ed@fxq.nl>
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--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20061109193342.GC16100>