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: <CAOQ4uxjyN-Sr4mV8EjhAJ9rvx4k4sSRSEFLF08RnT1ijvm2Y-g@mail.gmail.com>
Date: Wed, 6 Nov 2024 20:40:50 +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.
> >

Agreed.

> > 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.
>

That's the cost of doing a subtree filter.
Not sure how it could be avoided?

> >
> >> 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.
>
> Thanks again!
>
> Song
>
> >
> > Then, when you get a dentry-based event, you just walk the d_parent
> > pointers back up to the root of the vfsmount. If one of them matches
> > the dentry in your fd then you're done. If you hit the root of the
> > vfsmount, then you're also done (and know that it's not a child of that
> > dentry).

Agreed.

Thanks,
Amir.