d/freebsd-dev
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

unionfs: Ensure SAVENAME is set for unionfs vnode operations

Browse Source 
"rm-style" system calls such as kern_frmdirat() and kern_funlinkat()
don't supply SAVENAME to preserve the pathname buffer for subsequent
vnode ops.  For unionfs this poses an issue because the pathname may
be needed for a relookup operation in unionfs_remove()/unionfs_rmdir().
Currently unionfs doesn't check for this case, leading to a panic on
DIAGNOSTIC kernels and use-after-free of cn_nameptr otherwise.

The unionfs node's stored buffer would suffice as a replacement for
cnp->cn_nameptr in some (but not all) cases, but it's cleaner to just
ensure that unionfs vnode ops always have a valid cn_nameptr by setting
SAVENAME in unionfs_lookup().

While here, do some light cleanup in unionfs_lookup() and assert that
HASBUF is always present in the relevant relookup calls.

Reported by:	pho
Reviewed by:	markj
Differential Revision: https://reviews.freebsd.org/D32148
This commit is contained in:
Jason A. Harmening 2021-09-25 19:56:04 -07:00
parent 04c91ac48a
commit 152c35ee4f
2 changed files with 24 additions and 17 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
sys/fs/unionfs/union_subr.c
View File

@ -678,6 +678,8 @@ unionfs_relookup_for_create(struct vnode *dvp, struct componentname *cnp,
udvp = UNIONFSVPTOUPPERVP(dvp);
vp = NULLVP;
KASSERT((cnp->cn_flags & HASBUF) != 0,
("%s called without HASBUF", __func__));
error = unionfs_relookup(udvp, &vp, cnp, &cn, td, cnp->cn_nameptr,
cnp->cn_namelen, CREATE);
if (error)
@ -712,6 +714,8 @@ unionfs_relookup_for_delete(struct vnode *dvp, struct componentname *cnp,
udvp = UNIONFSVPTOUPPERVP(dvp);
vp = NULLVP;
KASSERT((cnp->cn_flags & HASBUF) != 0,
("%s called without HASBUF", __func__));
error = unionfs_relookup(udvp, &vp, cnp, &cn, td, cnp->cn_nameptr,
cnp->cn_namelen, DELETE);
if (error)
@ -746,6 +750,8 @@ unionfs_relookup_for_rename(struct vnode *dvp, struct componentname *cnp,
udvp = UNIONFSVPTOUPPERVP(dvp);
vp = NULLVP;
KASSERT((cnp->cn_flags & HASBUF) != 0,
("%s called without HASBUF", __func__));
error = unionfs_relookup(udvp, &vp, cnp, &cn, td, cnp->cn_nameptr,
cnp->cn_namelen, RENAME);
if (error)

35
sys/fs/unionfs/union_vnops.c
View File

@ -167,9 +167,7 @@ unionfs_lookup(struct vop_cachedlookup_args *ap)
} else if (error == ENOENT && (cnflags & MAKEENTRY) != 0)
cache_enter(dvp, NULLVP, cnp);
UNIONFS_INTERNAL_DEBUG("unionfs_lookup: leave (%d)\n", error);
return (error);
goto unionfs_lookup_return;
}
/*
@ -184,10 +182,8 @@ unionfs_lookup(struct vop_cachedlookup_args *ap)
*(ap->a_vpp) = dvp;
vref(dvp);
UNIONFS_INTERNAL_DEBUG(
"unionfs_lookup: leave (%d)\n", uerror);
return (uerror);
error = uerror;
goto unionfs_lookup_return;
}
if (nameiop == DELETE || nameiop == RENAME ||
(cnp->cn_lkflags & LK_TYPE_MASK))
@ -246,9 +242,8 @@ unionfs_lookup(struct vop_cachedlookup_args *ap)
* check lookup result
*/
if (uvp == NULLVP && lvp == NULLVP) {
UNIONFS_INTERNAL_DEBUG("unionfs_lookup: leave (%d)\n",
(udvp != NULLVP ? uerror : lerror));
return (udvp != NULLVP ? uerror : lerror);
error = (udvp != NULLVP ? uerror : lerror);
goto unionfs_lookup_return;
}
/*
@ -270,7 +265,7 @@ unionfs_lookup(struct vop_cachedlookup_args *ap)
error = unionfs_nodeget(dvp->v_mount, NULLVP, lvp, dvp, &vp,
cnp, td);
if (error != 0)
goto unionfs_lookup_out;
goto unionfs_lookup_cleanup;
if (LK_SHARED == (cnp->cn_lkflags & LK_TYPE_MASK))
VOP_UNLOCK(vp);
@ -289,7 +284,7 @@ unionfs_lookup(struct vop_cachedlookup_args *ap)
vput(vp);
else
vrele(vp);
goto unionfs_lookup_out;
goto unionfs_lookup_cleanup;
}
if ((cnp->cn_lkflags & LK_TYPE_MASK) == LK_SHARED)
vn_lock(vp, LK_SHARED | LK_RETRY);
@ -303,7 +298,7 @@ unionfs_lookup(struct vop_cachedlookup_args *ap)
else
error = lerror;
if (error != 0)
goto unionfs_lookup_out;
goto unionfs_lookup_cleanup;
/*
* get socket vnode.
*/
@ -328,7 +323,7 @@ unionfs_lookup(struct vop_cachedlookup_args *ap)
if (error != 0) {
UNIONFSDEBUG(
"unionfs_lookup: Unable to create unionfs vnode.");
goto unionfs_lookup_out;
goto unionfs_lookup_cleanup;
}
if ((nameiop == DELETE || nameiop == RENAME) &&
(cnp->cn_lkflags & LK_TYPE_MASK) == 0)
@ -340,7 +335,7 @@ unionfs_lookup(struct vop_cachedlookup_args *ap)
if ((cnflags & MAKEENTRY) && vp->v_type != VSOCK)
cache_enter(dvp, vp, cnp);
unionfs_lookup_out:
unionfs_lookup_cleanup:
if (uvp != NULLVP)
vrele(uvp);
if (lvp != NULLVP)
@ -349,6 +344,12 @@ unionfs_lookup(struct vop_cachedlookup_args *ap)
if (error == ENOENT && (cnflags & MAKEENTRY) != 0)
cache_enter(dvp, NULLVP, cnp);
unionfs_lookup_return:
/* Ensure subsequent vnops will get a valid pathname buffer. */
if (nameiop != LOOKUP && (error == 0 || error == EJUSTRETURN))
cnp->cn_flags |= SAVENAME;
UNIONFS_INTERNAL_DEBUG("unionfs_lookup: leave (%d)\n", error);
return (error);
@ -1372,7 +1373,7 @@ unionfs_mkdir(struct vop_mkdir_args *ap)
error = VOP_GETATTR(udvp, &va, cnp->cn_cred);
if (error != 0)
return (error);
if (va.va_flags & OPAQUE)
if ((va.va_flags & OPAQUE) != 0)
cnp->cn_flags |= ISWHITEOUT;
}
@ -1434,7 +1435,7 @@ unionfs_rmdir(struct vop_rmdir_args *ap)
if (ump->um_whitemode == UNIONFS_WHITE_ALWAYS || lvp != NULLVP)
cnp->cn_flags |= DOWHITEOUT;
error = unionfs_relookup_for_delete(ap->a_dvp, cnp, td);
if (!error)
if (error == 0)
error = VOP_RMDIR(udvp, uvp, cnp);
}
else if (lvp != NULLVP)