[RFC/PATCH v2 bpf-next fanotify 1/7] fanotify: Introduce fanotify fastpath handler

[Amir Goldstein]

On Fri, Nov 15, 2024 at 6:11 PM Song Liu <songliubraving at meta.com> wrote:
>
> Hi Amir,
>
> > On Nov 15, 2024, at 12:51 AM, Amir Goldstein <amir73il at gmail.com> wrote:
>
> [...]
>
> >>
> >> +#ifdef CONFIG_FANOTIFY_FASTPATH
> >> +       fp_hook = srcu_dereference(group->fanotify_data.fp_hook, &fsnotify_mark_srcu);
> >> +       if (fp_hook) {
> >> +               struct fanotify_fastpath_event fp_event = {
> >> +                       .mask = mask,
> >> +                       .data = data,
> >> +                       .data_type = data_type,
> >> +                       .dir = dir,
> >> +                       .file_name = file_name,
> >> +                       .fsid = &fsid,
> >> +                       .match_mask = match_mask,
> >> +               };
> >> +
> >> +               ret = fp_hook->ops->fp_handler(group, fp_hook, &fp_event);
> >> +               if (ret == FAN_FP_RET_SKIP_EVENT) {
> >> +                       ret = 0;
> >> +                       goto finish;
> >> +               }
> >> +       }
> >> +#endif
> >> +
> >
> > To me it makes sense that the fastpath module could also return a negative
> > (deny) result for permission events.
>
> Yes, this should just work. And I actually plan to use it.
>
> > Is there a specific reason that you did not handle this or just didn't think
> > of this option?
>
> But I haven't tested permission events yet. At first glance, maybe we just
> need to change the above code a bit, as:
>
>
> >> f (ret == FAN_FP_RET_SKIP_EVENT) {
> >> +                       ret = 0;
> >> +                       goto finish;
> >> +               }
>
> if (ret != FAN_FP_RET_SEND_TO_USERSPACE) {
>         if (ret == FAN_FP_RET_SKIP_EVENT)
>                 ret = 0;
>         goto finish;
> }
>
> Well, I guess we should change the value of FAN_FP_RET_SEND_TO_USERSPACE,
> so that this condition will look better.
>
> We may also consider reorder the code so that we do not call
> fsnotify_prepare_user_wait() when the fastpath handles the event.
>
> Does this look reasonable?

Yes.

Thanks,
Amir.