git: 5f73b3338ee1 - main - unionfs: Improve vnode validation
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
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