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: <E48C3CBB-7712-4707-AE70-1326445CE4C4@fb.com>
Date: Fri, 15 Nov 2024 17:11:27 +0000
From: Song Liu <songliubraving@...a.com>
To: Amir Goldstein <amir73il@...il.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

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?

Thanks,
Song