Struct file has reference count "f_count" field. When f_count > 0 file instance
exists, when f_count becomes 0 file instance should be removed. It's simple
but some strange magic exists. When file was created by falloc() it has
f_count == 2. One additional FREF() call was done inside falloc(). After
file was setupped, FILE_SET_MATURE() call do FRELE() and f_count becomes == 1.
Also closef() function exists. It wants f_count be >= 2 otherwise it panics.
closef() caller bumps f_count before closef() call just for prevents this
panic. I found this behaviour strange, and I described this before as "Strange
magic with f_count...". Ok, this explanation sucks :) so I dug into cvs
history.
Before sys/sys/file.h rev 1.15 struct file has only one reference counter
f_count. It was used sometimes by direct "fp->f_count++" or "--fp->f_count"
operations. In sys/sys/file.h rev 1.15 another reference counter "f_usecount"
wa added to struct file. Also new macro "FILE_USE()" and "FILE_UNUSE()" were
added for increase and decrease file reference count and they used
"f_usecount". Now, those macroses known as "FREF()" and "FRELE()". So, after
sys/sys/file.h rev 1.15 struct file has two reference counters and they are
both used up to sys/sys/file.h rev 1.30 where "f_usecount" was removed, but
not completely. Within FREF() and FRELE() "f_usecount" was just replaced by
"f_count" and existed old fp->f_count increments and decrements haven't be
removed. So after sys/sys/file.h rev 1.30 there are some places where f_count
is bumped twice and decremented twice and other related code has hacks for this
condition.
Now, the legacy of "f_usecount" coexists with "f_count" and I think it
should be fixed. I did the diff for this. I did system rebuild on diskless
NFS machine without panics, and kern.nfiles before rebuild is equal to
kern.nfiles after rebuild. So looks like there is no f_count corruption
and file instance leaks. In this diff falloc() returns file instance with
f_count == 1 so FRELE() call removed from FILE_SET_MATURE(). "struct proc *"
arg removed from FILE_SET_MATURE() because it is unnecessary. closef() wants
f_count >= 1 and closef() callers don't do additional bump on f_count.
Really, I think this diff doesn't fix anything :) The code has some places
with direct "fp->f_count++" and "--fp->f_count" operations, the code has some
places with unreferenced fp instances and the code has some places where
FREF() calls on fp instance were made wrong. I think the whole "f_count"
related stuff should be refactored but even the solution described above
should not be the first step. I think, the right file instance acquisition
(f_count increment) and direct "f_count" modification cleanups should be done
before.
Index: sys/dev/systrace.c
===================================================================
RCS file: /home/cvsync/openbsd-cvs/src/sys/dev/systrace.c,v
retrieving revision 1.75
diff -u -p -r1.75 systrace.c
--- sys/dev/systrace.c 14 Mar 2015 03:38:46 -0000 1.75
+++ sys/dev/systrace.c 5 May 2015 22:37:31 -0000
@@ -518,7 +518,7 @@ systraceioctl(dev_t dev, u_long cmd, cad
f->f_ops = &systracefops;
f->f_data = (caddr_t) fst;
*(int *)data = fd;
- FILE_SET_MATURE(f, p);
+ FILE_SET_MATURE(f);
break;
default:
error = EINVAL;
Index: sys/kern/exec_script.c
===================================================================
RCS file: /home/cvsync/openbsd-cvs/src/sys/kern/exec_script.c,v
retrieving revision 1.35
diff -u -p -r1.35 exec_script.c
--- sys/kern/exec_script.c 14 Mar 2015 03:38:50 -0000 1.35
+++ sys/kern/exec_script.c 5 May 2015 22:37:31 -0000
@@ -185,7 +185,7 @@ check_shell:
fp->f_ops = &vnops;
fp->f_data = (caddr_t) scriptvp;
fp->f_flag = FREAD;
- FILE_SET_MATURE(fp, p);
+ FILE_SET_MATURE(fp);
}
/* set up the parameters for the recursive check_exec() call */
Index: sys/kern/kern_descrip.c
===================================================================
RCS file: /home/cvsync/openbsd-cvs/src/sys/kern/kern_descrip.c,v
retrieving revision 1.119
diff -u -p -r1.119 kern_descrip.c
--- sys/kern/kern_descrip.c 30 Apr 2015 21:18:45 -0000 1.119
+++ sys/kern/kern_descrip.c 5 May 2015 22:37:31 -0000
@@ -576,8 +576,6 @@ finishdup(struct proc *p, struct file *f
* closef can deal with that.
*/
oldfp = fdp->fd_ofiles[new];
- if (oldfp != NULL)
- FREF(oldfp);
fdp->fd_ofiles[new] = fp;
fdp->fd_ofileflags[new] = fdp->fd_ofileflags[old] & ~UF_EXCLOSE;
@@ -620,7 +618,6 @@ fdrelease(struct proc *p, int fd)
fp = *fpp;
if (fp == NULL)
return (EBADF);
- FREF(fp);
*fpp = NULL;
fd_unused(fdp, fd);
if (fd < fdp->fd_knlistsize)
@@ -897,7 +894,6 @@ restart:
*resultfp = fp;
if (resultfd)
*resultfd = i;
- FREF(fp);
return (0);
}
@@ -1047,7 +1043,6 @@ fdfree(struct proc *p)
for (i = fdp->fd_lastfile; i >= 0; i--, fpp++) {
fp = *fpp;
if (fp != NULL) {
- FREF(fp);
*fpp = NULL;
(void) closef(fp, p);
}
@@ -1087,10 +1082,9 @@ closef(struct file *fp, struct proc *p)
return (0);
#ifdef DIAGNOSTIC
- if (fp->f_count < 2)
- panic("closef: count (%ld) < 2", fp->f_count);
+ if (fp->f_count < 1)
+ panic("closef: count (%ld) < 1", fp->f_count);
#endif
- fp->f_count--;
/*
* POSIX record locking dictates that any close releases ALL
Index: sys/kern/kern_event.c
===================================================================
RCS file: /home/cvsync/openbsd-cvs/src/sys/kern/kern_event.c,v
retrieving revision 1.61
diff -u -p -r1.61 kern_event.c
--- sys/kern/kern_event.c 19 Dec 2014 05:59:21 -0000 1.61
+++ sys/kern/kern_event.c 5 May 2015 22:37:31 -0000
@@ -458,7 +458,7 @@ sys_kqueue(struct proc *p, void *v, regi
if (fdp->fd_knlistsize < 0)
fdp->fd_knlistsize = 0; /* this process has a kq */
kq->kq_fdp = fdp;
- FILE_SET_MATURE(fp, p);
+ FILE_SET_MATURE(fp);
return (0);
}
Index: sys/kern/kern_exec.c
===================================================================
RCS file: /home/cvsync/openbsd-cvs/src/sys/kern/kern_exec.c,v
retrieving revision 1.161
diff -u -p -r1.161 kern_exec.c
--- sys/kern/kern_exec.c 14 Mar 2015 03:38:50 -0000 1.161
+++ sys/kern/kern_exec.c 5 May 2015 22:37:31 -0000
@@ -605,7 +605,7 @@ sys_execve(struct proc *p, void *v, regi
fp->f_type = DTYPE_VNODE;
fp->f_ops = &vnops;
fp->f_data = (caddr_t)vp;
- FILE_SET_MATURE(fp, p);
+ FILE_SET_MATURE(fp);
}
}
fdpunlock(p->p_fd);
Index: sys/kern/sys_pipe.c
===================================================================
RCS file: /home/cvsync/openbsd-cvs/src/sys/kern/sys_pipe.c,v
retrieving revision 1.69
diff -u -p -r1.69 sys_pipe.c
--- sys/kern/sys_pipe.c 10 Feb 2015 21:56:10 -0000 1.69
+++ sys/kern/sys_pipe.c 5 May 2015 22:37:31 -0000
@@ -167,8 +167,8 @@ dopipe(struct proc *p, int *ufds, int fl
rpipe->pipe_peer = wpipe;
wpipe->pipe_peer = rpipe;
- FILE_SET_MATURE(rf, p);
- FILE_SET_MATURE(wf, p);
+ FILE_SET_MATURE(rf);
+ FILE_SET_MATURE(wf);
error = copyout(fds, ufds, sizeof(fds));
if (error != 0) {
Index: sys/kern/tty_pty.c
===================================================================
RCS file: /home/cvsync/openbsd-cvs/src/sys/kern/tty_pty.c,v
retrieving revision 1.70
diff -u -p -r1.70 tty_pty.c
--- sys/kern/tty_pty.c 10 Feb 2015 21:56:10 -0000 1.70
+++ sys/kern/tty_pty.c 5 May 2015 22:37:31 -0000
@@ -1176,8 +1176,8 @@ retry:
memcpy(ptm->sn, pti->pty_sn, sizeof(pti->pty_sn));
/* mark the files mature now that we've passed all errors */
- FILE_SET_MATURE(cfp, p);
- FILE_SET_MATURE(sfp, p);
+ FILE_SET_MATURE(cfp);
+ FILE_SET_MATURE(sfp);
fdpunlock(fdp);
break;
Index: sys/kern/uipc_syscalls.c
===================================================================
RCS file: /home/cvsync/openbsd-cvs/src/sys/kern/uipc_syscalls.c,v
retrieving revision 1.100
diff -u -p -r1.100 uipc_syscalls.c
--- sys/kern/uipc_syscalls.c 14 Mar 2015 03:38:51 -0000 1.100
+++ sys/kern/uipc_syscalls.c 5 May 2015 22:37:31 -0000
@@ -100,7 +100,7 @@ sys_socket(struct proc *p, void *v, regi
fp->f_data = so;
if (type & SOCK_NONBLOCK)
(*fp->f_ops->fo_ioctl)(fp, FIONBIO, (caddr_t)&type, p);
- FILE_SET_MATURE(fp, p);
+ FILE_SET_MATURE(fp);
*retval = fd;
}
out:
@@ -291,7 +291,7 @@ redo:
fdpunlock(fdp);
} else {
(*fp->f_ops->fo_ioctl)(fp, FIONBIO, (caddr_t)&nflag, p);
- FILE_SET_MATURE(fp, p);
+ FILE_SET_MATURE(fp);
*retval = tmpfd;
}
m_freem(nam);
@@ -418,8 +418,8 @@ sys_socketpair(struct proc *p, void *v,
(*fp2->f_ops->fo_ioctl)(fp2, FIONBIO, (caddr_t)&flags,
p);
}
- FILE_SET_MATURE(fp1, p);
- FILE_SET_MATURE(fp2, p);
+ FILE_SET_MATURE(fp1);
+ FILE_SET_MATURE(fp2);
fdpunlock(fdp);
return (0);
}
Index: sys/kern/uipc_usrreq.c
===================================================================
RCS file: /home/cvsync/openbsd-cvs/src/sys/kern/uipc_usrreq.c,v
retrieving revision 1.80
diff -u -p -r1.80 uipc_usrreq.c
--- sys/kern/uipc_usrreq.c 28 Mar 2015 23:50:55 -0000 1.80
+++ sys/kern/uipc_usrreq.c 5 May 2015 22:37:31 -0000
@@ -962,7 +962,6 @@ unp_gc(void)
*fpp++ = fp;
nunref++;
FREF(fp);
- fp->f_count++;
}
}
for (i = nunref, fpp = extra_ref; --i >= 0; ++fpp)
@@ -1039,7 +1038,6 @@ unp_discard(struct file *fp)
if (fp == NULL)
return;
- FREF(fp);
fp->f_msgcount--;
unp_rights--;
(void) closef(fp, NULL);
Index: sys/kern/vfs_syscalls.c
===================================================================
RCS file: /home/cvsync/openbsd-cvs/src/sys/kern/vfs_syscalls.c,v
retrieving revision 1.219
diff -u -p -r1.219 vfs_syscalls.c
--- sys/kern/vfs_syscalls.c 30 Apr 2015 09:20:51 -0000 1.219
+++ sys/kern/vfs_syscalls.c 5 May 2015 22:37:31 -0000
@@ -924,7 +924,7 @@ doopenat(struct proc *p, int fd, const c
}
VOP_UNLOCK(vp, 0, p);
*retval = indx;
- FILE_SET_MATURE(fp, p);
+ FILE_SET_MATURE(fp);
out:
fdpunlock(fdp);
return (error);
@@ -1086,7 +1086,7 @@ sys_fhopen(struct proc *p, void *v, regi
}
VOP_UNLOCK(vp, 0, p);
*retval = indx;
- FILE_SET_MATURE(fp, p);
+ FILE_SET_MATURE(fp);
fdpunlock(fdp);
return (0);
Index: sys/nfs/nfs_syscalls.c
===================================================================
RCS file: /home/cvsync/openbsd-cvs/src/sys/nfs/nfs_syscalls.c,v
retrieving revision 1.99
diff -u -p -r1.99 nfs_syscalls.c
--- sys/nfs/nfs_syscalls.c 14 Mar 2015 03:38:52 -0000 1.99
+++ sys/nfs/nfs_syscalls.c 5 May 2015 22:37:31 -0000
@@ -478,7 +478,6 @@ nfsrv_zapsock(struct nfssvc_sock *slp)
slp->ns_flag &= ~SLP_ALLFLAGS;
fp = slp->ns_fp;
if (fp) {
- FREF(fp);
slp->ns_fp = NULL;
so = slp->ns_so;
so->so_upcall = NULL;
Index: sys/sys/file.h
===================================================================
RCS file: /home/cvsync/openbsd-cvs/src/sys/sys/file.h,v
retrieving revision 1.34
diff -u -p -r1.34 file.h
--- sys/sys/file.h 18 Nov 2014 15:16:35 -0000 1.34
+++ sys/sys/file.h 5 May 2015 22:37:31 -0000
@@ -96,9 +96,8 @@ struct file {
#define FREF(fp) do { (fp)->f_count++; } while (0)
#define FRELE(fp,p) (--(fp)->f_count == 0 ? fdrop(fp, p) : 0)
-#define FILE_SET_MATURE(fp,p) do { \
+#define FILE_SET_MATURE(fp) do { \
(fp)->f_iflags &= ~FIF_LARVAL; \
- FRELE(fp, p); \
} while (0)
int fdrop(struct file *, struct proc *);
