Re: [Patch] sys/kern/kern_descrip.c: remove double limit check in fcntl()

[Ed Schouten]

* Ed Schouten <[EMAIL PROTECTED]> 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)
 {
 
-       return (do_dup(td, DUP_FIXED, (int)uap->from, (int)uap->to,
-                   td->td_retval));
+       int error;
+
+       error = do_dup(td, DUP_FIXED, (int)uap->from, (int)uap->to,
+                   td->td_retval);
+       /* dup2() should only return EMFILE when exceeding limits */
+       if (error == EINVAL)
+               error = EMFILE;
+       return (error);
 }
 
 /*
@@ -302,8 +308,13 @@
 int
 dup(struct thread *td, struct dup_args *uap)
 {
+       int error;
 
-       return (do_dup(td, DUP_VARIABLE, (int)uap->fd, 0, td->td_retval));
+       error = do_dup(td, DUP_VARIABLE, (int)uap->fd, 0, td->td_retval);
+       /* dup() should only return EMFILE when exceeding limits */
+       if (error == EINVAL)
+               error = EMFILE;
+       return (error);
 }
 
 /*
@@ -358,7 +369,6 @@
        struct proc *p;
        char *pop;
        struct vnode *vp;
-       u_int newmin;
        int error, flg, tmp;
        int giant_locked;
 
@@ -396,16 +406,7 @@
        case F_DUPFD:
                /* mtx_assert(&Giant, MA_NOTOWNED); */
                FILEDESC_UNLOCK(fdp);
-               newmin = arg;
-               PROC_LOCK(p);
-               if (newmin >= lim_cur(p, RLIMIT_NOFILE) ||
-                   newmin >= maxfilesperproc) {
-                       PROC_UNLOCK(p);
-                       error = EINVAL;
-                       break;
-               }
-               PROC_UNLOCK(p);
-               error = do_dup(td, DUP_VARIABLE, fd, newmin, td->td_retval);
+               error = do_dup(td, DUP_VARIABLE, fd, arg, td->td_retval);
                break;
 
        case F_GETFD:
@@ -629,7 +630,7 @@
        maxfd = min((int)lim_cur(p, RLIMIT_NOFILE), maxfilesperproc);
        PROC_UNLOCK(p);
        if (new >= maxfd)
-               return (EMFILE);
+               return (EINVAL);
 
        FILEDESC_LOCK(fdp);
        if (old >= fdp->fd_nfiles || fdp->fd_ofiles[old] == NULL) {
%%%

-- 
 Ed Schouten <[EMAIL PROTECTED]>
 WWW: http://g-rave.nl/

pgpqP8VPzFHz8.pgp
Description: PGP signature