In article <20200512235040.gd9...@homeworld.netbsd.org>, Andrew Doran <a...@netbsd.org> wrote: >On Tue, May 12, 2020 at 10:37:27PM +0000, Andrew Doran wrote: >> On Sun, May 10, 2020 at 09:48:10AM -0400, Christos Zoulas wrote: >> > >> > > Maybe I missed it but I didn't see a way for cache_lookup_linked() and >> > > cache_have_id() to know a vnode has ACLs. The presence of ACLs >means those >> > > routines can't do their imitation of VOP_ACCESS() and need to fail so >> > > that >> > > the lookup is handled by VOP_LOOKUP(). To handle that on a >per-vnode basis >> > > you'd want to flag the "has ACLs" fact when an inode is loaded, and do >> > > the >> > > same when an ACL is added to an in-core inode. I'll look into it over >> > > the >> > > next few days unless you want to take care of it. >> > >> > So I looked into this a bit and genfs_can_access() is also used by >cache_revlookup. >> > I am not sure what to do here. It is simple enough to add a flag to >the vnode to indicate >> > if it has ACLs, but is it the right thing to do? >> >> I tried to pull down your patch again over the weekend to take a look but it >> was gone. The best place for a flag would be cache_enter_id() since it >> deals with the necessary synchronisation. I'll see about adding a flag now. >> In addition to the places it's called now it would need to be called >> whenever an inode gains an ACL. > >Ok done should just need to mark the cached ID as invalid when the inode has >/ gains an ACL.
Ok, I made some changes using this in my latest acl patch. Can you please check? https://www.netbsd.org/~christos/acl.diff Best, christos