diff options
author | Bob Beck <beck@cvs.openbsd.org> | 2016-04-29 14:40:37 +0000 |
---|---|---|
committer | Bob Beck <beck@cvs.openbsd.org> | 2016-04-29 14:40:37 +0000 |
commit | f4bcd1acc00bfdb816780ce4ec8c6a0771db7e48 (patch) | |
tree | 9195aa1d7c4e3f2c7634ace105c90bddcedc4f5e | |
parent | 3e2a80372cf7ab1ece40b6e7efb0ce2b7ccb409f (diff) |
Fix some gibbering horrors due to uninitialized struct nameidata's
1) turn NDINITAT into a function and make the macros point to it
with the later goal of getting rid of them.
2) Sweep the kernel for places where people make a struct nameidata manually
on the stack and fill in what they hope are the required fields and count
on prayer and clean living to always get them right. Instead use NDINIT
everywhere. NFS was especially bad for this and there have likely
been things lurking there for a while.
3) Add a couple of strategic KASSERT's to catch future offenders.
ok krw@ sthen@ deraadt@
-rw-r--r-- | sys/dev/diskmap.c | 8 | ||||
-rw-r--r-- | sys/kern/vfs_lookup.c | 21 | ||||
-rw-r--r-- | sys/nfs/nfs_serv.c | 48 | ||||
-rw-r--r-- | sys/nfs/nfs_subs.c | 9 | ||||
-rw-r--r-- | sys/nfs/nfs_var.h | 5 | ||||
-rw-r--r-- | sys/nfs/nfs_vnops.c | 7 | ||||
-rw-r--r-- | sys/sys/namei.h | 19 |
7 files changed, 69 insertions, 48 deletions
diff --git a/sys/dev/diskmap.c b/sys/dev/diskmap.c index d99591e1857..ef3e90c517d 100644 --- a/sys/dev/diskmap.c +++ b/sys/dev/diskmap.c @@ -1,4 +1,4 @@ -/* $OpenBSD: diskmap.c,v 1.14 2016/03/19 12:04:15 natano Exp $ */ +/* $OpenBSD: diskmap.c,v 1.15 2016/04/29 14:40:36 beck Exp $ */ /* * Copyright (c) 2009, 2010 Joel Sing <jsing@openbsd.org> @@ -37,6 +37,7 @@ #include <sys/proc.h> #include <sys/vnode.h> #include <sys/pledge.h> +#include <sys/namei.h> int diskmapopen(dev_t dev, int flag, int fmt, struct proc *p) @@ -85,10 +86,7 @@ diskmapioctl(dev_t dev, u_long cmd, caddr_t addr, int flag, struct proc *p) fdp = p->p_fd; fdplock(fdp); - ndp.ni_segflg = UIO_SYSSPACE; - ndp.ni_dirfd = AT_FDCWD; - ndp.ni_dirp = devname; - ndp.ni_cnd.cn_proc = p; + NDINIT(&ndp, 0, 0, UIO_SYSSPACE, devname, p); ndp.ni_pledge = PLEDGE_RPATH; if ((error = vn_open(&ndp, fp->f_flag, 0)) != 0) goto bad; diff --git a/sys/kern/vfs_lookup.c b/sys/kern/vfs_lookup.c index fa921d04b22..0d4a2c43437 100644 --- a/sys/kern/vfs_lookup.c +++ b/sys/kern/vfs_lookup.c @@ -1,4 +1,4 @@ -/* $OpenBSD: vfs_lookup.c,v 1.62 2016/04/28 14:25:08 beck Exp $ */ +/* $OpenBSD: vfs_lookup.c,v 1.63 2016/04/29 14:40:36 beck Exp $ */ /* $NetBSD: vfs_lookup.c,v 1.17 1996/02/09 19:00:59 christos Exp $ */ /* @@ -95,6 +95,19 @@ pop_symlink(struct nameidata *ndp) #endif } +void +ndinitat(struct nameidata *ndp, u_long op, u_long flags, + enum uio_seg segflg, int dirfd, const char *namep, struct proc *p) +{ + memset(ndp, 0, sizeof(*ndp)); + (ndp)->ni_cnd.cn_nameiop = op; + (ndp)->ni_cnd.cn_flags = flags; + (ndp)->ni_segflg = segflg; + (ndp)->ni_dirfd = dirfd; + (ndp)->ni_dirp = namep; + (ndp)->ni_cnd.cn_proc = p; +} + /* * Convert a pathname into a pointer to a vnode. * @@ -129,6 +142,12 @@ namei(struct nameidata *ndp) struct componentname *cnp = &ndp->ni_cnd; struct proc *p = cnp->cn_proc; + /* + * Should be 0, if not someone didn't init ndp with NDINIT, + * go find and murder the offender messily. + */ + KASSERT (ndp->ni_p_path == NULL && ndp->ni_p_size == 0); + ndp->ni_cnd.cn_cred = ndp->ni_cnd.cn_proc->p_ucred; #ifdef DIAGNOSTIC if (!cnp->cn_cred || !cnp->cn_proc) diff --git a/sys/nfs/nfs_serv.c b/sys/nfs/nfs_serv.c index 821c71e4095..fd4472b6b1b 100644 --- a/sys/nfs/nfs_serv.c +++ b/sys/nfs/nfs_serv.c @@ -1,4 +1,4 @@ -/* $OpenBSD: nfs_serv.c,v 1.107 2016/03/19 12:04:16 natano Exp $ */ +/* $OpenBSD: nfs_serv.c,v 1.108 2016/04/29 14:40:36 beck Exp $ */ /* $NetBSD: nfs_serv.c,v 1.34 1997/05/12 23:37:12 fvdl Exp $ */ /* @@ -368,9 +368,9 @@ nfsrv_lookup(struct nfsrv_descript *nfsd, struct nfssvc_sock *slp, fhp = &nfh.fh_generic; nfsm_srvmtofh(fhp); nfsm_srvnamesiz(len); + + NDINIT(&nd, LOOKUP, LOCKLEAF | SAVESTART, UIO_SYSSPACE, NULL, procp); nd.ni_cnd.cn_cred = cred; - nd.ni_cnd.cn_nameiop = LOOKUP; - nd.ni_cnd.cn_flags = LOCKLEAF | SAVESTART; error = nfs_namei(&nd, fhp, len, slp, nam, &info.nmi_md, &info.nmi_dpos, &dirp, procp); if (dirp) { if (info.nmi_v3) @@ -882,9 +882,10 @@ nfsrv_create(struct nfsrv_descript *nfsd, struct nfssvc_sock *slp, fhp = &nfh.fh_generic; nfsm_srvmtofh(fhp); nfsm_srvnamesiz(len); + + NDINIT(&nd, CREATE, LOCKPARENT | LOCKLEAF | SAVESTART, UIO_SYSSPACE, + NULL, procp); nd.ni_cnd.cn_cred = cred; - nd.ni_cnd.cn_nameiop = CREATE; - nd.ni_cnd.cn_flags = LOCKPARENT | LOCKLEAF | SAVESTART; error = nfs_namei(&nd, fhp, len, slp, nam, &info.nmi_md, &info.nmi_dpos, &dirp, procp); if (dirp) { @@ -1136,13 +1137,13 @@ nfsrv_mknod(struct nfsrv_descript *nfsd, struct nfssvc_sock *slp, info.nmi_dpos = nfsd->nd_dpos; info.nmi_v3 = (nfsd->nd_flag & ND_NFSV3); - nd.ni_cnd.cn_nameiop = 0; fhp = &nfh.fh_generic; nfsm_srvmtofh(fhp); nfsm_srvnamesiz(len); + + NDINIT(&nd, CREATE, LOCKPARENT | LOCKLEAF | SAVESTART, UIO_SYSSPACE, + NULL, procp); nd.ni_cnd.cn_cred = cred; - nd.ni_cnd.cn_nameiop = CREATE; - nd.ni_cnd.cn_flags = LOCKPARENT | LOCKLEAF | SAVESTART; error = nfs_namei(&nd, fhp, len, slp, nam, &info.nmi_md, &info.nmi_dpos, &dirp, procp); if (dirp) dirfor_ret = VOP_GETATTR(dirp, &dirfor, cred, procp); @@ -1289,9 +1290,9 @@ nfsrv_remove(struct nfsrv_descript *nfsd, struct nfssvc_sock *slp, fhp = &nfh.fh_generic; nfsm_srvmtofh(fhp); nfsm_srvnamesiz(len); + + NDINIT(&nd, DELETE, LOCKPARENT | LOCKLEAF, UIO_SYSSPACE, NULL, procp); nd.ni_cnd.cn_cred = cred; - nd.ni_cnd.cn_nameiop = DELETE; - nd.ni_cnd.cn_flags = LOCKPARENT | LOCKLEAF; error = nfs_namei(&nd, fhp, len, slp, nam, &info.nmi_md, &info.nmi_dpos, &dirp, procp); if (dirp) { if (info.nmi_v3) @@ -1374,8 +1375,6 @@ nfsrv_rename(struct nfsrv_descript *nfsd, struct nfssvc_sock *slp, ffhp = &fnfh.fh_generic; tfhp = &tnfh.fh_generic; - fromnd.ni_cnd.cn_nameiop = 0; - tond.ni_cnd.cn_nameiop = 0; nfsm_srvmtofh(ffhp); nfsm_srvnamesiz(len); @@ -1384,9 +1383,10 @@ nfsrv_rename(struct nfsrv_descript *nfsd, struct nfssvc_sock *slp, * the second nfs_namei() call, in case it is remapped. */ saved_uid = cred->cr_uid; + + NDINIT(&fromnd, DELETE, WANTPARENT | SAVESTART, UIO_SYSSPACE, NULL, + procp); fromnd.ni_cnd.cn_cred = cred; - fromnd.ni_cnd.cn_nameiop = DELETE; - fromnd.ni_cnd.cn_flags = WANTPARENT | SAVESTART; error = nfs_namei(&fromnd, ffhp, len, slp, nam, &info.nmi_md, &info.nmi_dpos, &fdirp, procp); if (fdirp) { @@ -1413,9 +1413,10 @@ nfsrv_rename(struct nfsrv_descript *nfsd, struct nfssvc_sock *slp, nfsm_srvmtofh(tfhp); nfsm_strsiz(len2, NFS_MAXNAMLEN); cred->cr_uid = saved_uid; + + NDINIT(&tond, RENAME, LOCKPARENT | LOCKLEAF| NOCACHE | SAVESTART, + UIO_SYSSPACE, NULL, procp); tond.ni_cnd.cn_cred = cred; - tond.ni_cnd.cn_nameiop = RENAME; - tond.ni_cnd.cn_flags = LOCKPARENT | LOCKLEAF | NOCACHE | SAVESTART; error = nfs_namei(&tond, tfhp, len2, slp, nam, &info.nmi_md, &info.nmi_dpos, &tdirp, procp); if (tdirp) { @@ -1579,9 +1580,9 @@ nfsrv_link(struct nfsrv_descript *nfsd, struct nfssvc_sock *slp, } if (vp->v_type == VDIR && (error = suser_ucred(cred)) != 0) goto out1; + + NDINIT(&nd, CREATE, LOCKPARENT, UIO_SYSSPACE, NULL, procp); nd.ni_cnd.cn_cred = cred; - nd.ni_cnd.cn_nameiop = CREATE; - nd.ni_cnd.cn_flags = LOCKPARENT; error = nfs_namei(&nd, dfhp, len, slp, nam, &info.nmi_md, &info.nmi_dpos, &dirp, procp); if (dirp) { @@ -1663,14 +1664,12 @@ nfsrv_symlink(struct nfsrv_descript *nfsd, struct nfssvc_sock *slp, info.nmi_dpos = nfsd->nd_dpos; info.nmi_v3 = (nfsd->nd_flag & ND_NFSV3); - nd.ni_cnd.cn_nameiop = 0; fhp = &nfh.fh_generic; nfsm_srvmtofh(fhp); nfsm_srvnamesiz(len); + NDINIT(&nd, CREATE, LOCKPARENT | SAVESTART, UIO_SYSSPACE, NULL, procp); nd.ni_cnd.cn_cred = cred; - nd.ni_cnd.cn_nameiop = CREATE; - nd.ni_cnd.cn_flags = LOCKPARENT | SAVESTART; error = nfs_namei(&nd, fhp, len, slp, nam, &info.nmi_md, &info.nmi_dpos, &dirp, procp); if (dirp) { @@ -1813,9 +1812,8 @@ nfsrv_mkdir(struct nfsrv_descript *nfsd, struct nfssvc_sock *slp, nfsm_srvmtofh(fhp); nfsm_srvnamesiz(len); + NDINIT(&nd, CREATE, LOCKPARENT, UIO_SYSSPACE, NULL, procp); nd.ni_cnd.cn_cred = cred; - nd.ni_cnd.cn_nameiop = CREATE; - nd.ni_cnd.cn_flags = LOCKPARENT; error = nfs_namei(&nd, fhp, len, slp, nam, &info.nmi_md, &info.nmi_dpos, &dirp, procp); if (dirp) { @@ -1929,9 +1927,9 @@ nfsrv_rmdir(struct nfsrv_descript *nfsd, struct nfssvc_sock *slp, fhp = &nfh.fh_generic; nfsm_srvmtofh(fhp); nfsm_srvnamesiz(len); + + NDINIT(&nd, DELETE, LOCKPARENT | LOCKLEAF, UIO_SYSSPACE, NULL, procp); nd.ni_cnd.cn_cred = cred; - nd.ni_cnd.cn_nameiop = DELETE; - nd.ni_cnd.cn_flags = LOCKPARENT | LOCKLEAF; error = nfs_namei(&nd, fhp, len, slp, nam, &info.nmi_md, &info.nmi_dpos, &dirp, procp); if (dirp) { diff --git a/sys/nfs/nfs_subs.c b/sys/nfs/nfs_subs.c index 97f7d6e8352..35ad17e64fb 100644 --- a/sys/nfs/nfs_subs.c +++ b/sys/nfs/nfs_subs.c @@ -1,4 +1,4 @@ -/* $OpenBSD: nfs_subs.c,v 1.130 2016/03/19 12:04:16 natano Exp $ */ +/* $OpenBSD: nfs_subs.c,v 1.131 2016/04/29 14:40:36 beck Exp $ */ /* $NetBSD: nfs_subs.c,v 1.27.4.3 1996/07/08 20:34:24 jtc Exp $ */ /* @@ -1244,6 +1244,13 @@ nfs_namei(struct nameidata *ndp, fhandle_t *fhp, int len, cnp->cn_flags |= (NOCROSSMOUNT | RDONLY); else cnp->cn_flags |= NOCROSSMOUNT; + + /* + * Should be 0, if not someone didn't init ndp with NDINIT, + * go find and murder the offender messily. + */ + KASSERT (ndp->ni_p_path == NULL && ndp->ni_p_size == 0); + /* * And call lookup() to do the real work */ diff --git a/sys/nfs/nfs_var.h b/sys/nfs/nfs_var.h index 56a52ac9d95..e11818aa19a 100644 --- a/sys/nfs/nfs_var.h +++ b/sys/nfs/nfs_var.h @@ -1,4 +1,4 @@ -/* $OpenBSD: nfs_var.h,v 1.60 2013/06/11 16:42:17 deraadt Exp $ */ +/* $OpenBSD: nfs_var.h,v 1.61 2016/04/29 14:40:36 beck Exp $ */ /* $NetBSD: nfs_var.h,v 1.3 1996/02/18 11:53:54 fvdl Exp $ */ /* @@ -95,7 +95,8 @@ int nfs_mkdir(void *); int nfs_rmdir(void *); int nfs_readdir(void *); int nfs_readdirrpc(struct vnode *, struct uio *, struct ucred *, int *); -int nfs_readdirplusrpc(struct vnode *, struct uio *, struct ucred *, int *); +int nfs_readdirplusrpc(struct vnode *, struct uio *, struct ucred *, int *, + struct proc *); int nfs_sillyrename(struct vnode *, struct vnode *, struct componentname *); int nfs_lookitup(struct vnode *, char *, int, struct ucred *, diff --git a/sys/nfs/nfs_vnops.c b/sys/nfs/nfs_vnops.c index 91d6359c388..4c58ebff904 100644 --- a/sys/nfs/nfs_vnops.c +++ b/sys/nfs/nfs_vnops.c @@ -1,4 +1,4 @@ -/* $OpenBSD: nfs_vnops.c,v 1.168 2016/03/19 12:04:16 natano Exp $ */ +/* $OpenBSD: nfs_vnops.c,v 1.169 2016/04/29 14:40:36 beck Exp $ */ /* $NetBSD: nfs_vnops.c,v 1.62.4.1 1996/07/08 20:26:52 jtc Exp $ */ /* @@ -2008,7 +2008,7 @@ nfs_readdir(void *v) if (nmp->nm_flag & NFSMNT_RDIRPLUS) { error = nfs_readdirplusrpc(vp, &readdir_uio, cred, - &eof); + &eof, p); if (error == NFSERR_NOTSUPP) nmp->nm_flag &= ~NFSMNT_RDIRPLUS; } @@ -2260,7 +2260,7 @@ nfsmout: */ int nfs_readdirplusrpc(struct vnode *vp, struct uio *uiop, struct ucred *cred, - int *end_of_directory) + int *end_of_directory, struct proc *p) { int len, left; struct nfs_dirent *ndirp = NULL; @@ -2287,6 +2287,7 @@ nfs_readdirplusrpc(struct vnode *vp, struct uio *uiop, struct ucred *cred, (uiop->uio_resid & (NFS_DIRBLKSIZ - 1))) panic("nfs readdirplusrpc bad uio"); #endif + NDINIT(ndp, 0, 0, UIO_SYSSPACE, NULL, p); ndp->ni_dvp = vp; newvp = NULLVP; diff --git a/sys/sys/namei.h b/sys/sys/namei.h index 799d9cd90c0..e69984ecd68 100644 --- a/sys/sys/namei.h +++ b/sys/sys/namei.h @@ -1,4 +1,4 @@ -/* $OpenBSD: namei.h,v 1.31 2016/04/28 14:25:08 beck Exp $ */ +/* $OpenBSD: namei.h,v 1.32 2016/04/29 14:40:36 beck Exp $ */ /* $NetBSD: namei.h,v 1.11 1996/02/09 18:25:20 christos Exp $ */ /* @@ -153,17 +153,14 @@ struct nameidata { /* * Initialization of an nameidata structure. */ -#define NDINITAT(ndp, op, flags, segflg, dirfd, namep, p) { \ - (ndp)->ni_cnd.cn_nameiop = op; \ - (ndp)->ni_cnd.cn_flags = flags; \ - (ndp)->ni_segflg = segflg; \ - (ndp)->ni_dirfd = dirfd; \ - (ndp)->ni_dirp = namep; \ - (ndp)->ni_cnd.cn_proc = p; \ - (ndp)->ni_pledge = 0; \ -} +void ndinitat(struct nameidata *ndp, u_long op, u_long flags, + enum uio_seg segflg, int dirfd, const char *namep, struct proc *p); + +#define NDINITAT(ndp, op, flags, segflg, dirfd, namep, p) \ + ndinitat(ndp, op, flags, segflg, dirfd, namep, p) + #define NDINIT(ndp, op, flags, segflp, namep, p) \ - NDINITAT(ndp, op, flags, segflp, AT_FDCWD, namep, p) + ndinitat(ndp, op, flags, segflp, AT_FDCWD, namep, p) /* Defined for users of NDINIT(). */ #define AT_FDCWD -100 |