git: 5f73b3338ee1 - main - unionfs: Improve vnode validation

From: Jason A. Harmening <jah_at_FreeBSD.org>
Date: Sat, 06 Nov 2021 14:02:21 UTC
The branch main has been updated by jah:

URL: https://cgit.FreeBSD.org/src/commit/?id=5f73b3338ee10ea843da3ae8b241320b8293a35b

commit 5f73b3338ee10ea843da3ae8b241320b8293a35b
Author:     Jason A. Harmening <jah@FreeBSD.org>
AuthorDate: 2021-10-28 05:37:48 +0000
Commit:     Jason A. Harmening <jah@FreeBSD.org>
CommitDate: 2021-11-06 14:08:34 +0000

    unionfs: Improve vnode validation
    
    Instead of validating that a vnode belongs to unionfs only when the
    caller attempts to extract the upper or lower vnode pointers, do this
    validation any time the caller tries to extract a unionfs_node from
    the vnode private data.
    
    Reviewed by:    kib
    Differential Revision:  https://reviews.freebsd.org/D32629
---
 sys/fs/unionfs/union.h      | 26 ++++++++++++++++----------
 sys/fs/unionfs/union_subr.c | 42 ------------------------------------------
 2 files changed, 16 insertions(+), 52 deletions(-)

diff --git a/sys/fs/unionfs/union.h b/sys/fs/unionfs/union.h
index 5708998c6043..eda24c1b8333 100644
--- a/sys/fs/unionfs/union.h
+++ b/sys/fs/unionfs/union.h
@@ -108,8 +108,23 @@ struct unionfs_node {
 #define UNIONFS_OPENEXTL	0x01	/* openextattr (lower) */
 #define UNIONFS_OPENEXTU	0x02	/* openextattr (upper) */
 
+extern struct vop_vector unionfs_vnodeops;
+
+static inline struct unionfs_node *
+unionfs_check_vnode(struct vnode *vp, const char *file __unused,
+    int line __unused)
+{
+	/*
+	 * unionfs_lock() needs the NULL check here, as it explicitly
+	 * handles the case in which the vnode has been vgonel()'ed.
+	 */
+	KASSERT(vp->v_op == &unionfs_vnodeops || vp->v_data == NULL,
+	    ("%s:%d: non-unionfs vnode %p", file, line, vp));
+	return ((struct unionfs_node *)vp->v_data);
+}
+
 #define	MOUNTTOUNIONFSMOUNT(mp) ((struct unionfs_mount *)((mp)->mnt_data))
-#define	VTOUNIONFS(vp) ((struct unionfs_node *)(vp)->v_data)
+#define	VTOUNIONFS(vp) unionfs_check_vnode(vp, __FILE__, __LINE__)
 #define	UNIONFSTOV(xp) ((xp)->un_vnode)
 
 int	unionfs_init(struct vfsconf *);
@@ -143,17 +158,8 @@ int	unionfs_relookup_for_delete(struct vnode *, struct componentname *,
 int	unionfs_relookup_for_rename(struct vnode *, struct componentname *,
 	    struct thread *);
 
-#ifdef DIAGNOSTIC
-struct vnode	*unionfs_checklowervp(struct vnode *, char *, int);
-struct vnode	*unionfs_checkuppervp(struct vnode *, char *, int);
-#define	UNIONFSVPTOLOWERVP(vp) unionfs_checklowervp((vp), __FILE__, __LINE__)
-#define	UNIONFSVPTOUPPERVP(vp) unionfs_checkuppervp((vp), __FILE__, __LINE__)
-#else
 #define	UNIONFSVPTOLOWERVP(vp) (VTOUNIONFS(vp)->un_lowervp)
 #define	UNIONFSVPTOUPPERVP(vp) (VTOUNIONFS(vp)->un_uppervp)
-#endif
-
-extern struct vop_vector unionfs_vnodeops;
 
 #ifdef MALLOC_DECLARE
 MALLOC_DECLARE(M_UNIONFSNODE);
diff --git a/sys/fs/unionfs/union_subr.c b/sys/fs/unionfs/union_subr.c
index 8269b1e11fa4..195d74583443 100644
--- a/sys/fs/unionfs/union_subr.c
+++ b/sys/fs/unionfs/union_subr.c
@@ -1310,45 +1310,3 @@ unionfs_check_rmdir(struct vnode *vp, struct ucred *cred, struct thread *td)
 	return (error);
 }
 
-#ifdef DIAGNOSTIC
-
-struct vnode   *
-unionfs_checkuppervp(struct vnode *vp, char *fil, int lno)
-{
-	struct unionfs_node *unp;
-
-	unp = VTOUNIONFS(vp);
-
-#ifdef notyet
-	if (vp->v_op != unionfs_vnodeop_p) {
-		printf("%s: on non-unionfs-node.\n", __func__);
-#ifdef KDB
-		kdb_enter(KDB_WHY_UNIONFS,
-		    "unionfs_checkuppervp: on non-unionfs-node.\n");
-#endif
-		panic("%s", __func__);
-	}
-#endif
-	return (unp->un_uppervp);
-}
-
-struct vnode   *
-unionfs_checklowervp(struct vnode *vp, char *fil, int lno)
-{
-	struct unionfs_node *unp;
-
-	unp = VTOUNIONFS(vp);
-
-#ifdef notyet
-	if (vp->v_op != unionfs_vnodeop_p) {
-		printf("%s: on non-unionfs-node.\n", __func__);
-#ifdef KDB
-		kdb_enter(KDB_WHY_UNIONFS,
-		    "unionfs_checklowervp: on non-unionfs-node.\n");
-#endif
-		panic("%s", __func__);
-	}
-#endif
-	return (unp->un_lowervp);
-}
-#endif