* [PATCH 0/2] kernfs, autofs: fix pipe lock cycles through sysfs stores
[not found] <6aa23178.f81106d8.2ab401.0006.GAE@google.com>
@ 2026-10-21 12:51 ` Krystian Kaniewski
2026-10-21 12:51 ` [PATCH 1/2] kernfs: do not hold of->mutex across sysfs ->store() Krystian Kaniewski
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Krystian Kaniewski @ 2026-10-21 12:51 UTC (permalink / raw)
To: Greg Kroah-Hartman, Ian Kent
Cc: Tejun Heo, Rafael J . Wysocki, Danilo Krummrich, Alexander Viro,
Christian Brauner, Jan Kara, Ingo Molnar, Peter Zijlstra,
Andrew Morton, Jens Axboe, driver-core, autofs, linux-fsdevel,
linux-mm, linux-kernel, syzbot+002fb7b2d0bc53bf7e75
syzbot reports a lockdep cycle &pipe->mutex --> &of->mutex -->
&sbi->pipe_mutex. splice holds a pipe mutex while it calls the write or
read handler of a kernfs file, kernfs takes of->mutex there, and a write
to /sys/power/resume looks up a path that can make autofs write a request
into its daemon pipe and wait for the daemon.
Patch 1 stops holding of->mutex around the ->store() callback of regular
sysfs attributes, which do not get the open file and already run
concurrently for separate opens.
Patch 2 handles the case that patch 1 leaves. A store called from splice
still runs with the pipe locked. autofs no longer sends or waits for a
request in that context and returns -EAGAIN instead. A new mount is
refused before it is marked pending, so no other lookup sends the request
in its place. This also removes a deadlock in which the spliced pipe is
the autofs daemon pipe itself.
Patch 2 changes behavior for one narrow case. A lookup from a splice
callback that needs an automount fails with -EAGAIN instead of waiting.
The series was tested in QEMU with lockdep using the original C and syz
reproducers and focused splice and autofs cases. The target reports were
not reproduced with the series applied, and the paired functional cases
matched the unpatched baseline.
Krystian Kaniewski (2):
kernfs: do not hold of->mutex across sysfs ->store()
autofs: do not wait for the daemon while splice holds a pipe
fs/autofs/expire.c | 6 +++++
fs/autofs/root.c | 28 +++++++++++++++++++++--
fs/kernfs/file.c | 52 +++++++++++++++++++++++++++---------------
fs/splice.c | 13 ++++++++++-
fs/sysfs/file.c | 18 ++++++++++++++-
include/linux/kernfs.h | 11 +++++++++
include/linux/sched.h | 2 ++
kernel/fork.c | 1 +
8 files changed, 109 insertions(+), 22 deletions(-)
base-commit: 93f51579e7df248780214094418f205253383cc5
--
2.53.0
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/2] kernfs: do not hold of->mutex across sysfs ->store()
2026-10-21 12:51 ` [PATCH 0/2] kernfs, autofs: fix pipe lock cycles through sysfs stores Krystian Kaniewski
@ 2026-10-21 12:51 ` Krystian Kaniewski
2026-10-21 12:51 ` [PATCH 2/2] autofs: do not wait for the daemon while splice holds a pipe Krystian Kaniewski
2026-10-21 21:48 ` [PATCH 0/2] kernfs, autofs: fix pipe lock cycles through sysfs stores Tejun Heo
2 siblings, 0 replies; 7+ messages in thread
From: Krystian Kaniewski @ 2026-10-21 12:51 UTC (permalink / raw)
To: Greg Kroah-Hartman, Ian Kent
Cc: Tejun Heo, Rafael J . Wysocki, Danilo Krummrich, Alexander Viro,
Christian Brauner, Jan Kara, Ingo Molnar, Peter Zijlstra,
Andrew Morton, Jens Axboe, driver-core, autofs, linux-fsdevel,
linux-mm, linux-kernel, syzbot+002fb7b2d0bc53bf7e75
Lockdep reports a circular dependency &pipe->mutex --> &of->mutex -->
&sbi->pipe_mutex. A splice between a pipe and a kernfs file calls
->write_iter() or ->read_iter() of the file with the pipe locked, and
kernfs takes of->mutex there. A write to /sys/power/resume runs
resume_store() under of->mutex, and its lookup_bdev() can trigger an
autofs mount request, whose autofs_write() takes sbi->pipe_mutex and then
the mutex of the daemon pipe.
of->mutex only has to keep the callbacks of one open file apart.
sysfs_kf_write() does not pass the open file to ->store(), and two opens
of the same attribute already run ->store() concurrently. Add a
write_unlocked flag to struct kernfs_ops, cache it in struct
kernfs_open_file next to atomic_write_len, and take of->mutex around
->write() only when the flag is clear. Set it for the regular sysfs
attributes. The preallocated buffer stays protected by
of->prealloc_mutex, and the other kernfs users keep of->mutex.
This does not remove every report through the autofs daemon pipe. A pipe
spliced straight into such an attribute stays locked while ->store()
looks up the path, and a ->store() that takes its own lock around a
lookup keeps that dependency.
Fixes: 1e8c813b083c ("PM: hibernate: don't use early_lookup_bdev in resume_store")
Assisted-by: Gemini:gemini-3.8-flash syzbot
Reported-by: syzbot+002fb7b2d0bc53bf7e75@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=002fb7b2d0bc53bf7e75
Link: https://syzkaller.appspot.com/ai_job?id=74fbf9ab-54e4-46c2-b9e9-7867573ff823
Signed-off-by: Krystian Kaniewski <krystianmkaniewski@gmail.com>
---
fs/kernfs/file.c | 52 +++++++++++++++++++++++++++---------------
fs/sysfs/file.c | 18 ++++++++++++++-
include/linux/kernfs.h | 11 +++++++++
3 files changed, 62 insertions(+), 19 deletions(-)
diff --git a/fs/kernfs/file.c b/fs/kernfs/file.c
index 8e0e90c933720..c954543c1a583 100644
--- a/fs/kernfs/file.c
+++ b/fs/kernfs/file.c
@@ -291,6 +291,26 @@ static ssize_t kernfs_fop_read_iter(struct kiocb *iocb, struct iov_iter *iter)
return kernfs_file_read_iter(iocb, iter);
}
+/* Call the ->write() operation of @of under an active reference. */
+static ssize_t kernfs_write_active(struct kernfs_open_file *of, char *buf,
+ size_t len, loff_t pos)
+{
+ const struct kernfs_ops *ops;
+ ssize_t ret;
+
+ if (!kernfs_get_active_of(of))
+ return -ENODEV;
+
+ ops = kernfs_ops(of->kn);
+ if (ops->write)
+ ret = ops->write(of, buf, len, pos);
+ else
+ ret = -EINVAL;
+
+ kernfs_put_active_of(of);
+ return ret;
+}
+
/*
* Copy data in from userland and pass it to the matching kernfs write
* operation.
@@ -305,7 +325,6 @@ static ssize_t kernfs_fop_write_iter(struct kiocb *iocb, struct iov_iter *iter)
{
struct kernfs_open_file *of = kernfs_of(iocb->ki_filp);
ssize_t len = iov_iter_count(iter);
- const struct kernfs_ops *ops;
char *buf;
if (of->atomic_write_len) {
@@ -330,25 +349,18 @@ static ssize_t kernfs_fop_write_iter(struct kiocb *iocb, struct iov_iter *iter)
buf[len] = '\0'; /* guarantee string termination */
/*
- * @of->mutex nests outside active ref and is used both to ensure that
- * the ops aren't called concurrently for the same open file.
+ * @of->mutex nests outside active ref and keeps the ops of one open
+ * file from running concurrently. ->write() of files with
+ * ->write_unlocked set does not need that and runs without it.
*/
- mutex_lock(&of->mutex);
- if (!kernfs_get_active_of(of)) {
+ if (of->write_unlocked) {
+ len = kernfs_write_active(of, buf, len, iocb->ki_pos);
+ } else {
+ mutex_lock(&of->mutex);
+ len = kernfs_write_active(of, buf, len, iocb->ki_pos);
mutex_unlock(&of->mutex);
- len = -ENODEV;
- goto out_free;
}
- ops = kernfs_ops(of->kn);
- if (ops->write)
- len = ops->write(of, buf, len, iocb->ki_pos);
- else
- len = -EINVAL;
-
- kernfs_put_active_of(of);
- mutex_unlock(&of->mutex);
-
if (len > 0)
iocb->ki_pos += len;
@@ -651,6 +663,8 @@ static int kernfs_fop_open(struct inode *inode, struct file *file)
* lockdep key, because the writable file /sys/power/resume may call vfs
* lookup helpers for arbitrary paths and readonly files can be read by
* overlayfs from vfs helpers when sysfs is a lower layer of overalyfs.
+ * (Note that regular sysfs attributes no longer run ->store() under
+ * @of->mutex, but other kernfs users still do.)
*
* All three cases look the same. They're supposed to
* look that way and give @of->mutex different static lockdep keys.
@@ -666,10 +680,12 @@ static int kernfs_fop_open(struct inode *inode, struct file *file)
of->file = file;
/*
- * Write path needs to atomic_write_len outside active reference.
- * Cache it in open_file. See kernfs_fop_write_iter() for details.
+ * Write path needs atomic_write_len and write_unlocked outside
+ * active reference. Cache them in open_file. See
+ * kernfs_fop_write_iter() for details.
*/
of->atomic_write_len = ops->atomic_write_len;
+ of->write_unlocked = ops->write_unlocked;
error = -EINVAL;
/*
diff --git a/fs/sysfs/file.c b/fs/sysfs/file.c
index cd5bb0f9fee6d..71a7faf434073 100644
--- a/fs/sysfs/file.c
+++ b/fs/sysfs/file.c
@@ -132,7 +132,19 @@ static ssize_t sysfs_kf_read(struct kernfs_open_file *of, char *buf,
return min_t(ssize_t, count, len);
}
-/* kernfs write callback for regular sysfs files */
+/*
+ * kernfs write callback for regular sysfs files.
+ *
+ * Regular sysfs files set .write_unlocked = true and are called without
+ * holding of->mutex. Sysfs ->store() maintains no per-open-file state, and
+ * concurrent opens already execute ->store() concurrently without
+ * serialization. Calling ->store() without of->mutex avoids circular
+ * locking dependencies when ->store() performs operations like path
+ * lookups or automounts while a generic VFS lock is held (such as a pipe
+ * lock during splice). Preallocated attributes still serialize buffer
+ * access via of->prealloc_mutex. Existing autofs and callback-owned lock
+ * dependencies remain outside this narrow change.
+ */
static ssize_t sysfs_kf_write(struct kernfs_open_file *of, char *buf,
size_t count, loff_t pos)
{
@@ -229,11 +241,13 @@ static const struct kernfs_ops sysfs_file_kfops_ro = {
static const struct kernfs_ops sysfs_file_kfops_wo = {
.write = sysfs_kf_write,
+ .write_unlocked = true,
};
static const struct kernfs_ops sysfs_file_kfops_rw = {
.seq_show = sysfs_kf_seq_show,
.write = sysfs_kf_write,
+ .write_unlocked = true,
};
static const struct kernfs_ops sysfs_prealloc_kfops_ro = {
@@ -244,12 +258,14 @@ static const struct kernfs_ops sysfs_prealloc_kfops_ro = {
static const struct kernfs_ops sysfs_prealloc_kfops_wo = {
.write = sysfs_kf_write,
.prealloc = true,
+ .write_unlocked = true,
};
static const struct kernfs_ops sysfs_prealloc_kfops_rw = {
.read = sysfs_kf_read,
.write = sysfs_kf_write,
.prealloc = true,
+ .write_unlocked = true,
};
static const struct kernfs_ops sysfs_bin_kfops_ro = {
diff --git a/include/linux/kernfs.h b/include/linux/kernfs.h
index 351a5101c8628..cbb09f9533e72 100644
--- a/include/linux/kernfs.h
+++ b/include/linux/kernfs.h
@@ -272,6 +272,7 @@ struct kernfs_open_file {
char *prealloc_buf;
size_t atomic_write_len;
+ bool write_unlocked;
bool mmapped:1;
bool released:1;
const struct vm_operations_struct *vm_ops;
@@ -320,6 +321,16 @@ struct kernfs_ops {
* ->prealloc. Provide ->read and ->write with ->prealloc.
*/
bool prealloc;
+ /*
+ * "write_unlocked" indicates that ->write() does not require
+ * per-open-file serialization with of->mutex and can be called
+ * concurrently for the same open file. Callbacks must handle
+ * concurrent execution. Buffer access for preallocated attributes
+ * is still protected by of->prealloc_mutex. Lock dependencies from
+ * callers (such as pipe locks during splice) or within callbacks
+ * remain outside this scope.
+ */
+ bool write_unlocked;
ssize_t (*write)(struct kernfs_open_file *of, char *buf, size_t bytes,
loff_t off);
--
2.53.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/2] autofs: do not wait for the daemon while splice holds a pipe
2026-10-21 12:51 ` [PATCH 0/2] kernfs, autofs: fix pipe lock cycles through sysfs stores Krystian Kaniewski
2026-10-21 12:51 ` [PATCH 1/2] kernfs: do not hold of->mutex across sysfs ->store() Krystian Kaniewski
@ 2026-10-21 12:51 ` Krystian Kaniewski
2026-10-21 21:48 ` [PATCH 0/2] kernfs, autofs: fix pipe lock cycles through sysfs stores Tejun Heo
2 siblings, 0 replies; 7+ messages in thread
From: Krystian Kaniewski @ 2026-10-21 12:51 UTC (permalink / raw)
To: Greg Kroah-Hartman, Ian Kent
Cc: Tejun Heo, Rafael J . Wysocki, Danilo Krummrich, Alexander Viro,
Christian Brauner, Jan Kara, Ingo Molnar, Peter Zijlstra,
Andrew Morton, Jens Axboe, driver-core, autofs, linux-fsdevel,
linux-mm, linux-kernel, syzbot+002fb7b2d0bc53bf7e75
splice holds the mutex of a pipe while it calls ->write_iter() of the
target file in iter_file_splice_write() or ->splice_read() of the source
file in splice_file_to_pipe(). When such a callback looks up a path in an
autofs mount, autofs sends a request through its daemon pipe and waits
for the daemon. A write to /sys/power/resume that comes from splice() does
this, so lockdep still reports &pipe->mutex --> &sbi->pipe_mutex -->
&pipe->mutex after of->mutex is gone from the sysfs store path. When the
spliced pipe is the daemon pipe itself, the task can wait for the mutex
that it already holds, which SIGKILL does not end, or wait for a daemon
that cannot read its pipe.
Mark the task while splice runs such a callback with a pipe really locked
(pipe->files set, as in pipe_lock()), and make autofs return -EAGAIN in
that context instead of sending a mount request or waiting for a pending
mount or expire. A new mount is refused before AUTOFS_INF_PENDING is set,
because another lookup that sees the flag would send the request instead.
The short wait for expire selection does not depend on the daemon and is
kept. RCU walks, lookups that need no request and all other callers keep
their behavior, and a child does not inherit the mark.
A lookup from such a callback that needs an automount now fails with
-EAGAIN instead of waiting for the daemon. This affects splice() from a
pipe into a file whose write handler looks up a path under an autofs
mount that is not mounted yet, for example /sys/power/resume. sendfile()
into a file goes through the private pipe of the task, which is never
locked, and keeps its behavior.
Fixes: 1e8c813b083c ("PM: hibernate: don't use early_lookup_bdev in resume_store")
Reported-by: syzbot+002fb7b2d0bc53bf7e75@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=002fb7b2d0bc53bf7e75
Signed-off-by: Krystian Kaniewski <krystianmkaniewski@gmail.com>
---
fs/autofs/expire.c | 6 ++++++
fs/autofs/root.c | 28 ++++++++++++++++++++++++++--
fs/splice.c | 13 ++++++++++++-
include/linux/sched.h | 2 ++
kernel/fork.c | 1 +
5 files changed, 47 insertions(+), 3 deletions(-)
diff --git a/fs/autofs/expire.c b/fs/autofs/expire.c
index 5c2d459e1e484..9f3bf1473acd4 100644
--- a/fs/autofs/expire.c
+++ b/fs/autofs/expire.c
@@ -514,6 +514,12 @@ int autofs_expire_wait(const struct path *path, int rcu_walk)
}
if (state & AUTOFS_INF_EXPIRING) {
spin_unlock(&sbi->fs_lock);
+ /*
+ * The expire waits for the daemon, which may need a pipe that
+ * the splice of this task holds. Refuse instead of waiting.
+ */
+ if (unlikely(current->in_splice_pipe_callback))
+ return -EAGAIN;
pr_debug("waiting for expire %p name=%pd\n", dentry, dentry);
diff --git a/fs/autofs/root.c b/fs/autofs/root.c
index b36439f4521eb..c4e04998a2345 100644
--- a/fs/autofs/root.c
+++ b/fs/autofs/root.c
@@ -252,6 +252,8 @@ static int autofs_mount_wait(const struct path *path, bool rcu_walk)
if (ino->flags & AUTOFS_INF_PENDING) {
if (rcu_walk)
return -ECHILD;
+ if (unlikely(current->in_splice_pipe_callback))
+ return -EAGAIN;
pr_debug("waiting for mount name=%pd\n", path->dentry);
status = autofs_wait(sbi, path, NFY_MOUNT);
pr_debug("mount wait done status=%d\n", status);
@@ -263,10 +265,18 @@ static int autofs_mount_wait(const struct path *path, bool rcu_walk)
return status;
}
+static bool autofs_expire_wait_blocked(int status)
+{
+ /* A marked caller is refused before a daemon can return a status. */
+ return status == -EAGAIN &&
+ unlikely(current->in_splice_pipe_callback);
+}
+
static int do_expire_wait(const struct path *path, bool rcu_walk)
{
struct dentry *dentry = path->dentry;
struct dentry *expiring;
+ int status;
expiring = autofs_lookup_expiring(dentry, rcu_walk);
if (IS_ERR(expiring))
@@ -280,7 +290,11 @@ static int do_expire_wait(const struct path *path, bool rcu_walk)
* be quite complete, but the directory has been removed
* so it must have been successful, just wait for it.
*/
- autofs_expire_wait(&this, 0);
+ status = autofs_expire_wait(&this, 0);
+ if (autofs_expire_wait_blocked(status)) {
+ dput(expiring);
+ return status;
+ }
autofs_del_expiring(expiring);
dput(expiring);
}
@@ -359,6 +373,8 @@ static struct vfsmount *autofs_d_automount(struct path *path)
* the mount.
*/
status = do_expire_wait(path, 0);
+ if (autofs_expire_wait_blocked(status))
+ return ERR_PTR(-EAGAIN);
if (status && status != -EAGAIN)
return NULL;
@@ -405,6 +421,11 @@ static struct vfsmount *autofs_d_automount(struct path *path)
goto done;
}
}
+ /* Do not publish PENDING without issuing its mount request. */
+ if (unlikely(current->in_splice_pipe_callback)) {
+ spin_unlock(&sbi->fs_lock);
+ return ERR_PTR(-EAGAIN);
+ }
ino->flags |= AUTOFS_INF_PENDING;
spin_unlock(&sbi->fs_lock);
status = autofs_mount_wait(path, 0);
@@ -442,8 +463,11 @@ static int autofs_d_manage(const struct path *path, bool rcu_walk)
}
/* Wait for pending expires */
- if (do_expire_wait(path, rcu_walk) == -ECHILD)
+ status = do_expire_wait(path, rcu_walk);
+ if (status == -ECHILD)
return -ECHILD;
+ if (autofs_expire_wait_blocked(status))
+ return -EAGAIN;
/*
* This dentry may be under construction so wait on mount
diff --git a/fs/splice.c b/fs/splice.c
index 9d8f63e2fd1ab..1756a8579776f 100644
--- a/fs/splice.c
+++ b/fs/splice.c
@@ -686,6 +686,7 @@ iter_file_splice_write(struct pipe_inode_info *pipe, struct file *out,
struct kiocb kiocb;
struct iov_iter from;
unsigned int head, tail;
+ bool was_in_splice_pipe_callback;
size_t left;
int n;
@@ -733,7 +734,11 @@ iter_file_splice_write(struct pipe_inode_info *pipe, struct file *out,
iov_iter_bvec(&from, ITER_SOURCE, array, n, sd.total_len - left);
init_sync_kiocb(&kiocb, out);
kiocb.ki_pos = sd.pos;
+ was_in_splice_pipe_callback = current->in_splice_pipe_callback;
+ if (pipe->files)
+ current->in_splice_pipe_callback = true;
ret = out->f_op->write_iter(&kiocb, &from);
+ current->in_splice_pipe_callback = was_in_splice_pipe_callback;
sd.pos = kiocb.ki_pos;
if (ret <= 0)
break;
@@ -1282,12 +1287,18 @@ ssize_t splice_file_to_pipe(struct file *in,
loff_t *offset,
size_t len, unsigned int flags)
{
+ bool was_in_splice_pipe_callback;
ssize_t ret;
pipe_lock(opipe);
ret = wait_for_space(opipe, flags);
- if (!ret)
+ if (!ret) {
+ was_in_splice_pipe_callback = current->in_splice_pipe_callback;
+ if (opipe->files)
+ current->in_splice_pipe_callback = true;
ret = do_splice_read(in, offset, opipe, len, flags);
+ current->in_splice_pipe_callback = was_in_splice_pipe_callback;
+ }
pipe_unlock(opipe);
if (ret > 0)
wakeup_pipe_readers(opipe);
diff --git a/include/linux/sched.h b/include/linux/sched.h
index 705970d07614f..3ffa284976cc6 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -1025,6 +1025,8 @@ struct task_struct {
/* Bit to tell TOMOYO we're in execve(): */
unsigned in_execve:1;
unsigned in_iowait:1;
+ /* A file callback is running while splice holds a pipe mutex. */
+ unsigned in_splice_pipe_callback:1;
#ifndef TIF_RESTORE_SIGMASK
unsigned restore_sigmask:1;
#endif
diff --git a/kernel/fork.c b/kernel/fork.c
index 5ef413368912e..d774e5092766c 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -2133,6 +2133,7 @@ __latent_entropy struct task_struct *copy_process(
p = dup_task_struct(current, node);
if (!p)
goto fork_out;
+ p->in_splice_pipe_callback = 0;
/*
* Must run before the first fallible op, so error paths never
* free the parent's ret_stack.
--
2.53.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 0/2] kernfs, autofs: fix pipe lock cycles through sysfs stores
2026-10-21 12:51 ` [PATCH 0/2] kernfs, autofs: fix pipe lock cycles through sysfs stores Krystian Kaniewski
2026-10-21 12:51 ` [PATCH 1/2] kernfs: do not hold of->mutex across sysfs ->store() Krystian Kaniewski
2026-10-21 12:51 ` [PATCH 2/2] autofs: do not wait for the daemon while splice holds a pipe Krystian Kaniewski
@ 2026-10-21 21:48 ` Tejun Heo
2026-10-22 2:46 ` Ian Kent
2 siblings, 1 reply; 7+ messages in thread
From: Tejun Heo @ 2026-10-21 21:48 UTC (permalink / raw)
To: Krystian Kaniewski
Cc: Greg Kroah-Hartman, Ian Kent, Rafael J . Wysocki,
Danilo Krummrich, Alexander Viro, Christian Brauner, Jan Kara,
Ingo Molnar, Peter Zijlstra, Andrew Morton, Jens Axboe,
driver-core, autofs, linux-fsdevel, linux-mm, linux-kernel,
syzbot+002fb7b2d0bc53bf7e75
Hello, Krystian.
On Wed, Oct 21, 2026 at 02:51:48PM +0200, Krystian Kaniewski wrote:
> syzbot reports a lockdep cycle &pipe->mutex --> &of->mutex -->
> &sbi->pipe_mutex. splice holds a pipe mutex while it calls the write or
> read handler of a kernfs file, kernfs takes of->mutex there, and a write
> to /sys/power/resume looks up a path that can make autofs write a request
> into its daemon pipe and wait for the daemon.
This is an extremely convoluted case that isn't going to happen in any
realistic scenario, and it doesn't make sense to change the locking scheme
for it. Can we just disallow splice on /sys/power/resume instead?
Thanks.
--
tejun
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 0/2] kernfs, autofs: fix pipe lock cycles through sysfs stores
2026-10-21 21:48 ` [PATCH 0/2] kernfs, autofs: fix pipe lock cycles through sysfs stores Tejun Heo
@ 2026-10-22 2:46 ` Ian Kent
2026-10-23 14:05 ` Krystian Kaniewski
0 siblings, 1 reply; 7+ messages in thread
From: Ian Kent @ 2026-10-22 2:46 UTC (permalink / raw)
To: Tejun Heo, Krystian Kaniewski
Cc: Greg Kroah-Hartman, Rafael J . Wysocki, Danilo Krummrich,
Alexander Viro, Christian Brauner, Jan Kara, Ingo Molnar,
Peter Zijlstra, Andrew Morton, Jens Axboe, driver-core, autofs,
linux-fsdevel, linux-mm, linux-kernel,
syzbot+002fb7b2d0bc53bf7e75
On Wed, 2026-10-21 at 11:48 -1000, Tejun Heo wrote:
> Hello, Krystian.
>
> On Wed, Oct 21, 2026 at 02:51:48PM +0200, Krystian Kaniewski wrote:
> > syzbot reports a lockdep cycle &pipe->mutex --> &of->mutex -->
> > &sbi->pipe_mutex. splice holds a pipe mutex while it calls the
> > write or
> > read handler of a kernfs file, kernfs takes of->mutex there, and a
> > write
> > to /sys/power/resume looks up a path that can make autofs write a
> > request
> > into its daemon pipe and wait for the daemon.
>
> This is an extremely convoluted case that isn't going to happen in
> any
> realistic scenario, and it doesn't make sense to change the locking
> scheme
> for it. Can we just disallow splice on /sys/power/resume instead?
I agree but it might not be as simple as disallowing that path.
Not only is the description hard to follow the example is flawed
because there should not be a mount callback on anything that's
not an empty directory or an autofs trigger mount (and the above
should be able to be either (systemd's /proc/sys/fs/binfmt_misc
is an example of an autofs trigger mount).
I mostly can't grok the reproducers for things like this so a
complicated description makes it so much harder to come up with
a sensible solution.
The other possibility I was thinking about when I saw that report
is to disallow file systems that don't make sense for automount.
That would take some thought though and isn't entirely clear for
kernfs since it's used by other file systems that may occasionally
need to automount something at some point.
OTOH, if the problem description could be cleaned up, and
simplified, I wouldn't be entirely against the change but still
wouldn't much like it. That's purely because I've always been in
favor of defensing programming even for cases that are unlikely.
Ian
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 0/2] kernfs, autofs: fix pipe lock cycles through sysfs stores
2026-10-22 2:46 ` Ian Kent
@ 2026-10-23 14:05 ` Krystian Kaniewski
2026-10-23 18:33 ` Tejun Heo
0 siblings, 1 reply; 7+ messages in thread
From: Krystian Kaniewski @ 2026-10-23 14:05 UTC (permalink / raw)
To: Ian Kent, Tejun Heo
Cc: Greg Kroah-Hartman, Rafael J . Wysocki, Danilo Krummrich,
Alexander Viro, Christian Brauner, Jan Kara, Ingo Molnar,
Peter Zijlstra, Andrew Morton, Jens Axboe, driver-core, autofs,
linux-fsdevel, linux-mm, linux-kernel,
syzbot+002fb7b2d0bc53bf7e75
Hi Tejun, Ian,
On 10/22/26 04:46, Ian Kent wrote:
> On Wed, 2026-10-21 at 11:48 -1000, Tejun Heo wrote:
>> it doesn't make sense to change the locking scheme for it.
>> Can we just disallow splice on /sys/power/resume instead?
That would address the direct case, but it would not make this report
go away. The lockdep report combines two separate operations.
First, the reproducer splices a pipe into the cgroup v1 devices.deny
file. This records:
&pipe->mutex -> &of->mutex
It later uses a normal write() to /sys/power/resume. That records:
&of->mutex -> &sbi->pipe_mutex -> &pipe->mutex
There is no splice into /sys/power/resume in the reported sequence.
Disabling splice on that file would prevent some direct deadlock
cases, but the first edge of this report would still come from
devices.deny.
A complete restriction for this attribute would also have to cover
both splice directions. With an O_RDWR open, a splice read takes the
output pipe mutex before the same of->mutex. Another thread can write
through the same open and reach autofs in the opposite order.
Kernfs splice support is established behavior. Commit f2d6c2708bd8
("kernfs: wire up ->splice_read and ->splice_write") explicitly
restored it in v5.11 after the VFS stopped providing the implicit
fallback. The pipe has been locked across ->write_iter() since 2014.
I agree that changing serialization for every regular sysfs store is
too broad for this case. Before preparing a v2, I would like to
clarify the intended scope.
> Not only is the description hard to follow the example is flawed
You are right that the cover letter was hard to follow. I should have
described the two operations separately. The automount itself uses the
normal empty directory case.
The reproducer creates a detached indirect autofs mount with its own
daemon pipe. It changes its current directory to the root of that
mount and moves out of the daemon process group. It then writes the
relative name "0x0000000004000921" to /sys/power/resume.
resume_store() calls lookup_bdev() on that string before trying to
parse it as a device number. The pathname lookup therefore starts in
the autofs root. The new negative name triggers the normal mount
request for an empty indirect autofs directory.
The dentry whose ->d_automount callback runs belongs to autofs, not
to sysfs. There is no autofs mount on top of kernfs in this
reproducer. Kernfs is involved only because resume_store() starts the
pathname lookup.
The change which made this path possible is 1e8c813b083c ("PM:
hibernate: don't use early_lookup_bdev in resume_store"). It replaced
early_lookup_bdev() with the normal pathname lookup in v6.5. That is
what made autofs reachable from resume_store().
> The other possibility I was thinking about when I saw that report
> is to disallow file systems that don't make sense for automount.
I may be misunderstanding this suggestion. In this reproducer, the
filesystem which owns the automount dentry is autofs. A restriction
based on the filesystem owning that dentry would therefore see autofs,
not kernfs.
The lookup from a sysfs store is also not unique to
/sys/power/resume. For example, with CONFIG_BCACHE=y,
sys/fs/bcache/register also resolves a user supplied pathname.
The exact reproducer records the lockdep edges with different pipes
and different open files. It does not deadlock on those mutex
instances. After sending the request, it waits killably for a daemon
reply that never arrives. Related setups can deadlock when the daemon
pipe itself participates in splice or tee.
Given these two separate paths, did you intend to prevent only direct
splice operations on /sys/power/resume even though the reported class
cycle would remain, or should a replacement also eliminate the
lockdep report?
A per-attribute splice restriction alone cannot do both. If this does
not justify new kernfs infrastructure, I can leave it alone.
Thanks,
--
Krystian Kaniewski
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 0/2] kernfs, autofs: fix pipe lock cycles through sysfs stores
2026-10-23 14:05 ` Krystian Kaniewski
@ 2026-10-23 18:33 ` Tejun Heo
0 siblings, 0 replies; 7+ messages in thread
From: Tejun Heo @ 2026-10-23 18:33 UTC (permalink / raw)
To: Krystian Kaniewski
Cc: Ian Kent, Greg Kroah-Hartman, Rafael J . Wysocki,
Danilo Krummrich, Alexander Viro, Christian Brauner, Jan Kara,
Ingo Molnar, Peter Zijlstra, Andrew Morton, Jens Axboe,
driver-core, autofs, linux-fsdevel, linux-mm, linux-kernel,
syzbot+002fb7b2d0bc53bf7e75
Hello, Krystian.
On Fri, Oct 23, 2026 at 04:05:38PM +0200, Krystian Kaniewski wrote:
> Given these two separate paths, did you intend to prevent only direct
> splice operations on /sys/power/resume even though the reported class
> cycle would remain, or should a replacement also eliminate the
> lockdep report?
The lockdep report should go away too.
What about disabling splice only for the files whose ->store() can look
up arbitrary paths, such as /sys/power/resume, and giving their of->mutex
a separate lockdep class? Splice is what nests of->mutex inside
pipe->mutex, so if those files can't be spliced, pipe->mutex never nests
outside the new class and the cycle is gone. Commit 16b52bbee482
("kernfs: annotate different lockdep class for of->mutex of writable
files") did the same kind of split for writable files.
Thanks.
--
tejun
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2026-10-23 18:33 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <6aa23178.f81106d8.2ab401.0006.GAE@google.com>
2026-10-21 12:51 ` [PATCH 0/2] kernfs, autofs: fix pipe lock cycles through sysfs stores Krystian Kaniewski
2026-10-21 12:51 ` [PATCH 1/2] kernfs: do not hold of->mutex across sysfs ->store() Krystian Kaniewski
2026-10-21 12:51 ` [PATCH 2/2] autofs: do not wait for the daemon while splice holds a pipe Krystian Kaniewski
2026-10-21 21:48 ` [PATCH 0/2] kernfs, autofs: fix pipe lock cycles through sysfs stores Tejun Heo
2026-10-22 2:46 ` Ian Kent
2026-10-23 14:05 ` Krystian Kaniewski
2026-10-23 18:33 ` Tejun Heo
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox