Skip site navigation (1)Skip section navigation (2)
Date:      Sat, 11 Jul 2015 01:08:04 +0200
From:      Mateusz Guzik <mjguzik@gmail.com>
To:        kib@freebsd.org
Cc:        freebsd-fs@freebsd.org
Subject:   [PATCH 2/2] Create a dedicated function for ensuring that cdir and rdir are populated.
Message-ID:  <1436569684-3939-3-git-send-email-mjguzik@gmail.com>
In-Reply-To: <1436569684-3939-1-git-send-email-mjguzik@gmail.com>
References:  <1436569684-3939-1-git-send-email-mjguzik@gmail.com>

next in thread | previous in thread | raw e-mail | index | archive | help
From: Mateusz Guzik <mjg@freebsd.org>

Previously several places were doing it on its own, partially
incorrectly (e.g. without the filedesc locked) or even actively harmful
by assigning rootvnode without vreling it or populating jdir.

This functionality should not exist and will be garbage collected after
all callers are properly reviewed.
---
 sys/cam/ctl/ctl_backend_block.c                     | 13 +------------
 sys/cddl/compat/opensolaris/kern/opensolaris_kobj.c | 13 +------------
 sys/cddl/compat/opensolaris/sys/vnode.h             | 13 +------------
 sys/compat/ndis/subr_ndis.c                         |  5 +----
 sys/dev/xen/blkback/blkback.c                       | 13 +------------
 sys/kern/kern_descrip.c                             | 19 +++++++++++++++++++
 sys/kern/subr_firmware.c                            | 13 +------------
 sys/sys/filedesc.h                                  |  1 +
 8 files changed, 26 insertions(+), 64 deletions(-)

diff --git a/sys/cam/ctl/ctl_backend_block.c b/sys/cam/ctl/ctl_backend_block.c
index c56023b..8ea52aa 100644
--- a/sys/cam/ctl/ctl_backend_block.c
+++ b/sys/cam/ctl/ctl_backend_block.c
@@ -2123,18 +2123,7 @@ ctl_be_block_open(struct ctl_be_block_softc *softc,
 		return (1);
 	}
 
-	if (!curthread->td_proc->p_fd->fd_cdir) {
-		curthread->td_proc->p_fd->fd_cdir = rootvnode;
-		VREF(rootvnode);
-	}
-	if (!curthread->td_proc->p_fd->fd_rdir) {
-		curthread->td_proc->p_fd->fd_rdir = rootvnode;
-		VREF(rootvnode);
-	}
-	if (!curthread->td_proc->p_fd->fd_jdir) {
-		curthread->td_proc->p_fd->fd_jdir = rootvnode;
-		VREF(rootvnode);
-	}
+	pwd_ensure_dirs();
 
  again:
 	NDINIT(&nd, LOOKUP, FOLLOW, UIO_SYSSPACE, be_lun->dev_path, curthread);
diff --git a/sys/cddl/compat/opensolaris/kern/opensolaris_kobj.c b/sys/cddl/compat/opensolaris/kern/opensolaris_kobj.c
index 9ff798a..52d695b 100644
--- a/sys/cddl/compat/opensolaris/kern/opensolaris_kobj.c
+++ b/sys/cddl/compat/opensolaris/kern/opensolaris_kobj.c
@@ -67,21 +67,10 @@ static void *
 kobj_open_file_vnode(const char *file)
 {
 	struct thread *td = curthread;
-	struct filedesc *fd;
 	struct nameidata nd;
 	int error, flags;
 
-	fd = td->td_proc->p_fd;
-	FILEDESC_XLOCK(fd);
-	if (fd->fd_rdir == NULL) {
-		fd->fd_rdir = rootvnode;
-		vref(fd->fd_rdir);
-	}
-	if (fd->fd_cdir == NULL) {
-		fd->fd_cdir = rootvnode;
-		vref(fd->fd_cdir);
-	}
-	FILEDESC_XUNLOCK(fd);
+	pwd_ensure_dirs();
 
 	flags = FREAD | O_NOFOLLOW;
 	NDINIT(&nd, LOOKUP, 0, UIO_SYSSPACE, file, td);
diff --git a/sys/cddl/compat/opensolaris/sys/vnode.h b/sys/cddl/compat/opensolaris/sys/vnode.h
index 22256cf..d7bc7f7 100644
--- a/sys/cddl/compat/opensolaris/sys/vnode.h
+++ b/sys/cddl/compat/opensolaris/sys/vnode.h
@@ -162,7 +162,6 @@ vn_openat(char *pnamep, enum uio_seg seg, int filemode, int createmode,
     int fd)
 {
 	struct thread *td = curthread;
-	struct filedesc *fdc;
 	struct nameidata nd;
 	int error, operation;
 
@@ -179,17 +178,7 @@ vn_openat(char *pnamep, enum uio_seg seg, int filemode, int createmode,
 	}
 	ASSERT(umask == 0);
 
-	fdc = td->td_proc->p_fd;
-	FILEDESC_XLOCK(fdc);
-	if (fdc->fd_rdir == NULL) {
-		fdc->fd_rdir = rootvnode;
-		vref(fdc->fd_rdir);
-	}
-	if (fdc->fd_cdir == NULL) {
-		fdc->fd_cdir = rootvnode;
-		vref(fdc->fd_rdir);
-	}
-	FILEDESC_XUNLOCK(fdc);
+	pwd_ensure_dirs();
 
 	if (startvp != NULL)
 		vref(startvp);
diff --git a/sys/compat/ndis/subr_ndis.c b/sys/compat/ndis/subr_ndis.c
index f3ba700..ac26a2e 100644
--- a/sys/compat/ndis/subr_ndis.c
+++ b/sys/compat/ndis/subr_ndis.c
@@ -2817,10 +2817,7 @@ NdisOpenFile(status, filehandle, filelength, filename, highestaddr)
 
 	/* Some threads don't have a current working directory. */
 
-	if (td->td_proc->p_fd->fd_rdir == NULL)
-		td->td_proc->p_fd->fd_rdir = rootvnode;
-	if (td->td_proc->p_fd->fd_cdir == NULL)
-		td->td_proc->p_fd->fd_cdir = rootvnode;
+	pwd_ensure_dirs();
 
 	NDINIT(&nd, LOOKUP, FOLLOW, UIO_SYSSPACE, path, td);
 
diff --git a/sys/dev/xen/blkback/blkback.c b/sys/dev/xen/blkback/blkback.c
index 459271e..f266ffd 100644
--- a/sys/dev/xen/blkback/blkback.c
+++ b/sys/dev/xen/blkback/blkback.c
@@ -2692,18 +2692,7 @@ xbb_open_backend(struct xbb_softc *xbb)
 	if ((xbb->flags & XBBF_READ_ONLY) == 0)
 		flags |= FWRITE;
 
-	if (!curthread->td_proc->p_fd->fd_cdir) {
-		curthread->td_proc->p_fd->fd_cdir = rootvnode;
-		VREF(rootvnode);
-	}
-	if (!curthread->td_proc->p_fd->fd_rdir) {
-		curthread->td_proc->p_fd->fd_rdir = rootvnode;
-		VREF(rootvnode);
-	}
-	if (!curthread->td_proc->p_fd->fd_jdir) {
-		curthread->td_proc->p_fd->fd_jdir = rootvnode;
-		VREF(rootvnode);
-	}
+	pwd_ensure_dirs();
 
  again:
 	NDINIT(&nd, LOOKUP, FOLLOW, UIO_SYSSPACE, xbb->dev_name, curthread);
diff --git a/sys/kern/kern_descrip.c b/sys/kern/kern_descrip.c
index 37381ee..dea9d35 100644
--- a/sys/kern/kern_descrip.c
+++ b/sys/kern/kern_descrip.c
@@ -68,6 +68,7 @@ __FBSDID("$FreeBSD$");
 #include <sys/sbuf.h>
 #include <sys/signalvar.h>
 #include <sys/socketvar.h>
+#include <sys/kdb.h>
 #include <sys/stat.h>
 #include <sys/sx.h>
 #include <sys/syscallsubr.h>
@@ -308,6 +309,24 @@ fdfree(struct filedesc *fdp, int fd)
 #endif
 }
 
+void
+pwd_ensure_dirs(void)
+{
+	struct filedesc *fdp;
+
+	fdp = curproc->p_fd;
+	FILEDESC_XLOCK(fdp);
+	if (fdp->fd_cdir == NULL) {
+		fdp->fd_cdir = rootvnode;
+		VREF(rootvnode);
+	}
+	if (fdp->fd_rdir == NULL) {
+		fdp->fd_rdir = rootvnode;
+		VREF(rootvnode);
+	}
+	FILEDESC_XUNLOCK(fdp);
+}
+
 /*
  * System calls on descriptors.
  */
diff --git a/sys/kern/subr_firmware.c b/sys/kern/subr_firmware.c
index 20ab76e..172d719 100644
--- a/sys/kern/subr_firmware.c
+++ b/sys/kern/subr_firmware.c
@@ -383,19 +383,8 @@ firmware_put(const struct firmware *p, int flags)
 static void
 set_rootvnode(void *arg, int npending)
 {
-	struct thread *td = curthread;
-	struct proc *p = td->td_proc;
 
-	FILEDESC_XLOCK(p->p_fd);
-	if (p->p_fd->fd_cdir == NULL) {
-		p->p_fd->fd_cdir = rootvnode;
-		VREF(rootvnode);
-	}
-	if (p->p_fd->fd_rdir == NULL) {
-		p->p_fd->fd_rdir = rootvnode;
-		VREF(rootvnode);
-	}
-	FILEDESC_XUNLOCK(p->p_fd);
+	pwd_ensure_dirs();
 
 	free(arg, M_TEMP);
 }
diff --git a/sys/sys/filedesc.h b/sys/sys/filedesc.h
index e569a3b..727a098 100644
--- a/sys/sys/filedesc.h
+++ b/sys/sys/filedesc.h
@@ -208,6 +208,7 @@ fd_modified(struct filedesc *fdp, int fd, seq_t seq)
 /* cdir/rdir/jdir manipulation functions. */
 void	pwd_chdir(struct thread *td, struct vnode *vp);
 int	pwd_chroot(struct thread *td, struct vnode *vp);
+void	pwd_ensure_dirs(void);
 
 #endif /* _KERNEL */
 
-- 
2.4.5




Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?1436569684-3939-3-git-send-email-mjguzik>