aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorMark Johnston <markj@FreeBSD.org>2023-03-22 12:52:57 +0000
committerMark Johnston <markj@FreeBSD.org>2023-04-05 14:29:29 +0000
commitcfae554b786636c3d2c8fb96c3804941a18245b3 (patch)
tree92928d506900d167d794bf1a7d999e1db7f6583d
parente85ca03254e0b93055919406285bc66c27114b64 (diff)
downloadsrc-cfae554b786636c3d2c8fb96c3804941a18245b3.tar.gz
src-cfae554b786636c3d2c8fb96c3804941a18245b3.zip
fdescfs: Fix a file ref leak
In fdesc_lookup(), vn_vget_ino_gen() may fail without invoking the callback, in which case the ref on fp is leaked. This happens if the fdescfs mount is being concurrently unmounted. Moreover, we cannot safely drop the ref while the dvp is locked. So: - Use a flag variable to indicate whether the ref is dropped. - Reorganize things to handle the leak. Reported by: C Turt <ecturt@gmail.com> Reviewed by: mjg, kib Tested by: pho MFC after: 2 weeks Sponsored by: The FreeBSD Foundation Differential Revision: https://reviews.freebsd.org/D39189 (cherry picked from commit 0f5b6f9a041e9cca3b376f6ec909374938887a3b)
-rw-r--r--sys/fs/fdescfs/fdesc_vnops.c42
1 files changed, 25 insertions, 17 deletions
diff --git a/sys/fs/fdescfs/fdesc_vnops.c b/sys/fs/fdescfs/fdesc_vnops.c
index 17320b2c8354..81b6690efc80 100644
--- a/sys/fs/fdescfs/fdesc_vnops.c
+++ b/sys/fs/fdescfs/fdesc_vnops.c
@@ -258,6 +258,7 @@ struct fdesc_get_ino_args {
int ix;
struct file *fp;
struct thread *td;
+ bool fdropped;
};
static int
@@ -280,6 +281,7 @@ fdesc_get_ino_alloc(struct mount *mp, void *arg, int lkflags,
error = fdesc_allocvp(a->ftype, a->fd_fd, a->ix, mp, rvp);
}
fdrop(a->fp, a->td);
+ a->fdropped = true;
return (error);
}
@@ -300,6 +302,7 @@ fdesc_lookup(struct vop_lookup_args *ap)
int nlen = cnp->cn_namelen;
u_int fd, fd1;
int error;
+ bool fdropped;
struct vnode *fvp;
if ((cnp->cn_flags & ISLASTCN) &&
@@ -343,24 +346,10 @@ fdesc_lookup(struct vop_lookup_args *ap)
*/
if ((error = fget(td, fd, &cap_no_rights, &fp)) != 0)
goto bad;
+ fdropped = false;
- /* Check if we're looking up ourselves. */
- if (VTOFDESC(dvp)->fd_ix == FD_DESC + fd) {
- /*
- * In case we're holding the last reference to the file, the dvp
- * will be re-acquired.
- */
- vhold(dvp);
- VOP_UNLOCK(dvp);
- fdrop(fp, td);
-
- /* Re-aquire the lock afterwards. */
- vn_lock(dvp, LK_RETRY | LK_EXCLUSIVE);
- vdrop(dvp);
- fvp = dvp;
- if (VN_IS_DOOMED(dvp))
- error = ENOENT;
- } else {
+ /* Make sure we're not looking up the dvp itself. */
+ if (VTOFDESC(dvp)->fd_ix != FD_DESC + fd) {
/*
* Unlock our root node (dvp) when doing this, since we might
* deadlock since the vnode might be locked by another thread
@@ -374,8 +363,27 @@ fdesc_lookup(struct vop_lookup_args *ap)
arg.ix = FD_DESC + fd;
arg.fp = fp;
arg.td = td;
+ arg.fdropped = fdropped;
error = vn_vget_ino_gen(dvp, fdesc_get_ino_alloc, &arg,
LK_EXCLUSIVE, &fvp);
+ fdropped = arg.fdropped;
+ }
+
+ if (!fdropped) {
+ /*
+ * In case we're holding the last reference to the file, the dvp
+ * will be re-acquired.
+ */
+ vhold(dvp);
+ VOP_UNLOCK(dvp);
+ fdrop(fp, td);
+ fdropped = true;
+
+ vn_lock(dvp, LK_RETRY | LK_EXCLUSIVE);
+ vdrop(dvp);
+ fvp = dvp;
+ if (error == 0 && VN_IS_DOOMED(dvp))
+ error = ENOENT;
}
if (error)