Date: Fri, 2 Apr 2010 15:14:59 +0300 From: Kostik Belousov <kostikbel@gmail.com> To: Mikolaj Golub <to.my.trociny@gmail.com> Cc: freebsd-fs@freebsd.org, Eugene Grosbein <eugen@grosbein.pp.ru> Subject: Re: Lock leakage on rename if vop->vop_rename is NULL (kern/107439) Message-ID: <20100402121459.GH2415@deviant.kiev.zoral.com.ua> In-Reply-To: <86wrwqnlgp.fsf@zhuzha.ua1> References: <86sk7e1gad.fsf@zhuzha.ua1> <20100402090401.GG2415@deviant.kiev.zoral.com.ua> <86wrwqnlgp.fsf@zhuzha.ua1>
next in thread | previous in thread | raw e-mail | index | archive | help
--GFHRPGHrS2AOMWit
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable
On Fri, Apr 02, 2010 at 02:27:18PM +0300, Mikolaj Golub wrote:
>=20
> On Fri, 2 Apr 2010 12:04:01 +0300 Kostik Belousov wrote:
>=20
> KB> Please test the patch below. If it works for you, I will combine
> KB> deadfs_rename() and vop_norename().
>=20
> 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 thin=
gs)
> 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 =3D=3D 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>
=20
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 =3D {
.vop_poll =3D vop_nopoll,
.vop_putpages =3D vop_stdputpages,
.vop_readlink =3D VOP_EINVAL,
+ .vop_rename =3D vop_norename,
.vop_revoke =3D VOP_PANIC,
.vop_strategy =3D vop_nostrategy,
.vop_unlock =3D vop_stdunlock,
@@ -206,6 +208,20 @@ vop_nolookup(ap)
}
=20
/*
+ * 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 */
=20
void
+vop_rename_fail(struct vop_rename_args *ap)
+{
+
+ if (ap->a_tvp !=3D NULL)
+ vput(ap->a_tvp);
+ if (ap->a_tdvp =3D=3D 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 =3D 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);
=20
+void vop_rename_fail(struct vop_rename_args *ap);
+
#define VOP_WRITE_PRE(ap) \
struct vattr va; \
int error, osize, ooffset, noffset; \
--GFHRPGHrS2AOMWit
Content-Type: application/pgp-signature
Content-Disposition: inline
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (FreeBSD)
iEYEARECAAYFAku138MACgkQC3+MBN1Mb4i2AQCgrUhzoR4uLKr06Nqaem/O0IiH
9msAoJcNPy+QKXnVhIriX+TDS8Wnx0a0
=rqUo
-----END PGP SIGNATURE-----
--GFHRPGHrS2AOMWit--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20100402121459.GH2415>