| 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: <20260122000227.GK5966@frogsfrogsfrogs>
Date: Wed, 21 Jan 2026 16:02:27 -0800
From: "Darrick J. Wong" <djwong@...nel.org>
To: Joanne Koong <joannelkoong@...il.com>
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 Wed, Jan 21, 2026 at 03:42:04PM -0800, Joanne Koong wrote:
> 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 is supposed to tear down the attrs that sysfs_create_files
attaches to iomap_kobj. Though you're right to be suspicious -- there
are a lot of places that explicitly call sysfs_remove_files to undo
sysfs_create_files; and also a lot of places that just let kobject_put
do the dirty work.
> > + 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?
You can dynamically enable debugging on a production system. I (by
which I really mean the support org) wishes they could do that with XFS.
Module parameters don't come with setter functions so you can't call
static_branch_{enable,disable} when the parameter value updates.
--D
> Thanks,
> Joanne
Powered by blists - more mailing lists