| 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: <CAJnrk1ZUbuAER90xbagWnBZ9dWKkdUAqVRa1vmZ5BtL_o=TnnA@mail.gmail.com>
Date: Wed, 21 Jan 2026 15:42:04 -0800
From: Joanne Koong <joannelkoong@...il.com>
To: "Darrick J. Wong" <djwong@...nel.org>
Cc: miklos@...redi.hu, bernd@...ernd.com, neal@...pa.dev,
linux-ext4@...r.kernel.org, linux-fsdevel@...r.kernel.org
Subject: Re: [PATCH 03/31] fuse: make debugging configurable at runtime
On Tue, Oct 28, 2025 at 5:45 PM Darrick J. Wong <djwong@...nel.org> wrote:
>
> From: Darrick J. Wong <djwong@...nel.org>
>
> Use static keys so that we can configure debugging assertions and dmesg
> warnings at runtime. By default this is turned off so the cost is
> merely scanning a nop sled. However, fuse server developers can turn
> it on for their debugging systems.
>
> Signed-off-by: "Darrick J. Wong" <djwong@...nel.org>
> ---
> fs/fuse/fuse_i.h | 8 +++++
> fs/fuse/iomap_i.h | 16 ++++++++--
> fs/fuse/Kconfig | 15 +++++++++
> fs/fuse/file_iomap.c | 81 ++++++++++++++++++++++++++++++++++++++++++++++++++
> fs/fuse/inode.c | 7 ++++
> 5 files changed, 124 insertions(+), 3 deletions(-)
>
>
> diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
> index 45be59df7ae592..61fb65f3604d61 100644
> --- a/fs/fuse/fuse_i.h
> +++ b/fs/fuse/fuse_i.h
> @@ -1691,6 +1691,14 @@ extern void fuse_sysctl_unregister(void);
> #define fuse_sysctl_unregister() do { } while (0)
> #endif /* CONFIG_SYSCTL */
>
> +#if IS_ENABLED(CONFIG_FUSE_IOMAP_DEBUG)
> +int fuse_iomap_sysfs_init(struct kobject *kobj);
> +void fuse_iomap_sysfs_cleanup(struct kobject *kobj);
> +#else
> +# define fuse_iomap_sysfs_init(...) (0)
> +# define fuse_iomap_sysfs_cleanup(...) ((void)0)
> +#endif
> +
> #if IS_ENABLED(CONFIG_FUSE_IOMAP)
> bool fuse_iomap_enabled(void);
>
> diff --git a/fs/fuse/iomap_i.h b/fs/fuse/iomap_i.h
> index 6d9ce9c0f40a04..3615ec76c0dec0 100644
> --- a/fs/fuse/iomap_i.h
> +++ b/fs/fuse/iomap_i.h
> @@ -6,19 +6,29 @@
> #ifndef _FS_FUSE_IOMAP_I_H
> #define _FS_FUSE_IOMAP_I_H
>
> +#if IS_ENABLED(CONFIG_FUSE_IOMAP_DEBUG_DEFAULT)
> +DECLARE_STATIC_KEY_TRUE(fuse_iomap_debug);
> +#else
> +DECLARE_STATIC_KEY_FALSE(fuse_iomap_debug);
> +#endif
> +
> #if IS_ENABLED(CONFIG_FUSE_IOMAP)
> #if IS_ENABLED(CONFIG_FUSE_IOMAP_DEBUG)
> -# define ASSERT(condition) do { \
> +# define ASSERT(condition) \
> +while (static_branch_unlikely(&fuse_iomap_debug)) { \
> int __cond = !!(condition); \
> if (unlikely(!__cond)) \
> trace_fuse_iomap_assert(__func__, __LINE__, #condition); \
> WARN(!__cond, "Assertion failed: %s, func: %s, line: %d", #condition, __func__, __LINE__); \
> -} while (0)
> + break; \
> +}
> # define BAD_DATA(condition) ({ \
> int __cond = !!(condition); \
> if (unlikely(__cond)) \
> trace_fuse_iomap_bad_data(__func__, __LINE__, #condition); \
> - WARN(__cond, "Bad mapping: %s, func: %s, line: %d", #condition, __func__, __LINE__); \
> + if (static_branch_unlikely(&fuse_iomap_debug)) \
> + WARN(__cond, "Bad mapping: %s, func: %s, line: %d", #condition, __func__, __LINE__); \
> + unlikely(__cond); \
> })
> #else
> # define ASSERT(condition)
> diff --git a/fs/fuse/Kconfig b/fs/fuse/Kconfig
> index 934d48076a010c..bb867afe6e867c 100644
> --- a/fs/fuse/Kconfig
> +++ b/fs/fuse/Kconfig
> @@ -101,6 +101,21 @@ config FUSE_IOMAP_DEBUG
> Enable debugging assertions for the fuse iomap code paths and logging
> of bad iomap file mapping data being sent to the kernel.
>
> + Say N here if you don't want any debugging code code compiled in at
> + all.
> +
> +config FUSE_IOMAP_DEBUG_BY_DEFAULT
> + bool "Debug FUSE file IO over iomap at boot time"
> + default n
> + depends on FUSE_IOMAP_DEBUG
> + help
> + At boot time, enable debugging assertions for the fuse iomap code
> + paths and warnings about bad iomap file mapping data. This enables
> + fuse server authors to control debugging at runtime even on a
> + distribution kernel while avoiding most of the overhead on production
> + systems. The setting can be changed at runtime via
> + /sys/fs/fuse/iomap/debug.
> +
> config FUSE_IO_URING
> bool "FUSE communication over io-uring"
> default y
> diff --git a/fs/fuse/file_iomap.c b/fs/fuse/file_iomap.c
> index a88f5d8d2bce15..b6fc70068c5542 100644
> --- a/fs/fuse/file_iomap.c
> +++ b/fs/fuse/file_iomap.c
> @@ -8,6 +8,12 @@
> #include "fuse_trace.h"
> #include "iomap_i.h"
>
> +#if IS_ENABLED(CONFIG_FUSE_IOMAP_DEBUG_DEFAULT)
> +DEFINE_STATIC_KEY_TRUE(fuse_iomap_debug);
> +#else
> +DEFINE_STATIC_KEY_FALSE(fuse_iomap_debug);
> +#endif
> +
> static bool __read_mostly enable_iomap =
> #if IS_ENABLED(CONFIG_FUSE_IOMAP_BY_DEFAULT)
> true;
> @@ -17,6 +23,81 @@ static bool __read_mostly enable_iomap =
> module_param(enable_iomap, bool, 0644);
> MODULE_PARM_DESC(enable_iomap, "Enable file I/O through iomap");
>
> +#if IS_ENABLED(CONFIG_FUSE_IOMAP_DEBUG)
> +static struct kobject *iomap_kobj;
> +
> +static ssize_t fuse_iomap_debug_show(struct kobject *kobject,
> + struct kobj_attribute *a, char *buf)
> +{
> + return sysfs_emit(buf, "%d\n", !!static_key_enabled(&fuse_iomap_debug));
> +}
> +
> +static ssize_t fuse_iomap_debug_store(struct kobject *kobject,
> + struct kobj_attribute *a,
> + const char *buf, size_t count)
> +{
> + int ret;
> + int val;
> +
> + ret = kstrtoint(buf, 0, &val);
> + if (ret)
> + return ret;
> +
> + if (val < 0 || val > 1)
> + return -EINVAL;
> +
> + if (val)
> + static_branch_enable(&fuse_iomap_debug);
> + else
> + static_branch_disable(&fuse_iomap_debug);
> +
> + return count;
> +}
> +
> +#define __INIT_KOBJ_ATTR(_name, _mode, _show, _store) \
> +{ \
> + .attr = { .name = __stringify(_name), .mode = _mode }, \
> + .show = _show, \
> + .store = _store, \
> +}
> +
> +#define FUSE_ATTR_RW(_name, _show, _store) \
> + static struct kobj_attribute fuse_attr_##_name = \
> + __INIT_KOBJ_ATTR(_name, 0644, _show, _store)
> +
> +#define FUSE_ATTR_PTR(_name) \
> + (&fuse_attr_##_name.attr)
> +
> +FUSE_ATTR_RW(debug, fuse_iomap_debug_show, fuse_iomap_debug_store);
> +
> +static const struct attribute *fuse_iomap_attrs[] = {
> + FUSE_ATTR_PTR(debug),
> + NULL,
> +};
> +
> +int fuse_iomap_sysfs_init(struct kobject *fuse_kobj)
> +{
> + int error;
> +
> + iomap_kobj = kobject_create_and_add("iomap", fuse_kobj);
> + if (!iomap_kobj)
> + return -ENOMEM;
> +
> + error = sysfs_create_files(iomap_kobj, fuse_iomap_attrs);
> + if (error) {
> + kobject_put(iomap_kobj);
> + return error;
> + }
> +
> + return 0;
> +}
> +
> +void fuse_iomap_sysfs_cleanup(struct kobject *fuse_kobj)
> +{
Is sysfs_remove_files() also needed here?
> + kobject_put(iomap_kobj);
> +}
> +#endif /* IS_ENABLED(CONFIG_FUSE_IOMAP_DEBUG) */
> +
> bool fuse_iomap_enabled(void)
> {
> /* Don't let anyone touch iomap until the end of the patchset. */
> diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
> index 1eea8dc6e723c6..eec711302a4a13 100644
> --- a/fs/fuse/inode.c
> +++ b/fs/fuse/inode.c
> @@ -2277,8 +2277,14 @@ static int fuse_sysfs_init(void)
> if (err)
> goto out_fuse_unregister;
>
> + err = fuse_iomap_sysfs_init(fuse_kobj);
> + if (err)
> + goto out_fuse_connections;
> +
> return 0;
>
> + out_fuse_connections:
> + sysfs_remove_mount_point(fuse_kobj, "connections");
> out_fuse_unregister:
> kobject_put(fuse_kobj);
> out_err:
> @@ -2287,6 +2293,7 @@ static int fuse_sysfs_init(void)
>
> static void fuse_sysfs_cleanup(void)
> {
> + fuse_iomap_sysfs_cleanup(fuse_kobj);
> sysfs_remove_mount_point(fuse_kobj, "connections");
> kobject_put(fuse_kobj);
> }
>
Could you explain why it's better that this goes through sysfs than
through a module param?
Thanks,
Joanne
Powered by blists - more mailing lists