Subject: Re: kern/35278: veriexec sometimes feeds user va to log(9)
To: None <kern-bug-people@netbsd.org, gnats-admin@netbsd.org,>
From: Elad Efrat <elad@NetBSD.org>
List: netbsd-bugs
Date: 12/19/2006 08:25:02
The following reply was made to PR kern/35278; it has been noted by GNATS.

From: Elad Efrat <elad@NetBSD.org>
To: YAMAMOTO Takashi <yamt@mwd.biglobe.ne.jp>
Cc: gnats-bugs@NetBSD.org
Subject: Re: kern/35278: veriexec sometimes feeds user va to log(9)
Date: Tue, 19 Dec 2006 10:21:30 +0200

 This is a multi-part message in MIME format.
 --------------070903020805020507010304
 Content-Type: text/plain; charset=ISO-8859-1
 Content-Transfer-Encoding: 7bit
 
 YAMAMOTO Takashi wrote:
 >> this is probably because we pass 'ni_dirp' in sys_unlink; possibly other
 >> places too.
 >>
 >> -e.
 > 
 > check_exec does it, at least.  i haven't checked others.
 
 attached diff handles the issue in check_exec (for both veriexec and
 segvguard), sys_unlink, and rename_files.
 
 -e.
 
 --------------070903020805020507010304
 Content-Type: text/plain;
  name="segflg.diff"
 Content-Transfer-Encoding: 7bit
 Content-Disposition: inline;
  filename="segflg.diff"
 
 Index: kern_exec.c
 ===================================================================
 RCS file: /usr/cvs/src/sys/kern/kern_exec.c,v
 retrieving revision 1.232
 diff -u -p -r1.232 kern_exec.c
 --- kern_exec.c	22 Nov 2006 02:02:51 -0000	1.232
 +++ kern_exec.c	18 Dec 2006 05:35:11 -0000
 @@ -251,6 +251,10 @@ check_exec(struct lwp *l, struct exec_pa
  	struct vnode	*vp;
  	struct nameidata *ndp;
  	size_t		resid;
 +#if NVERIEXEC > 0 || defined(PAX_SEGVGUARD)
 +	const char *pathbuf;
 +	char *tmppathbuf;
 +#endif /* NVERIEXEC > 0 || PAX_SEGVGUARD */
  
  	ndp = epp->ep_ndp;
  	ndp->ni_cnd.cn_nameiop = LOOKUP;
 @@ -287,17 +291,44 @@ check_exec(struct lwp *l, struct exec_pa
  	/* unlock vp, since we need it unlocked from here on out. */
  	VOP_UNLOCK(vp, 0);
  
 +#if NVERIEXEC > 0 || defined(PAX_SEGVGUARD)
 +	if (epp->ep_ndp->ni_segflg == UIO_USERSPACE) {
 +		tmppathbuf = PNBUF_GET();
 +		error = copyinstr(epp->ep_ndp->ni_dirp, tmppathbuf, MAXPATHLEN,
 +		    NULL);
 +		if (error) {
 +			PNBUF_PUT(tmppathbuf);
 +			goto bad2;
 +		}
 +		pathbuf = tmppathbuf;
 +	} else {
 +		tmppathbuf = NULL;
 +		pathbuf = epp->ep_ndp->ni_dirp;
 +	}
 +#endif /* NVERIEXEC > 0 || PAX_SEGVGUARD */
 +
  #if NVERIEXEC > 0
 -        if ((error = veriexec_verify(l, vp, epp->ep_ndp->ni_dirp, flag,
 -	    NULL)) != 0)
 -                goto bad2;
 +	error = veriexec_verify(l, vp, pathbuf, flag, NULL);
 +	if (error) {
 +		if (tmppathbuf != NULL)
 +			PNBUF_PUT(tmppathbuf);
 +		goto bad2;
 +	}
  #endif /* NVERIEXEC > 0 */
  
  #ifdef PAX_SEGVGUARD
 -	if (pax_segvguard(l, vp, epp->ep_ndp->ni_dirp, FALSE))
 -		return (EPERM);
 +	if (pax_segvguard(l, vp, pathbuf, FALSE)) {
 +		if (tmppathbuf != NULL)
 +			PNBUF_PUT(tmppathbuf);
 +		goto bad2;
 +	}
  #endif /* PAX_SEGVGUARD */
  
 +#if NVERIEXEC > 0 || defined(PAX_SEGVGUARD)
 +	if (tmppathbuf != NULL)
 +		PNBUF_PUT(tmppathbuf);
 +#endif /* NVERIEXEC > 0 || PAX_SEGVGUARD */
 +
  	/* now we have the file, get the exec header */
  	uvn_attach(vp, VM_PROT_READ);
  	error = vn_rdwr(UIO_READ, vp, epp->ep_hdr, epp->ep_hdrlen, 0,
 Index: vfs_syscalls.c
 ===================================================================
 RCS file: /usr/cvs/src/sys/kern/vfs_syscalls.c,v
 retrieving revision 1.281
 diff -u -p -r1.281 vfs_syscalls.c
 --- vfs_syscalls.c	14 Dec 2006 09:24:54 -0000	1.281
 +++ vfs_syscalls.c	18 Dec 2006 07:02:04 -0000
 @@ -2015,6 +2015,10 @@ sys_unlink(struct lwp *l, void *v, regis
  	struct vnode *vp;
  	int error;
  	struct nameidata nd;
 +#if NVERIEXEC > 0
 +	const char *pathbuf;
 +	char *tmppathbuf;
 +#endif /* NVERIEXEC > 0 */
  
  restart:
  	NDINIT(&nd, DELETE, LOCKPARENT | LOCKLEAF, UIO_USERSPACE,
 @@ -2038,8 +2042,13 @@ restart:
  	}
  
  #if NVERIEXEC > 0
 +	tmppathbuf = PNBUF_GET();
 +	error = copyinstr(nd.ni_dirp, tmppathbuf, MAXPATHLEN, NULL);
 +	pathbuf = tmppathbuf;
 +
  	/* Handle remove requests for veriexec entries. */
 -	if ((error = veriexec_removechk(vp, nd.ni_dirp, l)) != 0) {
 +	if (error || (error = veriexec_removechk(vp, pathbuf, l)) != 0) {
 +		PNBUF_PUT(tmppathbuf);
  		VOP_ABORTOP(nd.ni_dvp, &nd.ni_cnd);
  		if (nd.ni_dvp == vp)
  			vrele(nd.ni_dvp);
 @@ -2048,6 +2057,7 @@ restart:
  		vput(vp);
  		goto out;
  	}
 +	PNBUF_PUT(tmppathbuf);
  #endif /* NVERIEXEC > 0 */
  	
  	if (vn_start_write(nd.ni_dvp, &mp, V_NOWAIT) != 0) {
 @@ -3364,9 +3374,34 @@ rename_files(const char *from, const cha
  	}
  
  #if NVERIEXEC > 0
 -	if (!error)
 -		error = veriexec_renamechk(fvp, fromnd.ni_dirp, tvp,
 -		    tond.ni_dirp, l);
 +	if (!error) {
 +		const char *frompath, *topath;
 +		char *tmpfrom, *tmpto;
 +
 +		tmpfrom = NULL;
 +		tmpto = NULL;
 +
 +		tmpfrom = PNBUF_GET();
 +		error = copyinstr(fromnd.ni_dirp, tmpfrom, MAXPATHLEN, NULL);
 +		if (error)
 +			goto out2;
 +
 +		tmpto = PNBUF_GET();
 +		error = copyinstr(tond.ni_dirp, tmpto, MAXPATHLEN, NULL);
 +		if (error)
 +			goto out2;
 +
 +		frompath = tmpfrom;
 +		topath = tmpto;
 +
 +		error = veriexec_renamechk(fvp, frompath, tvp, topath, l);
 +
 + out2:
 +		if (tmpfrom != NULL)
 +			PNBUF_PUT(tmpfrom);
 +		if (tmpto != NULL)
 +			PNBUF_PUT(tmpto);
 +	}
  #endif /* NVERIEXEC > 0 */
  
  out:
 
 --------------070903020805020507010304--