Lock leakage on rename if vop->vop_rename is NULL (kern/107439)

[Kostik Belousov]

On Fri, Apr 02, 2010 at 02:27:18PM +0300, Mikolaj Golub wrote:
> 
> On Fri, 2 Apr 2010 12:04:01 +0300 Kostik Belousov wrote:
> 
>  KB> Please test the patch below. If it works for you, I will combine
>  KB> deadfs_rename() and vop_norename().
> 
> Tested on today's 8.0-STABLE i386. Trying to move from/to/inside ntfs: no
> locking issues or problems with unmounting fs (and any other strange things)
> have been detected.

Great, thank you.

Below is the patch I intent to commit. Will be grateful if you sanity-check
it too.

diff --git a/sys/fs/deadfs/dead_vnops.c b/sys/fs/deadfs/dead_vnops.c
index 7a07b38..e255654 100644
--- a/sys/fs/deadfs/dead_vnops.c
+++ b/sys/fs/deadfs/dead_vnops.c
@@ -225,13 +225,7 @@ dead_rename(ap)
 		struct componentname *a_tcnp;
 	} */ *ap;
 {
-	if (ap->a_tvp)
-		vput(ap->a_tvp);
-	if (ap->a_tdvp == ap->a_tvp)
-		vrele(ap->a_tdvp);
-	else
-		vput(ap->a_tdvp);
-	vrele(ap->a_fdvp);
-	vrele(ap->a_fvp);
+
+	vop_rename_fail(ap);
 	return (EXDEV);
 }
diff --git a/sys/kern/vfs_default.c b/sys/kern/vfs_default.c
index 50bf0d2..3788147 100644
--- a/sys/kern/vfs_default.c
+++ b/sys/kern/vfs_default.c
@@ -67,6 +67,7 @@ __FBSDID("$FreeBSD$");
 #include <vm/vnode_pager.h>
 
 static int	vop_nolookup(struct vop_lookup_args *);
+static int	vop_norename(struct vop_rename_args *);
 static int	vop_nostrategy(struct vop_strategy_args *);
 static int	get_next_dirent(struct vnode *vp, struct dirent **dpp,
 				char *dirbuf, int dirbuflen, off_t *off,
@@ -113,6 +114,7 @@ struct vop_vector default_vnodeops = {
 	.vop_poll =		vop_nopoll,
 	.vop_putpages =		vop_stdputpages,
 	.vop_readlink =		VOP_EINVAL,
+	.vop_rename =		vop_norename,
 	.vop_revoke =		VOP_PANIC,
 	.vop_strategy =		vop_nostrategy,
 	.vop_unlock =		vop_stdunlock,
@@ -206,6 +208,20 @@ vop_nolookup(ap)
 }
 
 /*
+ * vop_norename:
+ *
+ * Handle unlock and reference counting for arguments of vop_rename
+ * for filesystems that do not implement rename operation.
+ */
+static int
+vop_norename(struct vop_rename_args *ap)
+{
+
+	vop_rename_fail(ap);
+	return (EOPNOTSUPP);
+}
+
+/*
  *	vop_nostrategy:
  *
  *	Strategy routine for VFS devices that have none.
diff --git a/sys/kern/vfs_subr.c b/sys/kern/vfs_subr.c
index daaa5b1..9d4b3a9 100644
--- a/sys/kern/vfs_subr.c
+++ b/sys/kern/vfs_subr.c
@@ -3751,6 +3751,20 @@ assert_vop_slocked(struct vnode *vp, const char *str)
 #endif /* DEBUG_VFS_LOCKS */
 
 void
+vop_rename_fail(struct vop_rename_args *ap)
+{
+
+	if (ap->a_tvp != NULL)
+		vput(ap->a_tvp);
+	if (ap->a_tdvp == ap->a_tvp)
+		vrele(ap->a_tdvp);
+	else
+		vput(ap->a_tdvp);
+	vrele(ap->a_fdvp);
+	vrele(ap->a_fvp);
+}
+
+void
 vop_rename_pre(void *ap)
 {
 	struct vop_rename_args *a = ap;
diff --git a/sys/sys/vnode.h b/sys/sys/vnode.h
index 4c84ea3..b5784e4 100644
--- a/sys/sys/vnode.h
+++ b/sys/sys/vnode.h
@@ -720,6 +720,8 @@ void	vop_symlink_post(void *a, int rc);
 void	vop_unlock_post(void *a, int rc);
 void	vop_unlock_pre(void *a);
 
+void	vop_rename_fail(struct vop_rename_args *ap);
+
 #define	VOP_WRITE_PRE(ap)						\
 	struct vattr va;						\
 	int error, osize, ooffset, noffset;				\
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 196 bytes
Desc: not available
Url : http://lists.freebsd.org/pipermail/freebsd-fs/attachments/20100402/bb6c18d0/attachment.pgp