Subject: Re: kern/35278: veriexec sometimes feeds user va to log(9)
To: None <kern-bug-people@netbsd.org, gnats-admin@netbsd.org,>
From: YAMAMOTO Takashi <yamt@mwd.biglobe.ne.jp>
List: netbsd-bugs
Date: 12/23/2006 10:00:06
The following reply was made to PR kern/35278; it has been noted by GNATS.

From: yamt@mwd.biglobe.ne.jp (YAMAMOTO Takashi)
To: elad@NetBSD.org
Cc: gnats-bugs@NetBSD.org
Subject: Re: kern/35278: veriexec sometimes feeds user va to log(9)
Date: Sat, 23 Dec 2006 18:58:48 +0900 (JST)

 > 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'.
 
 i don't like "returning NULL means EFAULT".
 it can fail due to other reasons.  it's better to return errno.
 
 >   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).
 
 > +struct pathname_internal {
 > +	const char *pathbuf;
 > +	boolean_t needfree;
 
 there is no point to make pathbuf const.
 
 > +const char *
 > +pathname_path(pathname_t path)
 > +{
 > +	return (path == NULL ? NULL : path->pathbuf);
 > +}
 
 why do you want to handle path==NULL here?
 
 > +void
 > +pathname_put(pathname_t path)
 > +{
 > +	if ((path != NULL) && (path->pathbuf != NULL) && path->needfree) {
 > +		PNBUF_PUT(__UNCONST(path->pathbuf));
 > +		free(path, M_TEMP);
 > +	}
 > +}
 
 you need to free "path" even when you don't need to free "path->pathbuf".
 
 YAMAMOTO Takashi