Re: Removal of some KAUTH_GENERIC_ISSUSER (pass 1)

[Elad Efrat <elad%NetBSD.org@localhost>]

On Sun, Apr 12, 2009 at 3:52 PM, Andrew Doran <ad%netbsd.org@localhost> wrote:
> On Sun, Apr 12, 2009 at 02:10:31AM +0300, Elad Efrat wrote:
>
>> On Sun, Apr 12, 2009 at 12:40 AM, Andrew Doran <ad%netbsd.org@localhost> 
>> wrote:
>> > On Sat, Apr 11, 2009 at 04:46:34AM +0300, Elad Efrat wrote:
>> >
>> >> ? - KAUTH_MACHDEP_CACHEFLUSH_ALL, for checking if the whole CPU cache
>> >> ? ? can be flushed (used in compat code only).
>> >
>> > Is there a reason for _ALL (so specific)?
>>
>> Sure: the code where I use this allows flushing the "data" and
>> "instruction" caches as well, and makes a distinction about "all".
>
> Ok. The existing intent check is likely there for performance reasons. In
> this case I don't see the utility of being so granular when it comes to the
> access check.

I too don't have a strong opinion on this, just trying to replace one
thing with another. :) I can just remove the "_ALL", would that be okay?

>> (or, do you mean, even the original SCO code ignores it?).
>
> Your diff includes a seemingly pointless check to see if the first entry of
> 'mountlist' is the root file system and I am querying that.
>
>> The SCO documentation talks about the root file-system, so I figured we
>> should make sure it's the root file-system first...
>
> That would be fine but:
>
> - the diff does not verify that the root file system has been requested.
> - the diff checks for something that is currently an invariant, but something
>  that should not be assumed.

Okay, I figured that was the way to get the root file-system. We
probably won't be needing any, but is there a different way?

> - SCO_A_REMOUNT does nothing, and will almost certainly never do anything,
>  therefore there is not much point to checking anything.

I figured what I did would help whoever wanted to implement it. Do you
think we should just remove the kauth(9) checks from non-implemented
cases and stick in a comment instead? ("use KAUTH_FOO when implementing
this")

> Also FYI:
>
> - mountlist access needs locking.
> - mount access requires a reference held on or implied to one of these
>  objects: descriptor, file, vnode, mount.

Thanks,

-e.