Subject: Re: CVS commit: src/sys/secmodel/bsd44
To: YAMAMOTO Takashi <yamt@mwd.biglobe.ne.jp>
From: Elad Efrat <elad@NetBSD.org>
List: tech-kern
Date: 10/30/2006 14:49:16
This is a multi-part message in MIME format.

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

YAMAMOTO Takashi wrote:
>> YAMAMOTO Takashi wrote:
>>>> kauth_authorize_device(cred, KAUTH_DEVICE_RAWIO_MEMORY,
>>>>     KAUTH_REQ_DEVICE_RAWIO_RW, 0 /* dev of /dev/{,k}mem..? */, NULL,
>>>>     NULL)
>>>>
>>>> kauth_authorize_device(cred, KAUTH_DEVICE_RAWIO_DISK,
>>>>     KAUTH_REQ_DEVICE_RAWIO_READ, dev, vp, NULL)
>>>>
>>>> kauth_authorize_device(cred, KAUTH_DEVICE_RAWIO_PASSTHRU,
>>>>     0, dev, some_command_data, NULL)
>>> what's the last NULL argument for?
>> nothing, we can drop it. but the important bit is that with the above we
>> don't need a special passthru routine...
> 
> 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).

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

> 	- printf("nope.\n");

oops! fixed.

-e.

-- 
Elad Efrat

--Boundary_(ID_fwzqtTqYzuoll1yXeI1Bfg)
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.92
diff -u -p -r1.92 spec_vnops.c
--- spec_vnops.c	30 Sep 2006 21:00:13 -0000	1.92
+++ spec_vnops.c	30 Oct 2006 12:47:56 -0000
@@ -224,8 +224,7 @@ spec_open(v)
 					error = kauth_authorize_system(ap->a_cred,
 					    KAUTH_SYSTEM_RAWIO,
 					    KAUTH_REQ_SYSTEM_RAWIO_DISK,
-					    (void *)rw, vp, (void *)(u_long)dev);
-					if (error) printf("nope.\n");
+					    (void *)rw, bvp, (void *)(u_long)dev);
 				}
 			}
 
@@ -233,7 +232,8 @@ spec_open(v)
 				return (error);
 
 #if NVERIEXEC > 0
-			if (veriexec_strict >= VERIEXEC_IPS && iskmemdev(dev))
+			if (veriexec_strict >= VERIEXEC_IPS &&
+			    iskmemdev(dev) && (ap->a_mode & FWRITE))
 				return (error);
 			error = veriexec_rawchk(bvp);
 			if (error)

--Boundary_(ID_fwzqtTqYzuoll1yXeI1Bfg)--