linux-kernel - Re: [RFC bpf-next fanotify 2/5] samples/fanotify: Add a sample 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: <A6B3F486-6392-4A07-85B9-84FD6AA71F97@fb.com>
Date: Thu, 7 Nov 2024 19:39:12 +0000
From: Song Liu <songliubraving@...a.com>
To: Amir Goldstein <amir73il@...il.com>
CC: Song Liu <songliubraving@...a.com>, Jeff Layton <jlayton@...nel.org>,
        Song
 Liu <song@...nel.org>, bpf <bpf@...r.kernel.org>,
        Linux-Fsdevel
	<linux-fsdevel@...r.kernel.org>,
        LKML <linux-kernel@...r.kernel.org>,
        Kernel
 Team <kernel-team@...a.com>,
        Andrii Nakryiko <andrii@...nel.org>,
        Eduard
 Zingerman <eddyz87@...il.com>,
        Alexei Starovoitov <ast@...nel.org>,
        Daniel
 Borkmann <daniel@...earbox.net>,
        Martin KaFai Lau <martin.lau@...ux.dev>,
        Al
 Viro <viro@...iv.linux.org.uk>,
        Christian Brauner <brauner@...nel.org>, Jan
 Kara <jack@...e.cz>,
        KP Singh <kpsingh@...nel.org>,
        Matt Bobrowski
	<mattbobrowski@...gle.com>,
        "repnop@...gle.com" <repnop@...gle.com>,
        Josef
 Bacik <josef@...icpanda.com>
Subject: Re: [RFC bpf-next fanotify 2/5] samples/fanotify: Add a sample
 fanotify fastpath handler



> On Nov 7, 2024, at 3:19 AM, Amir Goldstein <amir73il@...il.com> wrote:
> 
> On Thu, Oct 31, 2024 at 2:52 AM Song Liu <songliubraving@...a.com> wrote:
>> 
>> Hi Jeff,
>> 
>>> On Oct 30, 2024, at 5:23 PM, Jeff Layton <jlayton@...nel.org> wrote:
>> 
>> [...]
>> 
>>>> If the subtree is all in the same file system, we can attach fanotify to
>>>> the whole file system, and then use some dget_parent() and follow_up()
>>>> to walk up the directory tree in the fastpath handler. However, if there
>>>> are other mount points in the subtree, we will need more logic to handle
>>>> these mount points.
>>>> 
>>> 
>>> My 2 cents...
>>> 
>>> I'd just confine it to a single vfsmount. If you want to monitor in
>>> several submounts, then you need to add new fanotify watches.
>>> 
>>> Alternately, maybe there is some way to designate that an entire
>>> vfsmount is a child of a watched (or ignored) directory?
>>> 
>>>> @Christian, I would like to know your thoughts on this (walking up the
>>>> directory tree in fanotify fastpath handler). It can be expensive for
>>>> very very deep subtree.
>>>> 
>>> 
>>> I'm not Christian, but I'll make the case for it. It's basically a
>>> bunch of pointer chasing. That's probably not "cheap", but if you can
>>> do it under RCU it might not be too awful. It might still suck with
>>> really deep paths, but this is a sample module. It's not expected that
>>> everyone will want to use it anyway.
>> 
>> Thanks for the suggestion! I will try to do it under RCU.
>> 
>>> 
>>>> How should we pass in the subtree? I guess we can just use full path in
>>>> a string as the argument.
>>>> 
>>> 
>>> I'd stay away from string parsing. How about this instead?
>>> 
>>> Allow a process to open a directory fd, and then hand that fd to an
>>> fanotify ioctl that says that you want to ignore everything that has
>>> that directory as an ancestor. Or, maybe make it so that you only watch
>>> dentries that have that directory as an ancestor? I'm not sure what
>>> makes the most sense.
>> 
>> Yes, directory fd is another option. Currently, the "attach to group"
>> function only takes a string as input. I guess it makes sense to allow
>> taking a fd, or maybe we should allow any random format (pass in a
>> pointer to a structure. Let me give it a try.
>> 
> 
> IIUC, the BFP program example uses another API to configure the filter
> (i.e. the inode map).

With BPF, the users can configure the filter via different BPF maps. 
The inode map is just one example, we can also use task map to create
a different filter for each task (task that generates the event). 

> IMO, passing any single argument during setup time is not scalable
> and any filter should have its own way to reconfigure its parameters
> in runtime (i.e. add/remove watched subtree).
> 
> Assuming that the same module/bfp_prog serves multiple fanotify
> groups and each group may have a different filter config, I think that
> passing an integer arg to identify the config (be it fd or something else)
> is the most we need for this minimal API.
> If we need something more elaborate, we can extend the ioctl size
> or add a new ioctl later.

With my local code, which is slightly different to the RFC, I have 
the ioctl pass in a pointer to fanotify_fastpath_args. 

struct fanotify_fastpath_args {
        char name[FAN_FP_NAME_MAX];

        __u32 version;
        __u32 flags;

        /*
         * user space pointer to the init args of fastpath handler,
         * up to init_args_len (<= FAN_FP_ARGS_MAX).
         */
        __u64 init_args;
        /* size of init_args */
        __u32 init_args_size;
} __attribute__((__packed__));

fanotify_fastpath_args->init_args is a user pointer to a custom (per
fast path) structure. Then fanotify_fastpath_args->init_args will be 
passed to fanotify_fastpath_ops->fp_init(). 

I think this is flexible enough for the "attach fast path to a group"
operation. If we want to reconfigure the fast path later, we may 
need another API. 

Thanks,
Song