The closing of a file can be a complex operation, and hence it would be
good to hold as few locks as possible when calling closef(). In
particular, the code behind close(2) already releases the file
descriptor table lock before closef(). This might not be strictly
necessary when the type of the file is known beforehand. However,
general file descriptor operations should be careful.
The following diff moves the setting of the UF_EXCLOSE fd flag inside
finishdup(). This makes possible to release fdplock before closef() in
finishdup() in a subsequent patch.
This changes the order of setting UF_EXCLOSE relative to
knote_fdclose(). However, that should not be a problem because the flag
concerns file descriptor handling and not kqueue.
OK?
Index: kern/kern_descrip.c
===================================================================
RCS file: src/sys/kern/kern_descrip.c,v
retrieving revision 1.198
diff -u -p -r1.198 kern_descrip.c
--- kern/kern_descrip.c 5 Feb 2020 17:03:13 -0000 1.198
+++ kern/kern_descrip.c 16 Feb 2020 16:56:14 -0000
@@ -82,6 +82,9 @@ int finishdup(struct proc *, struct file
int find_last_set(struct filedesc *, int);
int dodup3(struct proc *, int, int, int, register_t *);
+#define DUPF_CLOEXEC 0x01
+#define DUPF_DUP2 0x02
+
struct pool file_pool;
struct pool fdesc_pool;
@@ -350,7 +353,7 @@ dodup3(struct proc *p, int old, int new,
{
struct filedesc *fdp = p->p_fd;
struct file *fp;
- int i, error;
+ int dupflags, error, i;
restart:
if ((fp = fd_getfile(fdp, old)) == NULL)
@@ -385,10 +388,13 @@ restart:
panic("dup2: fdalloc");
fd_unused(fdp, new);
}
+
+ dupflags = DUPF_DUP2;
+ if (flags & O_CLOEXEC)
+ dupflags |= DUPF_CLOEXEC;
+
/* No need for FRELE(), finishdup() uses current ref. */
- error = finishdup(p, fp, old, new, retval, 1);
- if (!error && flags & O_CLOEXEC)
- fdp->fd_ofileflags[new] |= UF_EXCLOSE;
+ error = finishdup(p, fp, old, new, retval, dupflags);
out:
fdpunlock(fdp);
@@ -440,11 +446,13 @@ restart:
goto restart;
}
} else {
- /* No need for FRELE(), finishdup() uses current ref. */
- error = finishdup(p, fp, fd, i, retval, 0);
+ int dupflags = 0;
+
+ if (SCARG(uap, cmd) == F_DUPFD_CLOEXEC)
+ dupflags |= DUPF_CLOEXEC;
- if (!error && SCARG(uap, cmd) == F_DUPFD_CLOEXEC)
- fdp->fd_ofileflags[i] |= UF_EXCLOSE;
+ /* No need for FRELE(), finishdup() uses current ref. */
+ error = finishdup(p, fp, fd, i, retval, dupflags);
}
fdpunlock(fdp);
@@ -645,7 +653,7 @@ out:
*/
int
finishdup(struct proc *p, struct file *fp, int old, int new,
- register_t *retval, int dup2)
+ register_t *retval, int dupflags)
{
struct file *oldfp;
struct filedesc *fdp = p->p_fd;
@@ -659,7 +667,7 @@ finishdup(struct proc *p, struct file *f
}
oldfp = fd_getfile(fdp, new);
- if (dup2 && oldfp == NULL) {
+ if ((dupflags & DUPF_DUP2) && oldfp == NULL) {
if (fd_inuse(fdp, new)) {
FRELE(fp, p);
return (EBUSY);
@@ -676,6 +684,8 @@ finishdup(struct proc *p, struct file *f
mtx_leave(&fdp->fd_fplock);
fdp->fd_ofileflags[new] = fdp->fd_ofileflags[old] & ~UF_EXCLOSE;
+ if (dupflags & DUPF_CLOEXEC)
+ fdp->fd_ofileflags[new] |= UF_EXCLOSE;
*retval = new;
if (oldfp != NULL) {
