On Sat, Nov 23, 2024 at 9:07 PM Amir Goldstein [off-list ref] wrote:

quoted

+++ b/samples/fanotify/filter-mod.c

@@ -0,0 +1,105 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/* Copyright (c) 2024 Meta Platforms, Inc. and affiliates. */
+
+#include <linux/fsnotify.h>
+#include <linux/fanotify.h>
+#include <linux/module.h>
+#include <linux/path.h>
+#include <linux/file.h>
+#include "filter.h"
+
+struct fan_filter_sample_data {
+       struct path subtree_path;
+       enum fan_filter_sample_mode mode;
+};
+
+static int sample_filter(struct fsnotify_group *group,
+                        struct fanotify_filter_hook *filter_hook,
+                        struct fanotify_filter_event *filter_event)
+{
+       struct fan_filter_sample_data *data;
+       struct dentry *dentry;
+
+       dentry = fsnotify_data_dentry(filter_event->data, filter_event->data_type);
+       if (!dentry)
+               return FAN_FILTER_RET_SEND_TO_USERSPACE;
+
+       data = filter_hook->data;
+
+       if (is_subdir(dentry, data->subtree_path.dentry)) {
+               if (data->mode == FAN_FILTER_SAMPLE_MODE_BLOCK)
+                       return -EPERM;
+               return FAN_FILTER_RET_SEND_TO_USERSPACE;
+       }
+       return FAN_FILTER_RET_SKIP_EVENT;
+}
+
+static int sample_filter_init(struct fsnotify_group *group,
+                             struct fanotify_filter_hook *filter_hook,
+                             void *argp)
+{
+       struct fan_filter_sample_args *args;
+       struct fan_filter_sample_data *data;
+       struct file *file;
+       int fd;
+
+       args = (struct fan_filter_sample_args *)argp;
+       fd = args->subtree_fd;
+
+       file = fget(fd);
+       if (!file)
+               return -EBADF;
+       data = kzalloc(sizeof(struct fan_filter_sample_data), GFP_KERNEL);
+       if (!data) {
+               fput(file);
+               return -ENOMEM;
+       }
+       path_get(&file->f_path);
+       data->subtree_path = file->f_path;
+       fput(file);
+       data->mode = args->mode;
+       filter_hook->data = data;
+       return 0;
+}
+
+static void sample_filter_free(struct fanotify_filter_hook *filter_hook)
+{
+       struct fan_filter_sample_data *data = filter_hook->data;
+
+       path_put(&data->subtree_path);
+       kfree(data);
+}
+

Hi Song,

This example looks fine but it raises a question.
This filter will keep the mount of subtree_path busy until the group is closed
or the filter is detached.
This is probably fine for many services that keep the mount busy anyway.

But what if this wasn't the intention?
What if an Anti-malware engine that watches all mounts wanted to use that
for configuring some ignore/block subtree filters?

One way would be to use a is_subtree() variant that looks for a
subtree root inode
number and then verifies it with a subtree root fid.
A production subtree filter will need to use a variant of is_subtree()
anyway that
looks for a set of subtree root inodes, because doing a loop of is_subtree() for
multiple paths is a no go.

Don't need to change anything in the example, unless other people
think that we do need to set a better example to begin with...

I think we have to treat this patch as a real filter and not as an example
to make sure that the whole approach is workable end to end.
The point about not holding path/dentry is very valid.
The algorithm needs to support that.
It may very well turn out that the logic of handling many filters
without a loop and not grabbing a path refcnt is too complex for bpf.
Then this subtree filtering would have to stay as a kernel module
or extra flag/feature for fanotify.