Re: [PATCH v8 10/19] fanotify: introduce FAN_PRE_ACCESS permission event

[Jan Kara <jack@suse.cz>]

On Thu 21-11-24 19:37:43, Amir Goldstein wrote:
> On Thu, Nov 21, 2024 at 7:31 PM Amir Goldstein <amir73il@gmail.com> wrote:
> > On Thu, Nov 21, 2024 at 5:36 PM Jan Kara <jack@suse.cz> wrote:
> > > On Thu 21-11-24 15:18:36, Amir Goldstein wrote:
> > > > On Thu, Nov 21, 2024 at 11:44 AM Jan Kara <jack@suse.cz> wrote:
> > > > and also always emitted ACCESS_PERM.
> > >
> > > I know that and it's one of those mostly useless events AFAICT.
> > >
> > > > my POC is using that PRE_ACCESS to populate
> > > > directories on-demand, although the functionality is incomplete without the
> > > > "populate on lookup" event.
> > >
> > > Exactly. Without "populate on lookup" doing "populate on readdir" is ok for
> > > a demo but not really usable in practice because you can get spurious
> > > ENOENT from a lookup.
> > >
> > > > > avoid the mistake of original fanotify which had some events available on
> > > > > directories but they did nothing and then you have to ponder hard whether
> > > > > you're going to break userspace if you actually start emitting them...
> > > >
> > > > But in any case, the FAN_ONDIR built-in filter is applicable to PRE_ACCESS.
> > >
> > > Well, I'm not so concerned about filtering out uninteresting events. I'm
> > > more concerned about emitting the event now and figuring out later that we
> > > need to emit it in different places or with some other info when actual
> > > production users appear.
> > >
> > > But I've realized we must allow pre-content marks to be placed on dirs so
> > > that such marks can be placed on parents watching children. What we'd need
> > > to forbid is a combination of FAN_ONDIR and FAN_PRE_ACCESS, wouldn't we?
> >
> > Yes, I think that can work well for now.
> >
> 
> Only it does not require only check at API time that both flags are not
> set, because FAN_ONDIR can be set earlier and then FAN_PRE_ACCESS
> can be added later and vice versa, so need to do this in
> fanotify_may_update_existing_mark() AFAICT.

I have now something like:

@@ -1356,7 +1356,7 @@ static int fanotify_group_init_error_pool(struct fsnotify_group *group)
 }
 
 static int fanotify_may_update_existing_mark(struct fsnotify_mark *fsn_mark,
-                                             unsigned int fan_flags)
+                                            __u32 mask, unsigned int fan_flags)
 {
        /*
         * Non evictable mark cannot be downgraded to evictable mark.
@@ -1383,6 +1383,11 @@ static int fanotify_may_update_existing_mark(struct fsnotify_mark *fsn_mark,
            fsn_mark->flags & FSNOTIFY_MARK_FLAG_IGNORED_SURV_MODIFY)
                return -EEXIST;
 
+       /* For now pre-content events are not generated for directories */
+       mask |= fsn_mark->mask;
+       if (mask & FANOTIFY_PRE_CONTENT_EVENTS && mask & FAN_ONDIR)
+               return -EEXIST;
+
        return 0;
 }
 
So far only compile tested...

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR