This is a multi-part message in MIME format.

--Boundary_(ID_DQnrklwpftVMIMXUBRFCKw)
Content-type: text/plain; charset=ISO-8859-1
Content-transfer-encoding: 7BIT

YAMAMOTO Takashi wrote:
>>> how about:
>>>
>>> kauth_authorize_device(cred, KAUTH_DEVICE_RAWIO_DISK, dev, vp);
>>>
>>> kauth_authorize_device(cred, KAUTH_DEVICE_RAWIO_MEMORY, dev, vp);
>>>
>>> kauth_authorize_device(cred, KAUTH_DEVICE_RAWIO_PASSTHRU, dev, some_data);
>> my concern is that we're losing the access (r/w/rw) with that, which is
>> important for policy decisions.
>>
>>> or, unify KAUTH_DEVICE_RAWIO_DISK and KAUTH_DEVICE_RAWIO_MEMORY and
>>> export an iskmemdev() equivalent so that listeners can check if
>>> it's a kmem access or not by themselves.
>> that's actually something I've been wanting to do. I like it.
>>
>>> i'm not sure why RAWIO_DISK needs both of dev and vp.
>>> isn't it simpler just to have listers use vfinddev or vp->v_rdev if necessary?
>> can we guarantee that we will have, in most cases, either dev or vp for
>> the disk device? if yes, choosing one of them should be enough. ideally
>> the vp would be best, imho, but either works (as you say, vfinddev() can
>> be used).
> 
> then, how about:
> 
> kauth_authorize_device(cred, KAUTH_DEVICE_RAWIO, r/w/passthru, vp, some_data);
> 
> i wonder what's the status of devvp branch.

we can only do the above if we can gurantee the vp at all times... for
now I think we should pass dev:

kauth_authorize_device(cred, KAUTH_DEVICE_RAWIO, r/w/passthru, dev,
    some_data);

(and we note in the man-page listeners for that request should use
iskmemdev())

and, that allows us to make kauth_authorize_device_tty() a wrapper
around that one, and not directly call kauth_authorize_action() if we
want.

>>> btw, your last changes to spec_open seems incomplete.
>>> 	- it does vfinddev but don't use the result (bvp) unless NVERIEXEC > 1.
>> is attached diff okay?
> 
> i thought KAUTH_REQ_SYSTEM_RAWIO_DISK excepts vp, not bvp.
> i'm not sure the veriexec part.

right, we're already doing the vfinddev() inside the secmodel code.
attached new diff that removes the bvp/blkdev usage for usual cases
and only uses them for veriexec.

-e.

-- 
Elad Efrat

--Boundary_(ID_DQnrklwpftVMIMXUBRFCKw)
Content-type: text/plain; name=spec.diff
Content-transfer-encoding: 7BIT
Content-disposition: inline; filename=spec.diff

Index: spec_vnops.c
===================================================================
RCS file: /cvsroot/src/sys/miscfs/specfs/spec_vnops.c,v
retrieving revision 1.93
diff -u -p -r1.93 spec_vnops.c
--- spec_vnops.c	30 Oct 2006 12:19:23 -0000	1.93
+++ spec_vnops.c	31 Oct 2006 11:04:02 -0000
@@ -179,10 +179,10 @@ spec_open(v)
 		struct lwp *a_l;
 	} */ *ap = v;
 	struct lwp *l = ap->a_l;
-	struct vnode *bvp, *vp = ap->a_vp;
+	struct vnode *vp = ap->a_vp;
 	const struct bdevsw *bdev;
 	const struct cdevsw *cdev;
-	dev_t blkdev, dev = (dev_t)vp->v_rdev;
+	dev_t dev = (dev_t)vp->v_rdev;
 	int error;
 	struct partinfo pi;
 	int (*d_ioctl)(dev_t, u_long, caddr_t, int, struct lwp *);
@@ -209,7 +209,6 @@ spec_open(v)
 
 			rw = M2K(ap->a_mode);
 			error = 0;
-			bvp = NULL;
 
 			/* XXX we're holding a vnode lock here */
 			if (iskmemdev(dev)) {
@@ -218,25 +217,32 @@ spec_open(v)
 				    KAUTH_REQ_SYSTEM_RAWIO_MEMORY,
 				    (void *)rw, NULL, NULL);
 			} else {
-				blkdev = devsw_chr2blk(dev);
-				if (blkdev != (dev_t)NODEV) {
-					vfinddev(blkdev, VBLK, &bvp);
-					error = kauth_authorize_system(ap->a_cred,
-					    KAUTH_SYSTEM_RAWIO,
-					    KAUTH_REQ_SYSTEM_RAWIO_DISK,
-					    (void *)rw, vp, (void *)(u_long)dev);
-				}
+				error = kauth_authorize_system(ap->a_cred,
+				    KAUTH_SYSTEM_RAWIO,
+				    KAUTH_REQ_SYSTEM_RAWIO_DISK,
+				    (void *)rw, vp, (void *)(u_long)dev);
 			}
 
 			if (error)
 				return (error);
 
 #if NVERIEXEC > 0
-			if (veriexec_strict >= VERIEXEC_IPS && iskmemdev(dev))
-				return (error);
-			error = veriexec_rawchk(bvp);
-			if (error)
+			if (veriexec_strict >= VERIEXEC_IPS &&
+			    iskmemdev(dev) && (ap->a_mode & FWRITE)) {
 				return (error);
+			} else {
+				struct vnode *bvp;
+				dev_t blkdev;
+
+				blkdev = devsw_chr2blk(dev);
+				if (blkdev != NODEV) {
+					bvp = NULL;
+					vfinddev(blkdev, VBLK, &bvp);
+					error = veriexec_rawchk(bvp);
+					if (error)
+						return (error);
+				}
+			}
 #endif /* NVERIEXEC > 0 */
 		}
 

--Boundary_(ID_DQnrklwpftVMIMXUBRFCKw)--