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--