Re: Removal of some KAUTH_GENERIC_ISSUSER (pass 1)

[Andrew Doran <ad%netbsd.org@localhost>]

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.

> > + ? ? ? ? ? ? ? /*
> > + ? ? ? ? ? ? ? ?* According to SCO documentation, this is the startup 
> > process
> > + ? ? ? ? ? ? ? ?* trying to remount the root file-system.
> > + ? ? ? ? ? ? ? ?*/
> > + ? ? ? ? ? ? ? struct mount *rootfs = CIRCLEQ_FIRST(&mountlist);
> > +
> > + ? ? ? ? ? ? ? /* Make sure this is the root file-system. */
> > + ? ? ? ? ? ? ? if (!(rootfs->mnt_flag & MNT_ROOTFS)) {
> > + ? ? ? ? ? ? ? ? ? ? ? return (EINVAL);
> > + ? ? ? ? ? ? ? }
> >
> > Why check this? In any case, SCO_A_REMOUNT is ignored.
> 
> Well, it is ignored because it's not implemented

Right.

> (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.
- SCO_A_REMOUNT does nothing, and will almost certainly never do anything,
  therefore there is not much point to checking anything.

Also FYI:

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