diff mbox series

[RFC,bpf-next,fanotify,2/5] samples/fanotify: Add a sample fanotify fastpath handler

Message ID 20241029231244.2834368-3-song@kernel.org (mailing list archive)
State New
Headers show
Series Fanotify fastpath handler | expand

Commit Message

Song Liu Oct. 29, 2024, 11:12 p.m. UTC 
This fastpath handler filters out events for files with certain prefixes.
To use it:

  [root] insmod fastpath-mod.ko    # This requires root.

  [user] ./fastpath-user /tmp a,b,c &    # Root is not needed
  [user] touch /tmp/aa   # a is in the prefix list (a,b,c), no events
  [user] touch /tmp/xx   # x is not in the prefix list, generates events

  Accessing file xx   # this is the output from fastpath_user

Signed-off-by: Song Liu <song@kernel.org>
---
 MAINTAINERS                      |   1 +
 samples/Kconfig                  |  20 ++++-
 samples/Makefile                 |   2 +-
 samples/fanotify/.gitignore      |   1 +
 samples/fanotify/Makefile        |   5 +-
 samples/fanotify/fastpath-mod.c  | 138 +++++++++++++++++++++++++++++++
 samples/fanotify/fastpath-user.c |  90 ++++++++++++++++++++
 7 files changed, 254 insertions(+), 3 deletions(-)
 create mode 100644 samples/fanotify/fastpath-mod.c
 create mode 100644 samples/fanotify/fastpath-user.c

Comments

Al Viro Oct. 30, 2024, 12:11 a.m. UTC | #1 
On Tue, Oct 29, 2024 at 04:12:41PM -0700, Song Liu wrote:
> +		if (strstr(file_name->name, item->prefix) == (char *)file_name->name)

	Huh?  "Find the first substring (if any) equal to item->prefix and
then check if that happens to be in the very beginning"???

	And you are placing that into the place where it's most likely to cause
the maximal braindamage and spread all over the tree.  Wonderful ;-/

	Where does that "idiom" come from, anyway?  Java?  Not the first time
I see that kind of garbage; typecast is an unusual twist, though...
Al Viro Oct. 30, 2024, 1:48 a.m. UTC | #2 
On Wed, Oct 30, 2024 at 12:11:55AM +0000, Al Viro wrote:
> On Tue, Oct 29, 2024 at 04:12:41PM -0700, Song Liu wrote:
> > +		if (strstr(file_name->name, item->prefix) == (char *)file_name->name)
> 
> 	Huh?  "Find the first substring (if any) equal to item->prefix and
> then check if that happens to be in the very beginning"???
> 
> 	And you are placing that into the place where it's most likely to cause
> the maximal braindamage and spread all over the tree.  Wonderful ;-/
> 
> 	Where does that "idiom" come from, anyway?  Java?  Not the first time
> I see that kind of garbage; typecast is an unusual twist, though...

	After some coproarchaeology: it's probably even worse than java - in
javashit indexOf() predates startsWith() by quite a few years; java itself
had both methods from the very beginning.  So that's probably where it had
been cargo-culted from...

	The thing is, performance requirements of some garbage script from
a javascript-infested webshite are somewhat different from what you want
to see anywhere near the kernel-side filesystem event handling...
Song Liu Oct. 30, 2024, 2:07 a.m. UTC | #3 
> On Oct 29, 2024, at 5:11 PM, Al Viro <viro@zeniv.linux.org.uk> wrote:
> 
> On Tue, Oct 29, 2024 at 04:12:41PM -0700, Song Liu wrote:
>> + if (strstr(file_name->name, item->prefix) == (char *)file_name->name)
> 
> Huh?  "Find the first substring (if any) equal to item->prefix and
> then check if that happens to be in the very beginning"???

Replaced it with strcmp locally, assuming strncmp is not necessary.

> 
> And you are placing that into the place where it's most likely to cause
> the maximal braindamage and spread all over the tree.  Wonderful ;-/
> 
> Where does that "idiom" come from, anyway?  Java?  Not the first time
> I see that kind of garbage; typecast is an unusual twist, though...

The strstr() was probably from lack of coffee. The cast is most likely 
from the compiler yelling at sleepy me. 

BTW: Why do we need "const unsigned char *" in qstr instead of 
"const char *"? I do notice git doesn't show the real history of it..

Thanks,
Song
Song Liu Oct. 30, 2024, 2:35 a.m. UTC | #4 
> On Oct 29, 2024, at 7:07 PM, Song Liu <songliubraving@meta.com> wrote:
> 
> 
> 
>> On Oct 29, 2024, at 5:11 PM, Al Viro <viro@zeniv.linux.org.uk> wrote:
>> 
>> On Tue, Oct 29, 2024 at 04:12:41PM -0700, Song Liu wrote:
>>> + if (strstr(file_name->name, item->prefix) == (char *)file_name->name)
>> 
>> Huh?  "Find the first substring (if any) equal to item->prefix and
>> then check if that happens to be in the very beginning"???
> 
> Replaced it with strcmp locally, assuming strncmp is not necessary.

Oops, we do need strncmp and more coffee..
Jeff Layton Oct. 30, 2024, 1:03 p.m. UTC | #5 
On Tue, 2024-10-29 at 16:12 -0700, Song Liu wrote:
> This fastpath handler filters out events for files with certain prefixes.
> To use it:
> 
>   [root] insmod fastpath-mod.ko    # This requires root.
> 
>   [user] ./fastpath-user /tmp a,b,c &    # Root is not needed
>   [user] touch /tmp/aa   # a is in the prefix list (a,b,c), no events
>   [user] touch /tmp/xx   # x is not in the prefix list, generates events
> 
>   Accessing file xx   # this is the output from fastpath_user
> 
> Signed-off-by: Song Liu <song@kernel.org>
> ---
>  MAINTAINERS                      |   1 +
>  samples/Kconfig                  |  20 ++++-
>  samples/Makefile                 |   2 +-
>  samples/fanotify/.gitignore      |   1 +
>  samples/fanotify/Makefile        |   5 +-
>  samples/fanotify/fastpath-mod.c  | 138 +++++++++++++++++++++++++++++++
>  samples/fanotify/fastpath-user.c |  90 ++++++++++++++++++++
>  7 files changed, 254 insertions(+), 3 deletions(-)
>  create mode 100644 samples/fanotify/fastpath-mod.c
>  create mode 100644 samples/fanotify/fastpath-user.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 7ad507f49324..8939a48b2d99 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -8658,6 +8658,7 @@ S:	Maintained
>  F:	fs/notify/fanotify/
>  F:	include/linux/fanotify.h
>  F:	include/uapi/linux/fanotify.h
> +F:	samples/fanotify/
>  
>  FARADAY FOTG210 USB2 DUAL-ROLE CONTROLLER
>  M:	Linus Walleij <linus.walleij@linaro.org>
> diff --git a/samples/Kconfig b/samples/Kconfig
> index b288d9991d27..b0d3dff48bb0 100644
> --- a/samples/Kconfig
> +++ b/samples/Kconfig
> @@ -149,15 +149,33 @@ config SAMPLE_CONNECTOR
>  	  with it.
>  	  See also Documentation/driver-api/connector.rst
>  
> +config SAMPLE_FANOTIFY
> +	bool "Build fanotify monitoring sample"
> +	depends on FANOTIFY && CC_CAN_LINK && HEADERS_INSTALL
> +	help
> +	  When enabled, this builds samples for fanotify.
> +	  There multiple samples for fanotify. Please see the
> +	  following configs for more details of these
> +	  samples.
> +
>  config SAMPLE_FANOTIFY_ERROR
>  	bool "Build fanotify error monitoring sample"
> -	depends on FANOTIFY && CC_CAN_LINK && HEADERS_INSTALL
> +	depends on SAMPLE_FANOTIFY
>  	help
>  	  When enabled, this builds an example code that uses the
>  	  FAN_FS_ERROR fanotify mechanism to monitor filesystem
>  	  errors.
>  	  See also Documentation/admin-guide/filesystem-monitoring.rst.
>  
> +config SAMPLE_FANOTIFY_FASTPATH
> +	tristate "Build fanotify fastpath sample"
> +	depends on SAMPLE_FANOTIFY && m
> +	help
> +	  When enabled, this builds kernel module that contains a
> +	  fanotify fastpath handler.
> +	  The fastpath handler filters out certain filename
> +	  prefixes for the fanotify user.
> +
>  config SAMPLE_HIDRAW
>  	bool "hidraw sample"
>  	depends on CC_CAN_LINK && HEADERS_INSTALL
> diff --git a/samples/Makefile b/samples/Makefile
> index b85fa64390c5..108360972626 100644
> --- a/samples/Makefile
> +++ b/samples/Makefile
> @@ -6,7 +6,7 @@ subdir-$(CONFIG_SAMPLE_ANDROID_BINDERFS) += binderfs
>  subdir-$(CONFIG_SAMPLE_CGROUP) += cgroup
>  obj-$(CONFIG_SAMPLE_CONFIGFS)		+= configfs/
>  obj-$(CONFIG_SAMPLE_CONNECTOR)		+= connector/
> -obj-$(CONFIG_SAMPLE_FANOTIFY_ERROR)	+= fanotify/
> +obj-$(CONFIG_SAMPLE_FANOTIFY)		+= fanotify/
>  subdir-$(CONFIG_SAMPLE_HIDRAW)		+= hidraw
>  obj-$(CONFIG_SAMPLE_HW_BREAKPOINT)	+= hw_breakpoint/
>  obj-$(CONFIG_SAMPLE_KDB)		+= kdb/
> diff --git a/samples/fanotify/.gitignore b/samples/fanotify/.gitignore
> index d74593e8b2de..306e1ddec4e0 100644
> --- a/samples/fanotify/.gitignore
> +++ b/samples/fanotify/.gitignore
> @@ -1 +1,2 @@
>  fs-monitor
> +fastpath-user
> diff --git a/samples/fanotify/Makefile b/samples/fanotify/Makefile
> index e20db1bdde3b..f5bbd7380104 100644
> --- a/samples/fanotify/Makefile
> +++ b/samples/fanotify/Makefile
> @@ -1,5 +1,8 @@
>  # SPDX-License-Identifier: GPL-2.0-only
> -userprogs-always-y += fs-monitor
> +userprogs-always-$(CONFIG_SAMPLE_FANOTIFY_ERROR) += fs-monitor
>  
>  userccflags += -I usr/include -Wall
>  
> +obj-$(CONFIG_SAMPLE_FANOTIFY_FASTPATH) += fastpath-mod.o
> +
> +userprogs-always-$(CONFIG_SAMPLE_FANOTIFY_FASTPATH) += fastpath-user
> diff --git a/samples/fanotify/fastpath-mod.c b/samples/fanotify/fastpath-mod.c
> new file mode 100644
> index 000000000000..06c4b42ff114
> --- /dev/null
> +++ b/samples/fanotify/fastpath-mod.c
> @@ -0,0 +1,138 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +#include <linux/fsnotify.h>
> +#include <linux/fanotify.h>
> +#include <linux/list.h>
> +#include <linux/module.h>
> +#include <linux/string.h>
> +
> +struct prefix_item {
> +	const char *prefix;
> +	struct list_head list;
> +};
> +
> +struct sample_fp_data {
> +	/*
> +	 * str_table contains all the prefixes to ignore. For example,
> +	 * "prefix1\0prefix2\0prefix3"
> +	 */
> +	char *str_table;
> +
> +	/* item->prefix points to different prefixes in the str_table. */
> +	struct list_head item_list;
> +};
> +
> +static int sample_fp_handler(struct fsnotify_group *group,
> +			     struct fanotify_fastpath_hook *fp_hook,
> +			     struct fanotify_fastpath_event *fp_event)
> +{
> +	const struct qstr *file_name = fp_event->file_name;
> +	struct sample_fp_data *fp_data;
> +	struct prefix_item *item;
> +
> +	if (!file_name)
> +		return FAN_FP_RET_SEND_TO_USERSPACE;
> +	fp_data = fp_hook->data;
> +
> +	list_for_each_entry(item, &fp_data->item_list, list) {
> +		if (strstr(file_name->name, item->prefix) == (char *)file_name->name)
> +			return FAN_FP_RET_SKIP_EVENT;
> +	}
> +
> +	return FAN_FP_RET_SEND_TO_USERSPACE;
> +}

The sample is a little underwhelming and everyone hates string parsing
in the kernel ;). It'd be nice to see a more real-world use-case for
this.

Could this be used to implement subtree filtering? I guess you'd have
to walk back up the directory tree and see whether it had a given
ancestor?

> +
> +static int add_item(struct sample_fp_data *fp_data, const char *prev)
> +{
> +	struct prefix_item *item;
> +
> +	item = kzalloc(sizeof(*item), GFP_KERNEL);
> +	if (!item)
> +		return -ENOMEM;
> +	item->prefix = prev;
> +	list_add_tail(&item->list, &fp_data->item_list);
> +	return 0;
> +}
> +
> +static void free_sample_fp_data(struct sample_fp_data *fp_data)
> +{
> +	struct prefix_item *item, *tmp;
> +
> +	list_for_each_entry_safe(item, tmp, &fp_data->item_list, list) {
> +		list_del_init(&item->list);
> +		kfree(item);
> +	}
> +	kfree(fp_data->str_table);
> +	kfree(fp_data);
> +}
> +
> +static int sample_fp_init(struct fanotify_fastpath_hook *fp_hook, const char *args)
> +{
> +	struct sample_fp_data *fp_data = kzalloc(sizeof(struct sample_fp_data), GFP_KERNEL);
> +	char *p, *prev;
> +	int ret;
> +
> +	if (!fp_data)
> +		return -ENOMEM;
> +
> +	/* Make a copy of the list of prefix to ignore */
> +	fp_data->str_table = kstrndup(args, FAN_FP_ARGS_MAX, GFP_KERNEL);
> +	if (!fp_data->str_table) {
> +		ret = -ENOMEM;
> +		goto err_out;
> +	}
> +
> +	INIT_LIST_HEAD(&fp_data->item_list);
> +	prev = fp_data->str_table;
> +	p = fp_data->str_table;
> +
> +	/* Update the list replace ',' with '\n'*/
> +	while ((p = strchr(p, ',')) != NULL) {
> +		*p = '\0';
> +		ret = add_item(fp_data, prev);
> +		if (ret)
> +			goto err_out;
> +		p = p + 1;
> +		prev = p;
> +	}
> +
> +	ret = add_item(fp_data, prev);
> +	if (ret)
> +		goto err_out;
> +
> +	fp_hook->data = fp_data;
> +
> +	return 0;
> +
> +err_out:
> +	free_sample_fp_data(fp_data);
> +	return ret;
> +}
> +
> +static void sample_fp_free(struct fanotify_fastpath_hook *fp_hook)
> +{
> +	free_sample_fp_data(fp_hook->data);
> +}
> +
> +static struct fanotify_fastpath_ops fan_fp_ignore_a_ops = {
> +	.fp_handler = sample_fp_handler,
> +	.fp_init = sample_fp_init,
> +	.fp_free = sample_fp_free,
> +	.name = "ignore-prefix",
> +	.owner = THIS_MODULE,
> +};
> +
> +static int __init fanotify_fastpath_sample_init(void)
> +{
> +	return fanotify_fastpath_register(&fan_fp_ignore_a_ops);
> +}
> +static void __exit fanotify_fastpath_sample_exit(void)
> +{
> +	fanotify_fastpath_unregister(&fan_fp_ignore_a_ops);
> +}
> +
> +module_init(fanotify_fastpath_sample_init);
> +module_exit(fanotify_fastpath_sample_exit);
> +
> +MODULE_AUTHOR("Song Liu");
> +MODULE_DESCRIPTION("Example fanotify fastpath handler");
> +MODULE_LICENSE("GPL");
> diff --git a/samples/fanotify/fastpath-user.c b/samples/fanotify/fastpath-user.c
> new file mode 100644
> index 000000000000..f301c4e0d21a
> --- /dev/null
> +++ b/samples/fanotify/fastpath-user.c
> @@ -0,0 +1,90 @@
> +// SPDX-License-Identifier: GPL-2.0
> +#define _GNU_SOURCE
> +#include <err.h>
> +#include <fcntl.h>
> +#include <stdio.h>
> +#include <string.h>
> +#include <sys/fanotify.h>
> +#include <unistd.h>
> +#include <sys/ioctl.h>
> +
> +static int total_event_cnt;
> +
> +static void handle_notifications(char *buffer, int len)
> +{
> +	struct fanotify_event_metadata *event =
> +		(struct fanotify_event_metadata *) buffer;
> +	struct fanotify_event_info_header *info;
> +	struct fanotify_event_info_fid *fid;
> +	struct file_handle *handle;
> +	char *name;
> +	int off;
> +
> +	for (; FAN_EVENT_OK(event, len); event = FAN_EVENT_NEXT(event, len)) {
> +		for (off = sizeof(*event) ; off < event->event_len;
> +		     off += info->len) {
> +			info = (struct fanotify_event_info_header *)
> +				((char *) event + off);
> +			switch (info->info_type) {
> +			case FAN_EVENT_INFO_TYPE_DFID_NAME:
> +				fid = (struct fanotify_event_info_fid *) info;
> +				handle = (struct file_handle *)&fid->handle;
> +				name = (char *)handle + sizeof(*handle) + handle->handle_bytes;
> +
> +				printf("Accessing file %s\n", name);
> +				total_event_cnt++;
> +				break;
> +			default:
> +				break;
> +			}
> +		}
> +	}
> +}
> +
> +int main(int argc, char **argv)
> +{
> +	struct fanotify_fastpath_args args = {
> +		.name = "ignore-prefix",
> +		.version = 1,
> +		.flags = 0,
> +	};
> +	char buffer[BUFSIZ];
> +	int fd;
> +
> +	if (argc < 3) {
> +		printf("Usage\n"
> +		       "\t %s <path to monitor> <prefix to ignore>\n",
> +			argv[0]);
> +		return 1;
> +	}
> +
> +	args.init_args = (__u64)argv[2];
> +	args.init_args_len = strlen(argv[2]) + 1;
> +
> +	fd = fanotify_init(FAN_CLASS_NOTIF | FAN_REPORT_NAME | FAN_REPORT_DIR_FID, O_RDONLY);
> +	if (fd < 0)
> +		errx(1, "fanotify_init");
> +
> +	if (fanotify_mark(fd, FAN_MARK_ADD,
> +			  FAN_OPEN | FAN_ONDIR | FAN_EVENT_ON_CHILD,
> +			  AT_FDCWD, argv[1])) {
> +		errx(1, "fanotify_mark");
> +	}
> +
> +	if (ioctl(fd, FAN_IOC_ADD_FP, &args))
> +		errx(1, "ioctl");
> +
> +	while (total_event_cnt < 10) {
> +		int n = read(fd, buffer, BUFSIZ);
> +
> +		if (n < 0)
> +			errx(1, "read");
> +
> +		handle_notifications(buffer, n);
> +	}
> +
> +	ioctl(fd, FAN_IOC_DEL_FP);
> +	close(fd);
> +
> +	return 0;
> +}
Song Liu Oct. 30, 2024, 8:30 p.m. UTC | #6 
Hi Jeff, 

Thanks for your review!

I will update 1/2 based on the feedback. (Replying here to save everyone 
an email..)

> On Oct 30, 2024, at 6:03 AM, Jeff Layton <jlayton@kernel.org> wrote:

[...]

>> +
>> +static int sample_fp_handler(struct fsnotify_group *group,
>> +     struct fanotify_fastpath_hook *fp_hook,
>> +     struct fanotify_fastpath_event *fp_event)
>> +{
>> + const struct qstr *file_name = fp_event->file_name;
>> + struct sample_fp_data *fp_data;
>> + struct prefix_item *item;
>> +
>> + if (!file_name)
>> + return FAN_FP_RET_SEND_TO_USERSPACE;
>> + fp_data = fp_hook->data;
>> +
>> + list_for_each_entry(item, &fp_data->item_list, list) {
>> + if (strstr(file_name->name, item->prefix) == (char *)file_name->name)
>> + return FAN_FP_RET_SKIP_EVENT;
>> + }
>> +
>> + return FAN_FP_RET_SEND_TO_USERSPACE;
>> +}
> 
> The sample is a little underwhelming and everyone hates string parsing
> in the kernel ;). It'd be nice to see a more real-world use-case for
> this.
> 
> Could this be used to implement subtree filtering? I guess you'd have
> to walk back up the directory tree and see whether it had a given
> ancestor?

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. 

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

How should we pass in the subtree? I guess we can just use full path in
a string as the argument.

Thanks,
Song
Jeff Layton Oct. 31, 2024, 12:23 a.m. UTC | #7 
On Wed, 2024-10-30 at 20:30 +0000, Song Liu wrote:
> Hi Jeff, 
> 
> Thanks for your review!
> 
> I will update 1/2 based on the feedback. (Replying here to save everyone 
> an email..)
> 
> > On Oct 30, 2024, at 6:03 AM, Jeff Layton <jlayton@kernel.org> wrote:
> 
> [...]
> 
> > > +
> > > +static int sample_fp_handler(struct fsnotify_group *group,
> > > +     struct fanotify_fastpath_hook *fp_hook,
> > > +     struct fanotify_fastpath_event *fp_event)
> > > +{
> > > + const struct qstr *file_name = fp_event->file_name;
> > > + struct sample_fp_data *fp_data;
> > > + struct prefix_item *item;
> > > +
> > > + if (!file_name)
> > > + return FAN_FP_RET_SEND_TO_USERSPACE;
> > > + fp_data = fp_hook->data;
> > > +
> > > + list_for_each_entry(item, &fp_data->item_list, list) {
> > > + if (strstr(file_name->name, item->prefix) == (char *)file_name->name)
> > > + return FAN_FP_RET_SKIP_EVENT;
> > > + }
> > > +
> > > + return FAN_FP_RET_SEND_TO_USERSPACE;
> > > +}
> > 
> > The sample is a little underwhelming and everyone hates string parsing
> > in the kernel ;). It'd be nice to see a more real-world use-case for
> > this.
> > 
> > Could this be used to implement subtree filtering? I guess you'd have
> > to walk back up the directory tree and see whether it had a given
> > ancestor?
> 
> 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.

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

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).
Song Liu Oct. 31, 2024, 1:52 a.m. UTC | #8 
Hi Jeff, 

> On Oct 30, 2024, at 5:23 PM, Jeff Layton <jlayton@kernel.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. 

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).
> -- 
> Jeff Layton <jlayton@kernel.org>
diff mbox series

Patch

diff --git a/MAINTAINERS b/MAINTAINERS
index 7ad507f49324..8939a48b2d99 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8658,6 +8658,7 @@  S:	Maintained
 F:	fs/notify/fanotify/
 F:	include/linux/fanotify.h
 F:	include/uapi/linux/fanotify.h
+F:	samples/fanotify/
 
 FARADAY FOTG210 USB2 DUAL-ROLE CONTROLLER
 M:	Linus Walleij <linus.walleij@linaro.org>
diff --git a/samples/Kconfig b/samples/Kconfig
index b288d9991d27..b0d3dff48bb0 100644
--- a/samples/Kconfig
+++ b/samples/Kconfig
@@ -149,15 +149,33 @@  config SAMPLE_CONNECTOR
 	  with it.
 	  See also Documentation/driver-api/connector.rst
 
+config SAMPLE_FANOTIFY
+	bool "Build fanotify monitoring sample"
+	depends on FANOTIFY && CC_CAN_LINK && HEADERS_INSTALL
+	help
+	  When enabled, this builds samples for fanotify.
+	  There multiple samples for fanotify. Please see the
+	  following configs for more details of these
+	  samples.
+
 config SAMPLE_FANOTIFY_ERROR
 	bool "Build fanotify error monitoring sample"
-	depends on FANOTIFY && CC_CAN_LINK && HEADERS_INSTALL
+	depends on SAMPLE_FANOTIFY
 	help
 	  When enabled, this builds an example code that uses the
 	  FAN_FS_ERROR fanotify mechanism to monitor filesystem
 	  errors.
 	  See also Documentation/admin-guide/filesystem-monitoring.rst.
 
+config SAMPLE_FANOTIFY_FASTPATH
+	tristate "Build fanotify fastpath sample"
+	depends on SAMPLE_FANOTIFY && m
+	help
+	  When enabled, this builds kernel module that contains a
+	  fanotify fastpath handler.
+	  The fastpath handler filters out certain filename
+	  prefixes for the fanotify user.
+
 config SAMPLE_HIDRAW
 	bool "hidraw sample"
 	depends on CC_CAN_LINK && HEADERS_INSTALL
diff --git a/samples/Makefile b/samples/Makefile
index b85fa64390c5..108360972626 100644
--- a/samples/Makefile
+++ b/samples/Makefile
@@ -6,7 +6,7 @@  subdir-$(CONFIG_SAMPLE_ANDROID_BINDERFS) += binderfs
 subdir-$(CONFIG_SAMPLE_CGROUP) += cgroup
 obj-$(CONFIG_SAMPLE_CONFIGFS)		+= configfs/
 obj-$(CONFIG_SAMPLE_CONNECTOR)		+= connector/
-obj-$(CONFIG_SAMPLE_FANOTIFY_ERROR)	+= fanotify/
+obj-$(CONFIG_SAMPLE_FANOTIFY)		+= fanotify/
 subdir-$(CONFIG_SAMPLE_HIDRAW)		+= hidraw
 obj-$(CONFIG_SAMPLE_HW_BREAKPOINT)	+= hw_breakpoint/
 obj-$(CONFIG_SAMPLE_KDB)		+= kdb/
diff --git a/samples/fanotify/.gitignore b/samples/fanotify/.gitignore
index d74593e8b2de..306e1ddec4e0 100644
--- a/samples/fanotify/.gitignore
+++ b/samples/fanotify/.gitignore
@@ -1 +1,2 @@ 
 fs-monitor
+fastpath-user
diff --git a/samples/fanotify/Makefile b/samples/fanotify/Makefile
index e20db1bdde3b..f5bbd7380104 100644
--- a/samples/fanotify/Makefile
+++ b/samples/fanotify/Makefile
@@ -1,5 +1,8 @@ 
 # SPDX-License-Identifier: GPL-2.0-only
-userprogs-always-y += fs-monitor
+userprogs-always-$(CONFIG_SAMPLE_FANOTIFY_ERROR) += fs-monitor
 
 userccflags += -I usr/include -Wall
 
+obj-$(CONFIG_SAMPLE_FANOTIFY_FASTPATH) += fastpath-mod.o
+
+userprogs-always-$(CONFIG_SAMPLE_FANOTIFY_FASTPATH) += fastpath-user
diff --git a/samples/fanotify/fastpath-mod.c b/samples/fanotify/fastpath-mod.c
new file mode 100644
index 000000000000..06c4b42ff114
--- /dev/null
+++ b/samples/fanotify/fastpath-mod.c
@@ -0,0 +1,138 @@ 
+// SPDX-License-Identifier: GPL-2.0-only
+#include <linux/fsnotify.h>
+#include <linux/fanotify.h>
+#include <linux/list.h>
+#include <linux/module.h>
+#include <linux/string.h>
+
+struct prefix_item {
+	const char *prefix;
+	struct list_head list;
+};
+
+struct sample_fp_data {
+	/*
+	 * str_table contains all the prefixes to ignore. For example,
+	 * "prefix1\0prefix2\0prefix3"
+	 */
+	char *str_table;
+
+	/* item->prefix points to different prefixes in the str_table. */
+	struct list_head item_list;
+};
+
+static int sample_fp_handler(struct fsnotify_group *group,
+			     struct fanotify_fastpath_hook *fp_hook,
+			     struct fanotify_fastpath_event *fp_event)
+{
+	const struct qstr *file_name = fp_event->file_name;
+	struct sample_fp_data *fp_data;
+	struct prefix_item *item;
+
+	if (!file_name)
+		return FAN_FP_RET_SEND_TO_USERSPACE;
+	fp_data = fp_hook->data;
+
+	list_for_each_entry(item, &fp_data->item_list, list) {
+		if (strstr(file_name->name, item->prefix) == (char *)file_name->name)
+			return FAN_FP_RET_SKIP_EVENT;
+	}
+
+	return FAN_FP_RET_SEND_TO_USERSPACE;
+}
+
+static int add_item(struct sample_fp_data *fp_data, const char *prev)
+{
+	struct prefix_item *item;
+
+	item = kzalloc(sizeof(*item), GFP_KERNEL);
+	if (!item)
+		return -ENOMEM;
+	item->prefix = prev;
+	list_add_tail(&item->list, &fp_data->item_list);
+	return 0;
+}
+
+static void free_sample_fp_data(struct sample_fp_data *fp_data)
+{
+	struct prefix_item *item, *tmp;
+
+	list_for_each_entry_safe(item, tmp, &fp_data->item_list, list) {
+		list_del_init(&item->list);
+		kfree(item);
+	}
+	kfree(fp_data->str_table);
+	kfree(fp_data);
+}
+
+static int sample_fp_init(struct fanotify_fastpath_hook *fp_hook, const char *args)
+{
+	struct sample_fp_data *fp_data = kzalloc(sizeof(struct sample_fp_data), GFP_KERNEL);
+	char *p, *prev;
+	int ret;
+
+	if (!fp_data)
+		return -ENOMEM;
+
+	/* Make a copy of the list of prefix to ignore */
+	fp_data->str_table = kstrndup(args, FAN_FP_ARGS_MAX, GFP_KERNEL);
+	if (!fp_data->str_table) {
+		ret = -ENOMEM;
+		goto err_out;
+	}
+
+	INIT_LIST_HEAD(&fp_data->item_list);
+	prev = fp_data->str_table;
+	p = fp_data->str_table;
+
+	/* Update the list replace ',' with '\n'*/
+	while ((p = strchr(p, ',')) != NULL) {
+		*p = '\0';
+		ret = add_item(fp_data, prev);
+		if (ret)
+			goto err_out;
+		p = p + 1;
+		prev = p;
+	}
+
+	ret = add_item(fp_data, prev);
+	if (ret)
+		goto err_out;
+
+	fp_hook->data = fp_data;
+
+	return 0;
+
+err_out:
+	free_sample_fp_data(fp_data);
+	return ret;
+}
+
+static void sample_fp_free(struct fanotify_fastpath_hook *fp_hook)
+{
+	free_sample_fp_data(fp_hook->data);
+}
+
+static struct fanotify_fastpath_ops fan_fp_ignore_a_ops = {
+	.fp_handler = sample_fp_handler,
+	.fp_init = sample_fp_init,
+	.fp_free = sample_fp_free,
+	.name = "ignore-prefix",
+	.owner = THIS_MODULE,
+};
+
+static int __init fanotify_fastpath_sample_init(void)
+{
+	return fanotify_fastpath_register(&fan_fp_ignore_a_ops);
+}
+static void __exit fanotify_fastpath_sample_exit(void)
+{
+	fanotify_fastpath_unregister(&fan_fp_ignore_a_ops);
+}
+
+module_init(fanotify_fastpath_sample_init);
+module_exit(fanotify_fastpath_sample_exit);
+
+MODULE_AUTHOR("Song Liu");
+MODULE_DESCRIPTION("Example fanotify fastpath handler");
+MODULE_LICENSE("GPL");
diff --git a/samples/fanotify/fastpath-user.c b/samples/fanotify/fastpath-user.c
new file mode 100644
index 000000000000..f301c4e0d21a
--- /dev/null
+++ b/samples/fanotify/fastpath-user.c
@@ -0,0 +1,90 @@ 
+// SPDX-License-Identifier: GPL-2.0
+#define _GNU_SOURCE
+#include <err.h>
+#include <fcntl.h>
+#include <stdio.h>
+#include <string.h>
+#include <sys/fanotify.h>
+#include <unistd.h>
+#include <sys/ioctl.h>
+
+static int total_event_cnt;
+
+static void handle_notifications(char *buffer, int len)
+{
+	struct fanotify_event_metadata *event =
+		(struct fanotify_event_metadata *) buffer;
+	struct fanotify_event_info_header *info;
+	struct fanotify_event_info_fid *fid;
+	struct file_handle *handle;
+	char *name;
+	int off;
+
+	for (; FAN_EVENT_OK(event, len); event = FAN_EVENT_NEXT(event, len)) {
+		for (off = sizeof(*event) ; off < event->event_len;
+		     off += info->len) {
+			info = (struct fanotify_event_info_header *)
+				((char *) event + off);
+			switch (info->info_type) {
+			case FAN_EVENT_INFO_TYPE_DFID_NAME:
+				fid = (struct fanotify_event_info_fid *) info;
+				handle = (struct file_handle *)&fid->handle;
+				name = (char *)handle + sizeof(*handle) + handle->handle_bytes;
+
+				printf("Accessing file %s\n", name);
+				total_event_cnt++;
+				break;
+			default:
+				break;
+			}
+		}
+	}
+}
+
+int main(int argc, char **argv)
+{
+	struct fanotify_fastpath_args args = {
+		.name = "ignore-prefix",
+		.version = 1,
+		.flags = 0,
+	};
+	char buffer[BUFSIZ];
+	int fd;
+
+	if (argc < 3) {
+		printf("Usage\n"
+		       "\t %s <path to monitor> <prefix to ignore>\n",
+			argv[0]);
+		return 1;
+	}
+
+	args.init_args = (__u64)argv[2];
+	args.init_args_len = strlen(argv[2]) + 1;
+
+	fd = fanotify_init(FAN_CLASS_NOTIF | FAN_REPORT_NAME | FAN_REPORT_DIR_FID, O_RDONLY);
+	if (fd < 0)
+		errx(1, "fanotify_init");
+
+	if (fanotify_mark(fd, FAN_MARK_ADD,
+			  FAN_OPEN | FAN_ONDIR | FAN_EVENT_ON_CHILD,
+			  AT_FDCWD, argv[1])) {
+		errx(1, "fanotify_mark");
+	}
+
+	if (ioctl(fd, FAN_IOC_ADD_FP, &args))
+		errx(1, "ioctl");
+
+	while (total_event_cnt < 10) {
+		int n = read(fd, buffer, BUFSIZ);
+
+		if (n < 0)
+			errx(1, "read");
+
+		handle_notifications(buffer, n);
+	}
+
+	ioctl(fd, FAN_IOC_DEL_FP);
+	close(fd);
+
+	return 0;
+}