| 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
| ||
|
Message-ID: <CAOQ4uxhfd8ryQ6ua5u60yN5sh06fyiieS3XgfR9jvkAOeDSZUg@mail.gmail.com>
Date: Sun, 24 Nov 2024 06:07:00 +0100
From: Amir Goldstein <amir73il@...il.com>
To: Song Liu <song@...nel.org>
Cc: bpf@...r.kernel.org, linux-fsdevel@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-security-module@...r.kernel.org,
kernel-team@...a.com, andrii@...nel.org, eddyz87@...il.com, ast@...nel.org,
daniel@...earbox.net, martin.lau@...ux.dev, viro@...iv.linux.org.uk,
brauner@...nel.org, jack@...e.cz, kpsingh@...nel.org,
mattbobrowski@...gle.com, repnop@...gle.com, jlayton@...nel.org,
josef@...icpanda.com, mic@...ikod.net, gnoack@...gle.com
Subject: Re: [PATCH v3 fanotify 2/2] samples/fanotify: Add a sample fanotify fiter
On Sat, Nov 23, 2024 at 12:00 AM Song Liu <song@...nel.org> wrote:
>
> This sample can run in two different mode: filter mode and block mode.
> In filter mode, the filter only sends events in a subtree to user space.
> To use it:
>
> [root] insmod ./filter-mod.ko
> [root] mkdir -p /tmp/a/b/c/d
> [root] ./filter-user /tmp/ /tmp/a/b &
> Running in filter mode
> [root] touch /tmp/xx # Doesn't generate event
> [root]# touch /tmp/a/xxa # Doesn't generate event
> [root]# touch /tmp/a/b/xxab # Generates an event
> Accessing file xxab # this is the output from filter-user
> [root@]# touch /tmp/a/b/c/xxabc # Generates an event
> Accessing file xxabc # this is the output from filter-user
>
> In block mode, the filter will block accesses to file in a subtree. To
> use it:
>
> [root] insmod ./filter-mod.ko
> [root] mkdir -p /tmp/a/b/c/d
> [root] ./filter-user /tmp/ /tmp/a/b block &
> Running in block mode
> [root]# dd if=/dev/zero of=/tmp/a/b/xx # this will fail
> dd: failed to open '/tmp/a/b/xx': Operation not permitted
>
> Signed-off-by: Song Liu <song@...nel.org>
> ---
> MAINTAINERS | 1 +
> samples/Kconfig | 20 ++++-
> samples/Makefile | 2 +-
> samples/fanotify/.gitignore | 1 +
> samples/fanotify/Makefile | 5 +-
> samples/fanotify/filter-mod.c | 105 ++++++++++++++++++++++++++
> samples/fanotify/filter-user.c | 131 +++++++++++++++++++++++++++++++++
> samples/fanotify/filter.h | 19 +++++
> 8 files changed, 281 insertions(+), 3 deletions(-)
> create mode 100644 samples/fanotify/filter-mod.c
> create mode 100644 samples/fanotify/filter-user.c
> create mode 100644 samples/fanotify/filter.h
>
> 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@...aro.org>
> diff --git a/samples/Kconfig b/samples/Kconfig
> index b288d9991d27..9cc0a5cdf604 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_FILTER
> + tristate "Build fanotify filter sample"
> + depends on SAMPLE_FANOTIFY && m
> + help
> + When enabled, this builds kernel module that contains a
> + fanotify filter handler.
> + The filter 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..df75eb5b8f95 100644
> --- a/samples/fanotify/.gitignore
> +++ b/samples/fanotify/.gitignore
> @@ -1 +1,2 @@
> fs-monitor
> +filter-user
> diff --git a/samples/fanotify/Makefile b/samples/fanotify/Makefile
> index e20db1bdde3b..c33e9460772e 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_FILTER) += filter-mod.o
> +
> +userprogs-always-$(CONFIG_SAMPLE_FANOTIFY_FILTER) += filter-user
> diff --git a/samples/fanotify/filter-mod.c b/samples/fanotify/filter-mod.c
> new file mode 100644
> index 000000000000..eafe55b1840a
> --- /dev/null
> +++ 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...
Thanks,
Amir.
Powered by blists - more mailing lists