The following diff makes finishdup() release the file descriptor table
lock before calling closef(). This makes the order of operations similar
to that of fdrelease().
The dup* code still has an issue with file closing that this diff does
not address. If fdalloc() fails in dodup3(), sys_dup() or sys_fcntl(),
the code releases the reference to the file while still holding fdplock.
The reference is acquired before locking the fdp, so another thread
might have closed the file descriptor and the current thread now holds
the last file reference. In that case the FRELE() invokes the file
close path.
OK?
Index: kern/kern_descrip.c
===================================================================
RCS file: src/sys/kern/kern_descrip.c,v
retrieving revision 1.199
diff -u -p -r1.199 kern_descrip.c
--- kern/kern_descrip.c 18 Feb 2020 03:47:18 -0000 1.199
+++ kern/kern_descrip.c 18 Feb 2020 16:29:51 -0000
@@ -307,14 +307,11 @@ restart:
fdpunlock(fdp);
goto restart;
}
- goto out;
+ fdpunlock(fdp);
+ return (error);
}
/* No need for FRELE(), finishdup() uses current ref. */
- error = finishdup(p, fp, old, new, retval, 0);
-
-out:
- fdpunlock(fdp);
- return (error);
+ return (finishdup(p, fp, old, new, retval, 0));
}
/*
@@ -382,7 +379,8 @@ restart:
fdpunlock(fdp);
goto restart;
}
- goto out;
+ fdpunlock(fdp);
+ return (error);
}
if (new != i)
panic("dup2: fdalloc");
@@ -394,11 +392,7 @@ restart:
dupflags |= DUPF_CLOEXEC;
/* No need for FRELE(), finishdup() uses current ref. */
- error = finishdup(p, fp, old, new, retval, dupflags);
-
-out:
- fdpunlock(fdp);
- return (error);
+ return (finishdup(p, fp, old, new, retval, dupflags));
}
/*
@@ -445,6 +439,7 @@ restart:
fdpunlock(fdp);
goto restart;
}
+ fdpunlock(fdp);
} else {
int dupflags = 0;
@@ -454,8 +449,6 @@ restart:
/* No need for FRELE(), finishdup() uses current ref. */
error = finishdup(p, fp, fd, i, retval, dupflags);
}
-
- fdpunlock(fdp);
return (error);
case F_GETFD:
@@ -657,23 +650,24 @@ finishdup(struct proc *p, struct file *f
{
struct file *oldfp;
struct filedesc *fdp = p->p_fd;
+ int error;
fdpassertlocked(fdp);
KASSERT(fp->f_iflags & FIF_INSERTED);
if (fp->f_count >= FDUP_MAX_COUNT) {
- FRELE(fp, p);
- return (EDEADLK);
+ error = EDEADLK;
+ goto fail;
}
oldfp = fd_getfile(fdp, new);
if ((dupflags & DUPF_DUP2) && oldfp == NULL) {
if (fd_inuse(fdp, new)) {
- FRELE(fp, p);
- return (EBUSY);
- }
+ error = EBUSY;
+ goto fail;
+ }
fd_used(fdp, new);
- }
+ }
/*
* Use `fd_fplock' to synchronize with fd_getfile() so that
@@ -690,10 +684,18 @@ finishdup(struct proc *p, struct file *f
if (oldfp != NULL) {
knote_fdclose(p, new);
+ fdpunlock(fdp);
closef(oldfp, p);
+ } else {
+ fdpunlock(fdp);
}
return (0);
+
+fail:
+ fdpunlock(fdp);
+ FRELE(fp, p);
+ return (error);
}
void
