Subject: Re: eliminating veriexec #ifdefs in vfs_vnops.c
To: YAMAMOTO Takashi <yamt@mwd.biglobe.ne.jp>
From: Elad Efrat <elad@NetBSD.org>
List: tech-kern
Date: 12/30/2006 02:21:43
This is a multi-part message in MIME format.
--------------030202020204020901070700
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: 7bit
YAMAMOTO Takashi wrote:
>> attached is a new diff. one namei().
>>
>> -e.
>
> the userland pathname buffer can be modified between
> namei() and pathname_get(). is it ok for you?
>
> YAMAMOTO Takashi
errr - yeah, forgot to do that. must be time (I'm waiting for a scrubs
re-run, you see). see attached diff.
-e.
--------------030202020204020901070700
Content-Type: text/plain;
name="openchk.diff"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
filename="openchk.diff"
Index: sys/verified_exec.h
===================================================================
RCS file: /usr/cvs/src/sys/sys/verified_exec.h,v
retrieving revision 1.47
diff -u -p -r1.47 verified_exec.h
--- sys/verified_exec.h 26 Dec 2006 07:50:40 -0000 1.47
+++ sys/verified_exec.h 28 Dec 2006 13:51:26 -0000
@@ -122,6 +122,7 @@ int veriexec_removechk(struct vnode *, c
int veriexec_renamechk(struct vnode *, const char *, struct vnode *,
const char *, struct lwp *);
int veriexec_unmountchk(struct mount *);
+int veriexec_openchk(struct lwp *, struct vnode *, const char *, int);
#endif /* _KERNEL */
#endif /* !_SYS_VERIFIED_EXEC_H_ */
Index: kern/vfs_vnops.c
===================================================================
RCS file: /usr/cvs/src/sys/kern/vfs_vnops.c,v
retrieving revision 1.129
diff -u -p -r1.129 vfs_vnops.c
--- kern/vfs_vnops.c 30 Nov 2006 01:09:47 -0000 1.129
+++ kern/vfs_vnops.c 28 Dec 2006 13:50:51 -0000
@@ -104,29 +104,7 @@ vn_open(struct nameidata *ndp, int fmode
kauth_cred_t cred = l->l_cred;
struct vattr va;
int error;
-#if NVERIEXEC > 0
- const char *pathbuf;
- char *tmppathbuf;
- boolean_t veriexec_monitored = FALSE;
-#endif /* NVERIEXEC > 0 */
-
-#if NVERIEXEC > 0
- if (ndp->ni_segflg == UIO_USERSPACE) {
- tmppathbuf = PNBUF_GET();
- error = copyinstr(ndp->ni_dirp, tmppathbuf, MAXPATHLEN,
- NULL);
- if (error) {
- if (veriexec_verbose >= 1)
- log(LOG_NOTICE, "Veriexec: Can't copy path."
- " (error=%d)\n", error);
- goto bad2;
- }
- pathbuf = tmppathbuf;
- } else {
- tmppathbuf = NULL;
- pathbuf = ndp->ni_dirp;
- }
-#endif /* NVERIEXEC > 0 */
+ pathname_t pn = NULL;
restart:
if (fmode & O_CREAT) {
@@ -135,23 +113,31 @@ restart:
if ((fmode & O_EXCL) == 0 &&
((fmode & O_NOFOLLOW) == 0))
ndp->ni_cnd.cn_flags |= FOLLOW;
- if ((error = namei(ndp)) != 0)
- goto bad2;
- if (ndp->ni_vp == NULL) {
-#if NVERIEXEC > 0
- /* Lockdown mode: Prevent creation of new files. */
- if (veriexec_strict >= VERIEXEC_LOCKDOWN) {
- VOP_ABORTOP(ndp->ni_dvp, &ndp->ni_cnd);
+ } else {
+ ndp->ni_cnd.cn_nameiop = LOOKUP;
+ ndp->ni_cnd.cn_flags = LOCKLEAF;
+ if ((fmode & O_NOFOLLOW) == 0)
+ ndp->ni_cnd.cn_flags |= FOLLOW;
+ }
+ error = pathname_get(ndp->ni_dirp, ndp->ni_segflg, &pn);
+ if (error)
+ goto bad2;
+ ndp->ni_dirp = pathname_path(pn);
+ ndp->ni_segflg = UIO_SYSSPACE;
+ error = namei(ndp);
+ if (error)
+ goto bad2;
- log(LOG_ALERT, "Veriexec: Preventing "
- "new file creation in %s.\n", pathbuf);
+ vp = ndp->ni_vp;
- vp = ndp->ni_dvp;
- error = EPERM;
- goto bad;
- }
+#if NVERIEXEC > 0
+ error = veriexec_openchk(l, ndp->ni_vp, ndp->ni_dirp, fmode);
+ if (error)
+ goto bad;
#endif /* NVERIEXEC > 0 */
+ if (fmode & O_CREAT) {
+ if (ndp->ni_vp == NULL) {
VATTR_NULL(&va);
va.va_type = VREG;
va.va_mode = cmode;
@@ -188,12 +174,6 @@ restart:
fmode &= ~O_CREAT;
}
} else {
- ndp->ni_cnd.cn_nameiop = LOOKUP;
- ndp->ni_cnd.cn_flags = LOCKLEAF;
- if ((fmode & O_NOFOLLOW) == 0)
- ndp->ni_cnd.cn_flags |= FOLLOW;
- if ((error = namei(ndp)) != 0)
- goto bad2;
vp = ndp->ni_vp;
}
if (vp->v_type == VSOCK) {
@@ -206,12 +186,6 @@ restart:
}
if ((fmode & O_CREAT) == 0) {
-#if NVERIEXEC > 0
- if ((error = veriexec_verify(l, vp, pathbuf, VERIEXEC_FILE,
- &veriexec_monitored)) != 0)
- goto bad;
-#endif /* NVERIEXEC > 0 */
-
if (fmode & FREAD) {
if ((error = VOP_ACCESS(vp, VREAD, cred, l)) != 0)
goto bad;
@@ -225,45 +199,10 @@ restart:
if ((error = vn_writechk(vp)) != 0 ||
(error = VOP_ACCESS(vp, VWRITE, cred, l)) != 0)
goto bad;
-#if NVERIEXEC > 0
- if (veriexec_monitored) {
- veriexec_report("Write access request.",
- pathbuf, l, REPORT_ALWAYS|REPORT_ALARM);
-
- /* IPS mode: Deny writing to monitored files. */
- if (veriexec_strict >= VERIEXEC_IPS) {
- error = EPERM;
- goto bad;
- } else {
- veriexec_purge(vp);
- }
- }
-#endif /* NVERIEXEC > 0 */
}
}
if (fmode & O_TRUNC) {
-#if NVERIEXEC > 0
- if ((error = veriexec_verify(l, vp, pathbuf, VERIEXEC_FILE,
- &veriexec_monitored)) != 0) {
- /*VOP_UNLOCK(vp, 0);*/
- goto bad;
- }
-
- if (veriexec_monitored) {
- veriexec_report("Truncate access request.", pathbuf, l,
- REPORT_VERBOSE | REPORT_ALARM);
-
- /* IPS mode: Deny truncating monitored files. */
- if (veriexec_strict >= 2) {
- error = EPERM;
- goto bad;
- } else {
- veriexec_purge(vp);
- }
- }
-#endif /* NVERIEXEC > 0 */
-
VOP_UNLOCK(vp, 0); /* XXX */
if ((error = vn_start_write(vp, &mp, V_WAIT | V_PCATCH)) != 0) {
@@ -294,10 +233,7 @@ bad:
vput(vp);
bad2:
-#if NVERIEXEC > 0
- if (tmppathbuf != NULL)
- PNBUF_PUT(tmppathbuf);
-#endif /* NVERIEXEC > 0 */
+ pathname_put(pn);
return (error);
}
Index: kern/kern_verifiedexec.c
===================================================================
RCS file: /usr/cvs/src/sys/kern/kern_verifiedexec.c,v
retrieving revision 1.87
diff -u -p -r1.87 kern_verifiedexec.c
--- kern/kern_verifiedexec.c 29 Dec 2006 11:34:14 -0000 1.87
+++ kern/kern_verifiedexec.c 28 Dec 2006 13:52:17 -0000
@@ -64,6 +64,7 @@ __KERNEL_RCSID(0, "$NetBSD: kern_verifie
#include <sys/conf.h>
#include <miscfs/specfs/specdev.h>
#include <prop/proplib.h>
+#include <sys/fcntl.h>
MALLOC_DEFINE(M_VERIEXEC, "Veriexec", "Veriexec data-structures");
@@ -1229,3 +1230,46 @@ veriexec_unmountchk(struct mount *mp)
return (error);
}
+
+int
+veriexec_openchk(struct lwp *l, struct vnode *vp, const char *path, int fmode)
+{
+ boolean_t monitored = FALSE;
+ int error = 0;
+
+ if (vp == NULL) {
+ /* If no creation requested, let this fail normally. */
+ if (!(fmode & O_CREAT)) {
+ error = 0;
+ goto out;
+ }
+
+ /* Lockdown mode: Prevent creation of new files. */
+ if (veriexec_strict >= VERIEXEC_LOCKDOWN) {
+ log(LOG_ALERT, "Veriexec: Preventing new file "
+ "creation in `%s'.\n", path);
+ error = EPERM;
+ }
+
+ goto out;
+ }
+
+ error = veriexec_verify(l, vp, path, VERIEXEC_FILE,
+ &monitored);
+ if (error)
+ goto out;
+
+ if (monitored && ((fmode & FWRITE) || (fmode & O_TRUNC))) {
+ veriexec_report("Write access request.", path, l,
+ REPORT_ALWAYS | REPORT_ALARM);
+
+ /* IPS mode: Deny writing to/truncating monitored files. */
+ if (veriexec_strict >= VERIEXEC_IPS)
+ error = EPERM;
+ else
+ veriexec_purge(vp);
+ }
+
+ out:
+ return (error);
+}
--------------030202020204020901070700--