upstream/opnsense-src
1
0
Fork 0
mirror of https://github.com/opnsense/src.git synced 2026-06-11 09:41:03 -04:00
Code Issues Projects Releases Packages Wiki Activity Actions

unionfs: rework pathname handling

Browse source 
Running stress2 unionfs tests reliably produces a namei_zone corruption
panic due to unionfs_relookup() attempting to NUL-terminate a newly-
allocate pathname buffer without first validating the buffer length.

Instead, avoid allocating new pathname buffers in unionfs entirely,
using already-provided buffers while ensuring the the correct flags
are set in struct componentname to prevent freeing or manipulation
of those buffers at lower layers.

While here, also compute and store the path length once in the unionfs
node instead of constantly invoking strlen() on it.

Reviewed by:	kib, markj
Differential Revision:	https://reviews.freebsd.org/D31728
This commit is contained in:
Jason A. Harmening 2021-08-29 14:36:15 -07:00
parent c98bf2a45e
commit abe95116ba
3 changed files with 39 additions and 79 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

4
sys/fs/unionfs/union.h
View file

@ -98,6 +98,7 @@ struct unionfs_node {
u_long un_hashmask; /* bit mask */
char *un_path; /* path */
int un_pathlen; /* strlen of path */
int un_flag; /* unionfs node flag */
};
@ -124,7 +125,8 @@ int unionfs_copyfile(struct unionfs_node *unp, int docopy, struct ucred *cred, s
void unionfs_create_uppervattr_core(struct unionfs_mount *ump, struct vattr *lva, struct vattr *uva, struct thread *td);
int unionfs_create_uppervattr(struct unionfs_mount *ump, struct vnode *lvp, struct vattr *uva, struct ucred *cred, struct thread *td);
int unionfs_mkshadowdir(struct unionfs_mount *ump, struct vnode *duvp, struct unionfs_node *unp, struct componentname *cnp, struct thread *td);
int unionfs_mkwhiteout(struct vnode *dvp, struct componentname *cnp, struct thread *td, char *path);
int unionfs_mkwhiteout(struct vnode *dvp, struct componentname *cnp,
struct thread *td, char *path, int pathlen);
int unionfs_relookup(struct vnode *dvp, struct vnode **vpp, struct componentname *cnp, struct componentname *cn, struct thread *td, char *path, int pathlen, u_long nameiop);
int unionfs_relookup_for_create(struct vnode *dvp, struct componentname *cnp, struct thread *td);
int unionfs_relookup_for_delete(struct vnode *dvp, struct componentname *cnp, struct thread *td);

97
sys/fs/unionfs/union_subr.c
View file

@ -332,6 +332,7 @@ unionfs_nodeget(struct mount *mp, struct vnode *uppervp,
malloc(cnp->cn_namelen +1, M_UNIONFSPATH, M_WAITOK|M_ZERO);
bcopy(cnp->cn_nameptr, unp->un_path, cnp->cn_namelen);
unp->un_path[cnp->cn_namelen] = '\0';
unp->un_pathlen = cnp->cn_namelen;
}
vp->v_type = vt;
vp->v_data = unp;
@ -420,6 +421,7 @@ unionfs_noderem(struct vnode *vp, struct thread *td)
if (unp->un_path != NULL) {
free(unp->un_path, M_UNIONFSPATH);
unp->un_path = NULL;
unp->un_pathlen = 0;
}
if (unp->un_hashtbl != NULL) {
@ -576,16 +578,12 @@ unionfs_relookup(struct vnode *dvp, struct vnode **vpp,
int error;
cn->cn_namelen = pathlen;
cn->cn_pnbuf = uma_zalloc(namei_zone, M_WAITOK);
bcopy(path, cn->cn_pnbuf, pathlen);
cn->cn_pnbuf[pathlen] = '\0';
cn->cn_pnbuf = path;
cn->cn_nameiop = nameiop;
cn->cn_flags = (LOCKPARENT | LOCKLEAF | HASBUF | SAVENAME | ISLASTCN);
cn->cn_lkflags = LK_EXCLUSIVE;
cn->cn_thread = td;
cn->cn_cred = cnp->cn_cred;
cn->cn_nameptr = cn->cn_pnbuf;
if (nameiop == DELETE)
@ -599,12 +597,16 @@ unionfs_relookup(struct vnode *dvp, struct vnode **vpp,
VOP_UNLOCK(dvp);
if ((error = relookup(dvp, vpp, cn))) {
uma_zfree(namei_zone, cn->cn_pnbuf);
cn->cn_flags &= ~HASBUF;
vn_lock(dvp, LK_EXCLUSIVE | LK_RETRY);
} else
vrele(dvp);
KASSERT((cn->cn_flags & HASBUF) != 0,
("%s: HASBUF cleared", __func__));
KASSERT((cn->cn_flags & SAVENAME) != 0,
("%s: SAVENAME cleared", __func__));
KASSERT(cn->cn_pnbuf == path, ("%s: cn_pnbuf changed", __func__));
return (error);
}
@ -630,7 +632,7 @@ unionfs_relookup_for_create(struct vnode *dvp, struct componentname *cnp,
vp = NULLVP;
error = unionfs_relookup(udvp, &vp, cnp, &cn, td, cnp->cn_nameptr,
strlen(cnp->cn_nameptr), CREATE);
cnp->cn_namelen, CREATE);
if (error)
return (error);
@ -643,16 +645,6 @@ unionfs_relookup_for_create(struct vnode *dvp, struct componentname *cnp,
error = EEXIST;
}
if (cn.cn_flags & HASBUF) {
uma_zfree(namei_zone, cn.cn_pnbuf);
cn.cn_flags &= ~HASBUF;
}
if (!error) {
cn.cn_flags |= (cnp->cn_flags & HASBUF);
cnp->cn_flags = cn.cn_flags;
}
return (error);
}
@ -674,7 +666,7 @@ unionfs_relookup_for_delete(struct vnode *dvp, struct componentname *cnp,
vp = NULLVP;
error = unionfs_relookup(udvp, &vp, cnp, &cn, td, cnp->cn_nameptr,
strlen(cnp->cn_nameptr), DELETE);
cnp->cn_namelen, DELETE);
if (error)
return (error);
@ -687,16 +679,6 @@ unionfs_relookup_for_delete(struct vnode *dvp, struct componentname *cnp,
vput(vp);
}
if (cn.cn_flags & HASBUF) {
uma_zfree(namei_zone, cn.cn_pnbuf);
cn.cn_flags &= ~HASBUF;
}
if (!error) {
cn.cn_flags |= (cnp->cn_flags & HASBUF);
cnp->cn_flags = cn.cn_flags;
}
return (error);
}
@ -718,7 +700,7 @@ unionfs_relookup_for_rename(struct vnode *dvp, struct componentname *cnp,
vp = NULLVP;
error = unionfs_relookup(udvp, &vp, cnp, &cn, td, cnp->cn_nameptr,
strlen(cnp->cn_nameptr), RENAME);
cnp->cn_namelen, RENAME);
if (error)
return (error);
@ -729,18 +711,7 @@ unionfs_relookup_for_rename(struct vnode *dvp, struct componentname *cnp,
vput(vp);
}
if (cn.cn_flags & HASBUF) {
uma_zfree(namei_zone, cn.cn_pnbuf);
cn.cn_flags &= ~HASBUF;
}
if (!error) {
cn.cn_flags |= (cnp->cn_flags & HASBUF);
cnp->cn_flags = cn.cn_flags;
}
return (error);
}
/*
@ -848,11 +819,11 @@ unionfs_mkshadowdir(struct unionfs_mount *ump, struct vnode *udvp,
vput(uvp);
error = EEXIST;
goto unionfs_mkshadowdir_free_out;
goto unionfs_mkshadowdir_abort;
}
if ((error = vn_start_write(udvp, &mp, V_WAIT | PCATCH)))
goto unionfs_mkshadowdir_free_out;
goto unionfs_mkshadowdir_abort;
unionfs_create_uppervattr_core(ump, &lva, &va, td);
error = VOP_MKDIR(udvp, &uvp, &nd.ni_cnd, &va);
@ -869,12 +840,6 @@ unionfs_mkshadowdir(struct unionfs_mount *ump, struct vnode *udvp,
}
vn_finished_write(mp);
unionfs_mkshadowdir_free_out:
if (nd.ni_cnd.cn_flags & HASBUF) {
uma_zfree(namei_zone, nd.ni_cnd.cn_pnbuf);
nd.ni_cnd.cn_flags &= ~HASBUF;
}
unionfs_mkshadowdir_abort:
cnp->cn_cred = credbk;
chgproccnt(cred->cr_ruidinfo, -1, 0);
@ -890,26 +855,20 @@ unionfs_mkshadowdir_abort:
*/
int
unionfs_mkwhiteout(struct vnode *dvp, struct componentname *cnp,
struct thread *td, char *path)
struct thread *td, char *path, int pathlen)
{
int error;
struct vnode *wvp;
struct nameidata nd;
struct mount *mp;
if (path == NULL)
path = cnp->cn_nameptr;
int error;
wvp = NULLVP;
NDPREINIT(&nd);
if ((error = unionfs_relookup(dvp, &wvp, cnp, &nd.ni_cnd, td, path,
strlen(path), CREATE)))
pathlen, CREATE))) {
return (error);
}
if (wvp != NULLVP) {
if (nd.ni_cnd.cn_flags & HASBUF) {
uma_zfree(namei_zone, nd.ni_cnd.cn_pnbuf);
nd.ni_cnd.cn_flags &= ~HASBUF;
}
if (dvp == wvp)
vrele(wvp);
else
@ -925,11 +884,6 @@ unionfs_mkwhiteout(struct vnode *dvp, struct componentname *cnp,
vn_finished_write(mp);
unionfs_mkwhiteout_free_out:
if (nd.ni_cnd.cn_flags & HASBUF) {
uma_zfree(namei_zone, nd.ni_cnd.cn_pnbuf);
nd.ni_cnd.cn_flags &= ~HASBUF;
}
return (error);
}
@ -969,9 +923,8 @@ unionfs_vn_create_on_upper(struct vnode **vpp, struct vnode *udvp,
if (unp->un_path == NULL)
panic("unionfs: un_path is null");
nd.ni_cnd.cn_namelen = strlen(unp->un_path);
nd.ni_cnd.cn_pnbuf = uma_zalloc(namei_zone, M_WAITOK);
bcopy(unp->un_path, nd.ni_cnd.cn_pnbuf, nd.ni_cnd.cn_namelen + 1);
nd.ni_cnd.cn_namelen = unp->un_pathlen;
nd.ni_cnd.cn_pnbuf = unp->un_path;
nd.ni_cnd.cn_nameiop = CREATE;
nd.ni_cnd.cn_flags = LOCKPARENT | LOCKLEAF | HASBUF | SAVENAME |
ISLASTCN;
@ -1015,10 +968,12 @@ unionfs_vn_create_on_upper_free_out1:
VOP_UNLOCK(udvp);
unionfs_vn_create_on_upper_free_out2:
if (nd.ni_cnd.cn_flags & HASBUF) {
uma_zfree(namei_zone, nd.ni_cnd.cn_pnbuf);
nd.ni_cnd.cn_flags &= ~HASBUF;
}
KASSERT((nd.ni_cnd.cn_flags & HASBUF) != 0,
("%s: HASBUF cleared", __func__));
KASSERT((nd.ni_cnd.cn_flags & SAVENAME) != 0,
("%s: SAVENAME cleared", __func__));
KASSERT(nd.ni_cnd.cn_pnbuf == unp->un_path,
("%s: cn_pnbuf changed", __func__));
return (error);
}

17
sys/fs/unionfs/union_vnops.c
View file

@ -961,7 +961,6 @@ unionfs_fsync(struct vop_fsync_args *ap)
static int
unionfs_remove(struct vop_remove_args *ap)
{
int error;
char *path;
struct unionfs_node *dunp;
struct unionfs_node *unp;
@ -973,6 +972,8 @@ unionfs_remove(struct vop_remove_args *ap)
struct componentname *cnp;
struct componentname cn;
struct thread *td;
int error;
int pathlen;
UNIONFS_INTERNAL_DEBUG("unionfs_remove: enter\n");
@ -992,7 +993,7 @@ unionfs_remove(struct vop_remove_args *ap)
/* search vnode */
VOP_UNLOCK(ap->a_vp);
error = unionfs_relookup(udvp, &vp, cnp, &cn, td,
cnp->cn_nameptr, strlen(cnp->cn_nameptr), DELETE);
cnp->cn_nameptr, cnp->cn_namelen, DELETE);
if (error != 0 && error != ENOENT) {
vn_lock(ap->a_vp, LK_EXCLUSIVE | LK_RETRY);
return (error);
@ -1002,7 +1003,6 @@ unionfs_remove(struct vop_remove_args *ap)
/* target vnode in upper */
uvp = vp;
vrele(vp);
path = NULL;
} else {
/* target vnode in lower */
if (vp != NULLVP) {
@ -1013,14 +1013,16 @@ unionfs_remove(struct vop_remove_args *ap)
}
vn_lock(ap->a_vp, LK_EXCLUSIVE | LK_RETRY);
lvp = ap->a_vp;
path = ap->a_cnp->cn_nameptr;
}
path = cnp->cn_nameptr;
pathlen = cnp->cn_namelen;
} else {
ump = MOUNTTOUNIONFSMOUNT(ap->a_vp->v_mount);
unp = VTOUNIONFS(ap->a_vp);
uvp = unp->un_uppervp;
lvp = unp->un_lowervp;
path = unp->un_path;
pathlen = unp->un_pathlen;
}
if (udvp == NULLVP)
@ -1036,7 +1038,7 @@ unionfs_remove(struct vop_remove_args *ap)
cnp->cn_flags |= DOWHITEOUT;
error = VOP_REMOVE(udvp, uvp, cnp);
} else if (lvp != NULLVP)
error = unionfs_mkwhiteout(udvp, cnp, td, path);
error = unionfs_mkwhiteout(udvp, cnp, td, path, pathlen);
UNIONFS_INTERNAL_DEBUG("unionfs_remove: leave (%d)\n", error);
@ -1262,7 +1264,7 @@ unionfs_rename(struct vop_rename_args *ap)
if (error != 0)
goto unionfs_rename_abort;
/* Locke of tvp is canceled in order to avoid recursive lock. */
/* Lock of tvp is canceled in order to avoid recursive lock. */
if (tvp != NULLVP && tvp != tdvp)
VOP_UNLOCK(tvp);
error = unionfs_relookup_for_rename(tdvp, tcnp, td);
@ -1421,7 +1423,8 @@ unionfs_rmdir(struct vop_rmdir_args *ap)
error = VOP_RMDIR(udvp, uvp, cnp);
}
else if (lvp != NULLVP)
error = unionfs_mkwhiteout(udvp, cnp, td, unp->un_path);
error = unionfs_mkwhiteout(udvp, cnp, td,
unp->un_path, unp->un_pathlen);
if (error == 0) {
cache_purge(ap->a_dvp);