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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAOQ4uxg5KmZMbqxYoTKKGWeHW_irjsr70gexwNnCt4KVRMuDyw@mail.gmail.com>
Date: Fri, 15 Nov 2024 18:32:21 +0100
From: Amir Goldstein <amir73il@...il.com>
To: Song Liu <songliubraving@...a.com>
Cc: Song Liu <song@...nel.org>, "bpf@...r.kernel.org" <bpf@...r.kernel.org>, 
	"linux-fsdevel@...r.kernel.org" <linux-fsdevel@...r.kernel.org>, 
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>, 
	"linux-security-module@...r.kernel.org" <linux-security-module@...r.kernel.org>, Kernel Team <kernel-team@...a.com>, 
	"andrii@...nel.org" <andrii@...nel.org>, "eddyz87@...il.com" <eddyz87@...il.com>, "ast@...nel.org" <ast@...nel.org>, 
	"daniel@...earbox.net" <daniel@...earbox.net>, "martin.lau@...ux.dev" <martin.lau@...ux.dev>, 
	"viro@...iv.linux.org.uk" <viro@...iv.linux.org.uk>, "brauner@...nel.org" <brauner@...nel.org>, 
	"jack@...e.cz" <jack@...e.cz>, "kpsingh@...nel.org" <kpsingh@...nel.org>, 
	"mattbobrowski@...gle.com" <mattbobrowski@...gle.com>, "repnop@...gle.com" <repnop@...gle.com>, 
	"jlayton@...nel.org" <jlayton@...nel.org>, Josef Bacik <josef@...icpanda.com>, 
	"mic@...ikod.net" <mic@...ikod.net>, "gnoack@...gle.com" <gnoack@...gle.com>
Subject: Re: [RFC/PATCH v2 bpf-next fanotify 1/7] fanotify: Introduce fanotify
 fastpath handler

On Fri, Nov 15, 2024 at 6:11 PM Song Liu <songliubraving@...a.com> wrote:
>
> Hi Amir,
>
> > On Nov 15, 2024, at 12:51 AM, Amir Goldstein <amir73il@...il.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.