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: <CAOQ4uxgyC=h4+kXvem8nDf0Niu-HgswoamxYnFXz03K5dFe6Zw@mail.gmail.com>
Date: Thu, 7 Nov 2024 12:19:56 +0100
From: Amir Goldstein <amir73il@...il.com>
To: Song Liu <songliubraving@...a.com>
Cc: 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 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).

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.

Thanks,
Amir.