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/21/2006 13:10:02
The following reply was made to PR kern/35278; it has been noted by GNATS.

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

 This is a multi-part message in MIME format.
 --------------010209080404000208090308
 Content-Type: text/plain; charset=ISO-8859-1
 Content-Transfer-Encoding: 7bit
 
 attached are two diffs:
 
 pathname_t.diff, adds type 'pathname_t' and these functions:
   pathname_t pathname_get(const char *dirp, enum uio_seg segflg) - do
     whatever is needed to get the pathname from 'dirp' according to
     'segflg'.
 
   const char *pathname_path(pathname_t path) - return pathname from
     a 'pathname_t'.
 
   void pathname_put(pathname_t) - free memory associated with a
     'pathname_t'. can handle NULL values.
 
   (prototypes and type in namei.h, backend in vfs_lookup.c)
 
 valog.diff, makes use of above interface to ensure no user va is
   passed to log(9).
 
 -e.
 
 --------------010209080404000208090308
 Content-Type: text/plain;
  name="pathname_t.diff"
 Content-Transfer-Encoding: 7bit
 Content-Disposition: inline;
  filename="pathname_t.diff"
 
 Index: kern/vfs_lookup.c
 ===================================================================
 RCS file: /usr/cvs/src/sys/kern/vfs_lookup.c,v
 retrieving revision 1.75
 diff -u -p -r1.75 vfs_lookup.c
 --- kern/vfs_lookup.c	13 Dec 2006 13:36:19 -0000	1.75
 +++ kern/vfs_lookup.c	20 Dec 2006 11:19:42 -0000
 @@ -70,6 +70,11 @@ __KERNEL_RCSID(0, "$NetBSD: vfs_lookup.c
  #define MAGICLINKS 0
  #endif
  
 +struct pathname_internal {
 +	const char *pathbuf;
 +	boolean_t needfree;
 +};
 +
  int vfs_magiclinks = MAGICLINKS;
  
  struct pool pnbuf_pool;		/* pathname buffer pool */
 @@ -191,6 +196,51 @@ symlink_magic(struct proc *p, char *cp, 
  #undef MATCH
  #undef SUBSTITUTE
  
 +pathname_t
 +pathname_get(const char *dirp, enum uio_seg segflg)
 +{
 +	pathname_t path;
 +	int error;
 +
 +	if (dirp == NULL)
 +		return (NULL);
 +
 +	path = malloc(sizeof(struct pathname_internal), M_TEMP,
 +	    M_ZERO|M_WAITOK);
 +
 +	if (segflg == UIO_USERSPACE) {
 +		path->pathbuf = PNBUF_GET();
 +		error = copyinstr(dirp, __UNCONST(path->pathbuf), MAXPATHLEN,
 +		    NULL);
 +		if (error) {
 +			PNBUF_PUT(__UNCONST(path->pathbuf));
 +			free(path, M_TEMP);
 +			return (NULL);
 +		}
 +		path->needfree = TRUE;
 +	} else {
 +		path->pathbuf = dirp;
 +		path->needfree = FALSE;
 +	}
 +
 +	return (path);
 +}
 +
 +const char *
 +pathname_path(pathname_t path)
 +{
 +	return (path == NULL ? NULL : path->pathbuf);
 +}
 +
 +void
 +pathname_put(pathname_t path)
 +{
 +	if ((path != NULL) && (path->pathbuf != NULL) && path->needfree) {
 +		PNBUF_PUT(__UNCONST(path->pathbuf));
 +		free(path, M_TEMP);
 +	}
 +}
 +
  /*
   * Convert a pathname into a pointer to a locked vnode.
   *
 Index: sys/namei.h
 ===================================================================
 RCS file: /usr/cvs/src/sys/sys/namei.h,v
 retrieving revision 1.46
 diff -u -p -r1.46 namei.h
 --- sys/namei.h	9 Dec 2006 16:11:52 -0000	1.46
 +++ sys/namei.h	20 Dec 2006 11:10:01 -0000
 @@ -182,6 +182,8 @@ extern struct pool_cache pnbuf_cache;	/*
  #define	PNBUF_GET()	pool_cache_get(&pnbuf_cache, PR_WAITOK)
  #define	PNBUF_PUT(pnb)	pool_cache_put(&pnbuf_cache, (pnb))
  
 +typedef struct pathname_internal *pathname_t;
 +
  int	namei(struct nameidata *);
  uint32_t namei_hash(const char *, const char **);
  int	lookup(struct nameidata *);
 @@ -199,6 +201,10 @@ void	nchinit(void);
  void	nchreinit(void);
  void	cache_purgevfs(struct mount *);
  void	namecache_print(struct vnode *, void (*)(const char *, ...));
 +
 +pathname_t pathname_get(const char *, enum uio_seg);
 +const char *pathname_path(pathname_t);
 +void pathname_put(pathname_t);
  #endif
  
  /*
 
 --------------010209080404000208090308
 Content-Type: text/plain;
  name="valog.diff"
 Content-Transfer-Encoding: 7bit
 Content-Disposition: inline;
  filename="valog.diff"
 
 Index: kern/kern_exec.c
 ===================================================================
 RCS file: /usr/cvs/src/sys/kern/kern_exec.c,v
 retrieving revision 1.233
 diff -u -p -r1.233 kern_exec.c
 --- kern/kern_exec.c	20 Dec 2006 11:35:29 -0000	1.233
 +++ kern/kern_exec.c	20 Dec 2006 11:41:33 -0000
 @@ -249,6 +249,9 @@ check_exec(struct lwp *l, struct exec_pa
  	struct vnode	*vp;
  	struct nameidata *ndp;
  	size_t		resid;
 +#if NVERIEXEC > 0 || defined(PAX_SEGVGUARD)
 +	pathname_t pathbuf;
 +#endif /* NVERIEXEC > 0 || PAX_SEGVGUARD */
  
  	ndp = epp->ep_ndp;
  	ndp->ni_cnd.cn_nameiop = LOOKUP;
 @@ -285,18 +288,35 @@ 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)
 +	pathbuf = pathname_get(ndp->ni_dirp, ndp->ni_segflg);
 +	if (pathbuf == NULL) {
 +		error = EFAULT;
 +		goto bad2;
 +	}
 +#endif /* NVERIEXEC > 0 || PAX_SEGVGUARD */
 +
  #if NVERIEXEC > 0
 -        if ((error = veriexec_verify(l, vp, epp->ep_ndp->ni_dirp,
 +	error = veriexec_verify(l, vp, pathname_path(pathbuf),
  	    epp->ep_flags & EXEC_INDIR ? VERIEXEC_INDIRECT : VERIEXEC_DIRECT,
 -	    NULL)) != 0)
 +	    NULL);
 +	if (error) {
 +		pathname_put(pathbuf);
                  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, pathname_path(pathbuf), FALSE)) {
 +		pathname_put(pathbuf);
 +		goto bad2;
 +	}
  #endif /* PAX_SEGVGUARD */
  
 +#if NVERIEXEC > 0 || defined(PAX_SEGVGUARD)
 +	pathname_put(pathbuf);
 +#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: kern/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
 --- kern/vfs_syscalls.c	14 Dec 2006 09:24:54 -0000	1.281
 +++ kern/vfs_syscalls.c	20 Dec 2006 11:36:07 -0000
 @@ -2015,6 +2015,9 @@ sys_unlink(struct lwp *l, void *v, regis
  	struct vnode *vp;
  	int error;
  	struct nameidata nd;
 +#if NVERIEXEC > 0
 +	pathname_t pathbuf;
 +#endif /* NVERIEXEC > 0 */
  
  restart:
  	NDINIT(&nd, DELETE, LOCKPARENT | LOCKLEAF, UIO_USERSPACE,
 @@ -2038,8 +2041,17 @@ restart:
  	}
  
  #if NVERIEXEC > 0
 +	pathbuf = pathname_get(nd.ni_dirp, nd.ni_segflg);
 +	if (pathbuf == NULL)
 +		error = EFAULT;
 +
  	/* Handle remove requests for veriexec entries. */
 -	if ((error = veriexec_removechk(vp, nd.ni_dirp, l)) != 0) {
 +	if (!error) {
 +		error = veriexec_removechk(vp, pathname_path(pathbuf), l);
 +		pathname_put(pathbuf);
 +	}
 +
 +	if (error) {
  		VOP_ABORTOP(nd.ni_dvp, &nd.ni_cnd);
  		if (nd.ni_dvp == vp)
  			vrele(nd.ni_dvp);
 @@ -3364,9 +3376,22 @@ 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) {
 +		pathname_t frompath, topath;
 +
 +		frompath = pathname_get(fromnd.ni_dirp, fromnd.ni_segflg);
 +		topath = pathname_get(tond.ni_dirp, tond.ni_segflg);
 +
 +		if (frompath == NULL || topath == NULL)
 +			error = EFAULT;
 +
 +		if (!error)
 +			error = veriexec_renamechk(fvp, pathname_path(frompath),
 +			    tvp, pathname_path(topath), l);
 +
 +		pathname_put(frompath);
 +		pathname_put(topath);
 +	}
  #endif /* NVERIEXEC > 0 */
  
  out:
 
 --------------010209080404000208090308--