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