* [PATCH v3 1/7] fuse: abort on fatal signal during sync init
2026-03-16 16:53 [PATCH v3 0/7] fuse: fix hang with sync init Miklos Szeredi
@ 2026-03-16 16:53 ` Miklos Szeredi
2026-03-16 18:48 ` Joanne Koong
2026-03-17 20:19 ` Bernd Schubert
2026-03-16 16:53 ` [PATCH v3 2/7] fuse: create fuse_dev on /dev/fuse open instead of mount Miklos Szeredi
` (5 subsequent siblings)
6 siblings, 2 replies; 25+ messages in thread
From: Miklos Szeredi @ 2026-03-16 16:53 UTC (permalink / raw)
To: linux-fsdevel; +Cc: Bernd Schubert, stable
When sync init is used and the server exits for some reason (error, crash)
while processing FUSE_INIT, the filesystem creation will hang. The reason
is that while all other threads will exit, the mounting thread (or process)
will keep the device fd open, which will prevent an abort from happening.
This is a regression from the async mount case, where the mount was done
first, and the FUSE_INIT processing afterwards, in which case there's no
such recursive syscall keeping the fd open.
Fixes: dfb84c330794 ("fuse: allow synchronous FUSE_INIT")
Cc: stable@vger.kernel.org # v6.18
Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
---
fs/fuse/dev.c | 6 +++++-
fs/fuse/fuse_i.h | 1 +
fs/fuse/inode.c | 1 +
3 files changed, 7 insertions(+), 1 deletion(-)
diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c
index 2c16b94357d5..f0631c48abef 100644
--- a/fs/fuse/dev.c
+++ b/fs/fuse/dev.c
@@ -576,6 +576,9 @@ static void request_wait_answer(struct fuse_req *req)
removed = fuse_remove_pending_req(req, &fiq->lock);
if (removed)
return;
+
+ if (req->args->abort_on_kill)
+ fuse_abort_conn(fc);
}
/*
@@ -676,7 +679,8 @@ ssize_t __fuse_simple_request(struct mnt_idmap *idmap,
fuse_force_creds(req);
__set_bit(FR_WAITING, &req->flags);
- __set_bit(FR_FORCE, &req->flags);
+ if (!args->abort_on_kill)
+ __set_bit(FR_FORCE, &req->flags);
} else {
WARN_ON(args->nocreds);
req = fuse_get_req(idmap, fm, false);
diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
index 7f16049387d1..23a241f18623 100644
--- a/fs/fuse/fuse_i.h
+++ b/fs/fuse/fuse_i.h
@@ -345,6 +345,7 @@ struct fuse_args {
bool is_ext:1;
bool is_pinned:1;
bool invalidate_vmap:1;
+ bool abort_on_kill:1;
struct fuse_in_arg in_args[4];
struct fuse_arg out_args[2];
void (*end)(struct fuse_mount *fm, struct fuse_args *args, int error);
diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
index e57b8af06be9..84f78fb89d35 100644
--- a/fs/fuse/inode.c
+++ b/fs/fuse/inode.c
@@ -1551,6 +1551,7 @@ int fuse_send_init(struct fuse_mount *fm)
int err;
if (fm->fc->sync_init) {
+ ia->args.abort_on_kill = true;
err = fuse_simple_request(fm, &ia->args);
/* Ignore size of init reply */
if (err > 0)
--
2.53.0
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH v3 1/7] fuse: abort on fatal signal during sync init
2026-03-16 16:53 ` [PATCH v3 1/7] fuse: abort on fatal signal during " Miklos Szeredi
@ 2026-03-16 18:48 ` Joanne Koong
2026-03-23 17:53 ` Darrick J. Wong
2026-03-17 20:19 ` Bernd Schubert
1 sibling, 1 reply; 25+ messages in thread
From: Joanne Koong @ 2026-03-16 18:48 UTC (permalink / raw)
To: Miklos Szeredi; +Cc: linux-fsdevel, Bernd Schubert, stable
On Mon, Mar 16, 2026 at 9:53 AM Miklos Szeredi <mszeredi@redhat.com> wrote:
>
> When sync init is used and the server exits for some reason (error, crash)
> while processing FUSE_INIT, the filesystem creation will hang. The reason
> is that while all other threads will exit, the mounting thread (or process)
> will keep the device fd open, which will prevent an abort from happening.
>
> This is a regression from the async mount case, where the mount was done
> first, and the FUSE_INIT processing afterwards, in which case there's no
> such recursive syscall keeping the fd open.
>
> Fixes: dfb84c330794 ("fuse: allow synchronous FUSE_INIT")
> Cc: stable@vger.kernel.org # v6.18
> Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
LGTM but left a comment below
Reviewed-by: Joanne Koong <joannelkoong@gmail.com>
> ---
> fs/fuse/dev.c | 6 +++++-
> fs/fuse/fuse_i.h | 1 +
> fs/fuse/inode.c | 1 +
> 3 files changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c
> index 2c16b94357d5..f0631c48abef 100644
> --- a/fs/fuse/dev.c
> +++ b/fs/fuse/dev.c
> @@ -576,6 +576,9 @@ static void request_wait_answer(struct fuse_req *req)
> removed = fuse_remove_pending_req(req, &fiq->lock);
> if (removed)
> return;
> +
> + if (req->args->abort_on_kill)
> + fuse_abort_conn(fc);
Maybe more straightforward to move this logic a few lines above? eg
@@ -570,6 +570,11 @@ static void request_wait_answer(struct fuse_req *req)
/* Only fatal signals may interrupt this */
err = wait_event_killable(req->waitq,
test_bit(FR_FINISHED, &req->flags));
if (!err)
return;
+ if (req->args->abort_on_kill) {
+ fuse_abort_conn(fc);
+ return;
+ }
Thanks,
Joanne
> }
>
> /*
> @@ -676,7 +679,8 @@ ssize_t __fuse_simple_request(struct mnt_idmap *idmap,
> fuse_force_creds(req);
>
> __set_bit(FR_WAITING, &req->flags);
> - __set_bit(FR_FORCE, &req->flags);
> + if (!args->abort_on_kill)
> + __set_bit(FR_FORCE, &req->flags);
> } else {
> WARN_ON(args->nocreds);
> req = fuse_get_req(idmap, fm, false);
> diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
> index 7f16049387d1..23a241f18623 100644
> --- a/fs/fuse/fuse_i.h
> +++ b/fs/fuse/fuse_i.h
> @@ -345,6 +345,7 @@ struct fuse_args {
> bool is_ext:1;
> bool is_pinned:1;
> bool invalidate_vmap:1;
> + bool abort_on_kill:1;
> struct fuse_in_arg in_args[4];
> struct fuse_arg out_args[2];
> void (*end)(struct fuse_mount *fm, struct fuse_args *args, int error);
> diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
> index e57b8af06be9..84f78fb89d35 100644
> --- a/fs/fuse/inode.c
> +++ b/fs/fuse/inode.c
> @@ -1551,6 +1551,7 @@ int fuse_send_init(struct fuse_mount *fm)
> int err;
>
> if (fm->fc->sync_init) {
> + ia->args.abort_on_kill = true;
> err = fuse_simple_request(fm, &ia->args);
> /* Ignore size of init reply */
> if (err > 0)
> --
> 2.53.0
>
>
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [PATCH v3 1/7] fuse: abort on fatal signal during sync init
2026-03-16 18:48 ` Joanne Koong
@ 2026-03-23 17:53 ` Darrick J. Wong
0 siblings, 0 replies; 25+ messages in thread
From: Darrick J. Wong @ 2026-03-23 17:53 UTC (permalink / raw)
To: Joanne Koong; +Cc: Miklos Szeredi, linux-fsdevel, Bernd Schubert, stable
On Mon, Mar 16, 2026 at 11:48:09AM -0700, Joanne Koong wrote:
> On Mon, Mar 16, 2026 at 9:53 AM Miklos Szeredi <mszeredi@redhat.com> wrote:
> >
> > When sync init is used and the server exits for some reason (error, crash)
> > while processing FUSE_INIT, the filesystem creation will hang. The reason
> > is that while all other threads will exit, the mounting thread (or process)
> > will keep the device fd open, which will prevent an abort from happening.
> >
> > This is a regression from the async mount case, where the mount was done
> > first, and the FUSE_INIT processing afterwards, in which case there's no
> > such recursive syscall keeping the fd open.
> >
> > Fixes: dfb84c330794 ("fuse: allow synchronous FUSE_INIT")
> > Cc: stable@vger.kernel.org # v6.18
> > Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
>
> LGTM but left a comment below
>
> Reviewed-by: Joanne Koong <joannelkoong@gmail.com>
>
> > ---
> > fs/fuse/dev.c | 6 +++++-
> > fs/fuse/fuse_i.h | 1 +
> > fs/fuse/inode.c | 1 +
> > 3 files changed, 7 insertions(+), 1 deletion(-)
> >
> > diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c
> > index 2c16b94357d5..f0631c48abef 100644
> > --- a/fs/fuse/dev.c
> > +++ b/fs/fuse/dev.c
> > @@ -576,6 +576,9 @@ static void request_wait_answer(struct fuse_req *req)
> > removed = fuse_remove_pending_req(req, &fiq->lock);
> > if (removed)
> > return;
> > +
> > + if (req->args->abort_on_kill)
> > + fuse_abort_conn(fc);
>
> Maybe more straightforward to move this logic a few lines above? eg
>
> @@ -570,6 +570,11 @@ static void request_wait_answer(struct fuse_req *req)
> /* Only fatal signals may interrupt this */
> err = wait_event_killable(req->waitq,
> test_bit(FR_FINISHED, &req->flags));
> if (!err)
> return;
>
> + if (req->args->abort_on_kill) {
> + fuse_abort_conn(fc);
> + return;
> + }
Either's fine with me -- AFAICT it doesn't matter if we just abort the
connection without fiddling with the request in which case FUSE_INIT
fails with ECONNABORTED; or we try to remove it and end up cancelling it
with EINTR. Either way the connection aborts, the mount stops working,
and the fuse server is gone.
I like this fix much better :)
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>
--D
>
> Thanks,
> Joanne
>
> > }
> >
> > /*
> > @@ -676,7 +679,8 @@ ssize_t __fuse_simple_request(struct mnt_idmap *idmap,
> > fuse_force_creds(req);
> >
> > __set_bit(FR_WAITING, &req->flags);
> > - __set_bit(FR_FORCE, &req->flags);
> > + if (!args->abort_on_kill)
> > + __set_bit(FR_FORCE, &req->flags);
> > } else {
> > WARN_ON(args->nocreds);
> > req = fuse_get_req(idmap, fm, false);
> > diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
> > index 7f16049387d1..23a241f18623 100644
> > --- a/fs/fuse/fuse_i.h
> > +++ b/fs/fuse/fuse_i.h
> > @@ -345,6 +345,7 @@ struct fuse_args {
> > bool is_ext:1;
> > bool is_pinned:1;
> > bool invalidate_vmap:1;
> > + bool abort_on_kill:1;
> > struct fuse_in_arg in_args[4];
> > struct fuse_arg out_args[2];
> > void (*end)(struct fuse_mount *fm, struct fuse_args *args, int error);
> > diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
> > index e57b8af06be9..84f78fb89d35 100644
> > --- a/fs/fuse/inode.c
> > +++ b/fs/fuse/inode.c
> > @@ -1551,6 +1551,7 @@ int fuse_send_init(struct fuse_mount *fm)
> > int err;
> >
> > if (fm->fc->sync_init) {
> > + ia->args.abort_on_kill = true;
> > err = fuse_simple_request(fm, &ia->args);
> > /* Ignore size of init reply */
> > if (err > 0)
> > --
> > 2.53.0
> >
> >
>
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v3 1/7] fuse: abort on fatal signal during sync init
2026-03-16 16:53 ` [PATCH v3 1/7] fuse: abort on fatal signal during " Miklos Szeredi
2026-03-16 18:48 ` Joanne Koong
@ 2026-03-17 20:19 ` Bernd Schubert
2026-03-18 9:33 ` Miklos Szeredi
1 sibling, 1 reply; 25+ messages in thread
From: Bernd Schubert @ 2026-03-17 20:19 UTC (permalink / raw)
To: Miklos Szeredi, linux-fsdevel; +Cc: stable
On 3/16/26 17:53, Miklos Szeredi wrote:
> When sync init is used and the server exits for some reason (error, crash)
> while processing FUSE_INIT, the filesystem creation will hang. The reason
> is that while all other threads will exit, the mounting thread (or process)
> will keep the device fd open, which will prevent an abort from happening.
>
> This is a regression from the async mount case, where the mount was done
> first, and the FUSE_INIT processing afterwards, in which case there's no
> such recursive syscall keeping the fd open.
>
> Fixes: dfb84c330794 ("fuse: allow synchronous FUSE_INIT")
> Cc: stable@vger.kernel.org # v6.18
> Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
> ---
> fs/fuse/dev.c | 6 +++++-
> fs/fuse/fuse_i.h | 1 +
> fs/fuse/inode.c | 1 +
> 3 files changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c
> index 2c16b94357d5..f0631c48abef 100644
> --- a/fs/fuse/dev.c
> +++ b/fs/fuse/dev.c
> @@ -576,6 +576,9 @@ static void request_wait_answer(struct fuse_req *req)
> removed = fuse_remove_pending_req(req, &fiq->lock);
> if (removed)
> return;
> +
> + if (req->args->abort_on_kill)
> + fuse_abort_conn(fc);
> }
>
> /*
> @@ -676,7 +679,8 @@ ssize_t __fuse_simple_request(struct mnt_idmap *idmap,
> fuse_force_creds(req);
>
> __set_bit(FR_WAITING, &req->flags);
> - __set_bit(FR_FORCE, &req->flags);
> + if (!args->abort_on_kill)
> + __set_bit(FR_FORCE, &req->flags);
> } else {
> WARN_ON(args->nocreds);
> req = fuse_get_req(idmap, fm, false);
> diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
> index 7f16049387d1..23a241f18623 100644
> --- a/fs/fuse/fuse_i.h
> +++ b/fs/fuse/fuse_i.h
> @@ -345,6 +345,7 @@ struct fuse_args {
> bool is_ext:1;
> bool is_pinned:1;
> bool invalidate_vmap:1;
> + bool abort_on_kill:1;
> struct fuse_in_arg in_args[4];
> struct fuse_arg out_args[2];
> void (*end)(struct fuse_mount *fm, struct fuse_args *args, int error);
> diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
> index e57b8af06be9..84f78fb89d35 100644
> --- a/fs/fuse/inode.c
> +++ b/fs/fuse/inode.c
> @@ -1551,6 +1551,7 @@ int fuse_send_init(struct fuse_mount *fm)
> int err;
>
> if (fm->fc->sync_init) {
> + ia->args.abort_on_kill = true;
> err = fuse_simple_request(fm, &ia->args);
> /* Ignore size of init reply */
> if (err > 0)
I haven't looked at the other patches yet, so basically the mount can be
aborted with ctrl-c, but no self abort in this patch yet.
Maybe worth to add to the commit message?
Reviewed-by: Bernd Schubert <bernd@bsbernd.com>
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [PATCH v3 1/7] fuse: abort on fatal signal during sync init
2026-03-17 20:19 ` Bernd Schubert
@ 2026-03-18 9:33 ` Miklos Szeredi
2026-03-23 14:19 ` Bernd Schubert
0 siblings, 1 reply; 25+ messages in thread
From: Miklos Szeredi @ 2026-03-18 9:33 UTC (permalink / raw)
To: Bernd Schubert; +Cc: Miklos Szeredi, linux-fsdevel, stable
On Tue, 17 Mar 2026 at 21:24, Bernd Schubert <bernd@bsbernd.com> wrote:
>
>
>
> On 3/16/26 17:53, Miklos Szeredi wrote:
> > When sync init is used and the server exits for some reason (error, crash)
> > while processing FUSE_INIT, the filesystem creation will hang. The reason
> > is that while all other threads will exit, the mounting thread (or process)
> > will keep the device fd open, which will prevent an abort from happening.
> >
> > This is a regression from the async mount case, where the mount was done
> > first, and the FUSE_INIT processing afterwards, in which case there's no
> > such recursive syscall keeping the fd open.
> >
> > Fixes: dfb84c330794 ("fuse: allow synchronous FUSE_INIT")
> > Cc: stable@vger.kernel.org # v6.18
> > Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
> > ---
> > fs/fuse/dev.c | 6 +++++-
> > fs/fuse/fuse_i.h | 1 +
> > fs/fuse/inode.c | 1 +
> > 3 files changed, 7 insertions(+), 1 deletion(-)
> >
> > diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c
> > index 2c16b94357d5..f0631c48abef 100644
> > --- a/fs/fuse/dev.c
> > +++ b/fs/fuse/dev.c
> > @@ -576,6 +576,9 @@ static void request_wait_answer(struct fuse_req *req)
> > removed = fuse_remove_pending_req(req, &fiq->lock);
> > if (removed)
> > return;
> > +
> > + if (req->args->abort_on_kill)
> > + fuse_abort_conn(fc);
> > }
> >
> > /*
> > @@ -676,7 +679,8 @@ ssize_t __fuse_simple_request(struct mnt_idmap *idmap,
> > fuse_force_creds(req);
> >
> > __set_bit(FR_WAITING, &req->flags);
> > - __set_bit(FR_FORCE, &req->flags);
> > + if (!args->abort_on_kill)
> > + __set_bit(FR_FORCE, &req->flags);
> > } else {
> > WARN_ON(args->nocreds);
> > req = fuse_get_req(idmap, fm, false);
> > diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
> > index 7f16049387d1..23a241f18623 100644
> > --- a/fs/fuse/fuse_i.h
> > +++ b/fs/fuse/fuse_i.h
> > @@ -345,6 +345,7 @@ struct fuse_args {
> > bool is_ext:1;
> > bool is_pinned:1;
> > bool invalidate_vmap:1;
> > + bool abort_on_kill:1;
> > struct fuse_in_arg in_args[4];
> > struct fuse_arg out_args[2];
> > void (*end)(struct fuse_mount *fm, struct fuse_args *args, int error);
> > diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
> > index e57b8af06be9..84f78fb89d35 100644
> > --- a/fs/fuse/inode.c
> > +++ b/fs/fuse/inode.c
> > @@ -1551,6 +1551,7 @@ int fuse_send_init(struct fuse_mount *fm)
> > int err;
> >
> > if (fm->fc->sync_init) {
> > + ia->args.abort_on_kill = true;
> > err = fuse_simple_request(fm, &ia->args);
> > /* Ignore size of init reply */
> > if (err > 0)
>
>
> I haven't looked at the other patches yet, so basically the mount can be
> aborted with ctrl-c, but no self abort in this patch yet.
This should work with all kinds of exit, since they ultimately
generate signals on all threads.
So the rest of the patchset are not even part of this fix, they are
just cleanups/improvements.
Thanks,
Miklos
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [PATCH v3 1/7] fuse: abort on fatal signal during sync init
2026-03-18 9:33 ` Miklos Szeredi
@ 2026-03-23 14:19 ` Bernd Schubert
0 siblings, 0 replies; 25+ messages in thread
From: Bernd Schubert @ 2026-03-23 14:19 UTC (permalink / raw)
To: Miklos Szeredi; +Cc: Miklos Szeredi, linux-fsdevel, stable
On 3/18/26 10:33, Miklos Szeredi wrote:
> On Tue, 17 Mar 2026 at 21:24, Bernd Schubert <bernd@bsbernd.com> wrote:
>>
>>
>>
>> On 3/16/26 17:53, Miklos Szeredi wrote:
>>> When sync init is used and the server exits for some reason (error, crash)
>>> while processing FUSE_INIT, the filesystem creation will hang. The reason
>>> is that while all other threads will exit, the mounting thread (or process)
>>> will keep the device fd open, which will prevent an abort from happening.
>>>
>>> This is a regression from the async mount case, where the mount was done
>>> first, and the FUSE_INIT processing afterwards, in which case there's no
>>> such recursive syscall keeping the fd open.
>>>
>>> Fixes: dfb84c330794 ("fuse: allow synchronous FUSE_INIT")
>>> Cc: stable@vger.kernel.org # v6.18
>>> Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
>>> ---
>>> fs/fuse/dev.c | 6 +++++-
>>> fs/fuse/fuse_i.h | 1 +
>>> fs/fuse/inode.c | 1 +
>>> 3 files changed, 7 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c
>>> index 2c16b94357d5..f0631c48abef 100644
>>> --- a/fs/fuse/dev.c
>>> +++ b/fs/fuse/dev.c
>>> @@ -576,6 +576,9 @@ static void request_wait_answer(struct fuse_req *req)
>>> removed = fuse_remove_pending_req(req, &fiq->lock);
>>> if (removed)
>>> return;
>>> +
>>> + if (req->args->abort_on_kill)
>>> + fuse_abort_conn(fc);
>>> }
>>>
>>> /*
>>> @@ -676,7 +679,8 @@ ssize_t __fuse_simple_request(struct mnt_idmap *idmap,
>>> fuse_force_creds(req);
>>>
>>> __set_bit(FR_WAITING, &req->flags);
>>> - __set_bit(FR_FORCE, &req->flags);
>>> + if (!args->abort_on_kill)
>>> + __set_bit(FR_FORCE, &req->flags);
>>> } else {
>>> WARN_ON(args->nocreds);
>>> req = fuse_get_req(idmap, fm, false);
>>> diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
>>> index 7f16049387d1..23a241f18623 100644
>>> --- a/fs/fuse/fuse_i.h
>>> +++ b/fs/fuse/fuse_i.h
>>> @@ -345,6 +345,7 @@ struct fuse_args {
>>> bool is_ext:1;
>>> bool is_pinned:1;
>>> bool invalidate_vmap:1;
>>> + bool abort_on_kill:1;
>>> struct fuse_in_arg in_args[4];
>>> struct fuse_arg out_args[2];
>>> void (*end)(struct fuse_mount *fm, struct fuse_args *args, int error);
>>> diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
>>> index e57b8af06be9..84f78fb89d35 100644
>>> --- a/fs/fuse/inode.c
>>> +++ b/fs/fuse/inode.c
>>> @@ -1551,6 +1551,7 @@ int fuse_send_init(struct fuse_mount *fm)
>>> int err;
>>>
>>> if (fm->fc->sync_init) {
>>> + ia->args.abort_on_kill = true;
>>> err = fuse_simple_request(fm, &ia->args);
>>> /* Ignore size of init reply */
>>> if (err > 0)
>>
>>
>> I haven't looked at the other patches yet, so basically the mount can be
>> aborted with ctrl-c, but no self abort in this patch yet.
>
> This should work with all kinds of exit, since they ultimately
> generate signals on all threads.
>
> So the rest of the patchset are not even part of this fix, they are
> just cleanups/improvements.
Ah right, my confusion was from my wrong thinking it would be the
"mount" process sending the init, but actually it is the daemon itself.
Thanks,
Bernd
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v3 2/7] fuse: create fuse_dev on /dev/fuse open instead of mount
2026-03-16 16:53 [PATCH v3 0/7] fuse: fix hang with sync init Miklos Szeredi
2026-03-16 16:53 ` [PATCH v3 1/7] fuse: abort on fatal signal during " Miklos Szeredi
@ 2026-03-16 16:53 ` Miklos Szeredi
2026-03-17 21:35 ` Bernd Schubert
2026-03-16 16:53 ` [PATCH v3 3/7] fuse: add refcount to fuse_dev Miklos Szeredi
` (4 subsequent siblings)
6 siblings, 1 reply; 25+ messages in thread
From: Miklos Szeredi @ 2026-03-16 16:53 UTC (permalink / raw)
To: linux-fsdevel; +Cc: Bernd Schubert, Darrick J. Wong
Allocate struct fuse_dev when opening the device. This means that unlike
before, ->private_data is always set to a valid pointer.
The use of USE_DEV_SYNC_INIT magic pointer for the private_data is now
replaced with a simple bool sync_init member.
If sync INIT is not set, I/O on the device returns error before mount.
Keep this behavior by checking for the ->fc member. If fud->fc is set, the
mount has succeeded. Testing this used READ_ONCE(file->private_data) and
smp_mb() to try and provide the necessary semantics. Switch this to
smp_store_release() and smp_load_acquire().
Setting fud->fc is protected by fuse_mutex, this is unchanged.
Will need this later so the /dev/fuse open file reference is not held
during FSCONFIG_CMD_CREATE.
Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>
---
fs/fuse/dev.c | 47 +++++++++++++++++---------------------------
fs/fuse/fuse_dev_i.h | 33 +++++++++++++++++++++++--------
fs/fuse/fuse_i.h | 6 +++---
fs/fuse/inode.c | 35 +++++++++++----------------------
fs/fuse/virtio_fs.c | 2 --
5 files changed, 57 insertions(+), 66 deletions(-)
diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c
index f0631c48abef..fe453634897b 100644
--- a/fs/fuse/dev.c
+++ b/fs/fuse/dev.c
@@ -1546,32 +1546,24 @@ static ssize_t fuse_dev_do_read(struct fuse_dev *fud, struct file *file,
static int fuse_dev_open(struct inode *inode, struct file *file)
{
- /*
- * The fuse device's file's private_data is used to hold
- * the fuse_conn(ection) when it is mounted, and is used to
- * keep track of whether the file has been mounted already.
- */
- file->private_data = NULL;
+ struct fuse_dev *fud = fuse_dev_alloc();
+
+ if (!fud)
+ return -ENOMEM;
+
+ file->private_data = fud;
return 0;
}
struct fuse_dev *fuse_get_dev(struct file *file)
{
- struct fuse_dev *fud = __fuse_get_dev(file);
+ struct fuse_dev *fud = fuse_file_to_fud(file);
int err;
- if (likely(fud))
- return fud;
-
- err = wait_event_interruptible(fuse_dev_waitq,
- READ_ONCE(file->private_data) != FUSE_DEV_SYNC_INIT);
+ err = wait_event_interruptible(fuse_dev_waitq, fuse_dev_fc_get(fud) != NULL);
if (err)
return ERR_PTR(err);
- fud = __fuse_get_dev(file);
- if (!fud)
- return ERR_PTR(-EPERM);
-
return fud;
}
@@ -2538,10 +2530,10 @@ void fuse_wait_aborted(struct fuse_conn *fc)
int fuse_dev_release(struct inode *inode, struct file *file)
{
- struct fuse_dev *fud = __fuse_get_dev(file);
+ struct fuse_dev *fud = fuse_file_to_fud(file);
+ struct fuse_conn *fc = fuse_dev_fc_get(fud);
- if (fud) {
- struct fuse_conn *fc = fud->fc;
+ if (fc) {
struct fuse_pqueue *fpq = &fud->pq;
LIST_HEAD(to_end);
unsigned int i;
@@ -2559,8 +2551,8 @@ int fuse_dev_release(struct inode *inode, struct file *file)
WARN_ON(fc->iq.fasync != NULL);
fuse_abort_conn(fc);
}
- fuse_dev_free(fud);
}
+ fuse_dev_free(fud);
return 0;
}
EXPORT_SYMBOL_GPL(fuse_dev_release);
@@ -2578,16 +2570,12 @@ static int fuse_dev_fasync(int fd, struct file *file, int on)
static int fuse_device_clone(struct fuse_conn *fc, struct file *new)
{
- struct fuse_dev *fud;
+ struct fuse_dev *new_fud = fuse_file_to_fud(new);
- if (__fuse_get_dev(new))
+ if (fuse_dev_fc_get(new_fud))
return -EINVAL;
- fud = fuse_dev_alloc_install(fc);
- if (!fud)
- return -ENOMEM;
-
- new->private_data = fud;
+ fuse_dev_install(new_fud, fc);
atomic_inc(&fc->dev_count);
return 0;
@@ -2661,10 +2649,11 @@ static long fuse_dev_ioctl_backing_close(struct file *file, __u32 __user *argp)
static long fuse_dev_ioctl_sync_init(struct file *file)
{
int err = -EINVAL;
+ struct fuse_dev *fud = fuse_file_to_fud(file);
mutex_lock(&fuse_mutex);
- if (!__fuse_get_dev(file)) {
- WRITE_ONCE(file->private_data, FUSE_DEV_SYNC_INIT);
+ if (!fuse_dev_fc_get(fud)) {
+ fud->sync_init = true;
err = 0;
}
mutex_unlock(&fuse_mutex);
diff --git a/fs/fuse/fuse_dev_i.h b/fs/fuse/fuse_dev_i.h
index 134bf44aff0d..522b2012cd1f 100644
--- a/fs/fuse/fuse_dev_i.h
+++ b/fs/fuse/fuse_dev_i.h
@@ -39,18 +39,35 @@ struct fuse_copy_state {
} ring;
};
-#define FUSE_DEV_SYNC_INIT ((struct fuse_dev *) 1)
-#define FUSE_DEV_PTR_MASK (~1UL)
+/*
+ * Lockless access is OK, because fud->fc is set once during mount and is valid
+ * until the file is released.
+ */
+static inline struct fuse_conn *fuse_dev_fc_get(struct fuse_dev *fud)
+{
+ /* Pairs with smp_store_release() in fuse_dev_fc_set() */
+ return smp_load_acquire(&fud->fc);
+}
+
+static inline void fuse_dev_fc_set(struct fuse_dev *fud, struct fuse_conn *fc)
+{
+ /* Pairs with smp_load_acquire() in fuse_dev_fc_get() */
+ smp_store_release(&fud->fc, fc);
+}
+
+static inline struct fuse_dev *fuse_file_to_fud(struct file *file)
+{
+ return file->private_data;
+}
static inline struct fuse_dev *__fuse_get_dev(struct file *file)
{
- /*
- * Lockless access is OK, because file->private data is set
- * once during mount and is valid until the file is released.
- */
- struct fuse_dev *fud = READ_ONCE(file->private_data);
+ struct fuse_dev *fud = fuse_file_to_fud(file);
+
+ if (!fuse_dev_fc_get(fud))
+ return NULL;
- return (typeof(fud)) ((unsigned long) fud & FUSE_DEV_PTR_MASK);
+ return fud;
}
struct fuse_dev *fuse_get_dev(struct file *file);
diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
index 23a241f18623..94b49384a2f7 100644
--- a/fs/fuse/fuse_i.h
+++ b/fs/fuse/fuse_i.h
@@ -577,6 +577,9 @@ struct fuse_pqueue {
* Fuse device instance
*/
struct fuse_dev {
+ /** Issue FUSE_INIT synchronously */
+ bool sync_init;
+
/** Fuse connection for this device */
struct fuse_conn *fc;
@@ -623,9 +626,6 @@ struct fuse_fs_context {
/* DAX device, may be NULL */
struct dax_device *dax_dev;
-
- /* fuse_dev pointer to fill in, should contain NULL on entry */
- void **fudptr;
};
struct fuse_sync_bucket {
diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
index 84f78fb89d35..c2d1184d5ba5 100644
--- a/fs/fuse/inode.c
+++ b/fs/fuse/inode.c
@@ -1638,7 +1638,7 @@ EXPORT_SYMBOL_GPL(fuse_dev_alloc);
void fuse_dev_install(struct fuse_dev *fud, struct fuse_conn *fc)
{
- fud->fc = fuse_conn_get(fc);
+ fuse_dev_fc_set(fud, fuse_conn_get(fc));
spin_lock(&fc->lock);
list_add_tail(&fud->entry, &fc->devices);
spin_unlock(&fc->lock);
@@ -1660,7 +1660,7 @@ EXPORT_SYMBOL_GPL(fuse_dev_alloc_install);
void fuse_dev_free(struct fuse_dev *fud)
{
- struct fuse_conn *fc = fud->fc;
+ struct fuse_conn *fc = fuse_dev_fc_get(fud);
if (fc) {
spin_lock(&fc->lock);
@@ -1823,7 +1823,7 @@ EXPORT_SYMBOL_GPL(fuse_init_fs_context_submount);
int fuse_fill_super_common(struct super_block *sb, struct fuse_fs_context *ctx)
{
- struct fuse_dev *fud = NULL;
+ struct fuse_dev *fud = ctx->file ? fuse_file_to_fud(ctx->file) : NULL;
struct fuse_mount *fm = get_fuse_mount_super(sb);
struct fuse_conn *fc = fm->fc;
struct inode *root;
@@ -1857,18 +1857,11 @@ int fuse_fill_super_common(struct super_block *sb, struct fuse_fs_context *ctx)
goto err;
}
- if (ctx->fudptr) {
- err = -ENOMEM;
- fud = fuse_dev_alloc_install(fc);
- if (!fud)
- goto err_free_dax;
- }
-
fc->dev = sb->s_dev;
fm->sb = sb;
err = fuse_bdi_init(fc, sb);
if (err)
- goto err_dev_free;
+ goto err_free_dax;
/* Handle umasking inside the fuse code */
if (sb->s_flags & SB_POSIXACL)
@@ -1890,15 +1883,15 @@ int fuse_fill_super_common(struct super_block *sb, struct fuse_fs_context *ctx)
set_default_d_op(sb, &fuse_dentry_operations);
root_dentry = d_make_root(root);
if (!root_dentry)
- goto err_dev_free;
+ goto err_free_dax;
mutex_lock(&fuse_mutex);
err = -EINVAL;
- if (ctx->fudptr && *ctx->fudptr) {
- if (*ctx->fudptr == FUSE_DEV_SYNC_INIT)
- fc->sync_init = 1;
- else
+ if (fud) {
+ if (fuse_dev_fc_get(fud))
goto err_unlock;
+ if (fud->sync_init)
+ fc->sync_init = 1;
}
err = fuse_ctl_add_conn(fc);
@@ -1907,8 +1900,8 @@ int fuse_fill_super_common(struct super_block *sb, struct fuse_fs_context *ctx)
list_add_tail(&fc->entry, &fuse_conn_list);
sb->s_root = root_dentry;
- if (ctx->fudptr) {
- *ctx->fudptr = fud;
+ if (fud) {
+ fuse_dev_install(fud, fc);
wake_up_all(&fuse_dev_waitq);
}
mutex_unlock(&fuse_mutex);
@@ -1917,9 +1910,6 @@ int fuse_fill_super_common(struct super_block *sb, struct fuse_fs_context *ctx)
err_unlock:
mutex_unlock(&fuse_mutex);
dput(root_dentry);
- err_dev_free:
- if (fud)
- fuse_dev_free(fud);
err_free_dax:
if (IS_ENABLED(CONFIG_FUSE_DAX))
fuse_dax_conn_free(fc);
@@ -1945,13 +1935,10 @@ static int fuse_fill_super(struct super_block *sb, struct fs_context *fsc)
if ((ctx->file->f_op != &fuse_dev_operations) ||
(ctx->file->f_cred->user_ns != sb->s_user_ns))
return -EINVAL;
- ctx->fudptr = &ctx->file->private_data;
err = fuse_fill_super_common(sb, ctx);
if (err)
return err;
- /* file->private_data shall be visible on all CPUs after this */
- smp_mb();
fm = get_fuse_mount_super(sb);
diff --git a/fs/fuse/virtio_fs.c b/fs/fuse/virtio_fs.c
index 2f7485ffac52..f685916754ad 100644
--- a/fs/fuse/virtio_fs.c
+++ b/fs/fuse/virtio_fs.c
@@ -1590,8 +1590,6 @@ static int virtio_fs_fill_super(struct super_block *sb, struct fs_context *fsc)
goto err_free_fuse_devs;
}
- /* virtiofs allocates and installs its own fuse devices */
- ctx->fudptr = NULL;
if (ctx->dax_mode != FUSE_DAX_NEVER) {
if (ctx->dax_mode == FUSE_DAX_ALWAYS && !fs->dax_dev) {
err = -EINVAL;
--
2.53.0
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH v3 2/7] fuse: create fuse_dev on /dev/fuse open instead of mount
2026-03-16 16:53 ` [PATCH v3 2/7] fuse: create fuse_dev on /dev/fuse open instead of mount Miklos Szeredi
@ 2026-03-17 21:35 ` Bernd Schubert
2026-03-18 9:39 ` Miklos Szeredi
0 siblings, 1 reply; 25+ messages in thread
From: Bernd Schubert @ 2026-03-17 21:35 UTC (permalink / raw)
To: Miklos Szeredi, linux-fsdevel; +Cc: Darrick J. Wong
On 3/16/26 17:53, Miklos Szeredi wrote:
> Allocate struct fuse_dev when opening the device. This means that unlike
> before, ->private_data is always set to a valid pointer.
>
> The use of USE_DEV_SYNC_INIT magic pointer for the private_data is now
> replaced with a simple bool sync_init member.
>
> If sync INIT is not set, I/O on the device returns error before mount.
> Keep this behavior by checking for the ->fc member. If fud->fc is set, the
> mount has succeeded. Testing this used READ_ONCE(file->private_data) and
> smp_mb() to try and provide the necessary semantics. Switch this to
> smp_store_release() and smp_load_acquire().
>
> Setting fud->fc is protected by fuse_mutex, this is unchanged.
>
> Will need this later so the /dev/fuse open file reference is not held
> during FSCONFIG_CMD_CREATE.
>
> Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
> Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>
> ---
> fs/fuse/dev.c | 47 +++++++++++++++++---------------------------
> fs/fuse/fuse_dev_i.h | 33 +++++++++++++++++++++++--------
> fs/fuse/fuse_i.h | 6 +++---
> fs/fuse/inode.c | 35 +++++++++++----------------------
> fs/fuse/virtio_fs.c | 2 --
> 5 files changed, 57 insertions(+), 66 deletions(-)
>
> diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c
> index f0631c48abef..fe453634897b 100644
> --- a/fs/fuse/dev.c
> +++ b/fs/fuse/dev.c
> @@ -1546,32 +1546,24 @@ static ssize_t fuse_dev_do_read(struct fuse_dev *fud, struct file *file,
I have a question about fuse_dev_do_read(), is there any risk that the
compiler might re-order and put the
struct fuse_conn *fc = fud->fc;
before the "struct fuse_dev *fud = fuse_get_dev(file);" line in the
callers? In order to be absolutely sure, wouldn't it make sense to have
a fuse_get_dev_fc() function, that also returns the fc that was acquired
anyway and then add this to fuse_dev_do_read()?
The other parts are a nice simplification, thank you Miklos!
>
> static int fuse_dev_open(struct inode *inode, struct file *file)
> {
> - /*
> - * The fuse device's file's private_data is used to hold
> - * the fuse_conn(ection) when it is mounted, and is used to
> - * keep track of whether the file has been mounted already.
> - */
> - file->private_data = NULL;
> + struct fuse_dev *fud = fuse_dev_alloc();
> +
> + if (!fud)
> + return -ENOMEM;
> +
> + file->private_data = fud;
> return 0;
> }
>
> struct fuse_dev *fuse_get_dev(struct file *file)
> {
> - struct fuse_dev *fud = __fuse_get_dev(file);
> + struct fuse_dev *fud = fuse_file_to_fud(file);
> int err;
>
> - if (likely(fud))
> - return fud;
> -
> - err = wait_event_interruptible(fuse_dev_waitq,
> - READ_ONCE(file->private_data) != FUSE_DEV_SYNC_INIT);
> + err = wait_event_interruptible(fuse_dev_waitq, fuse_dev_fc_get(fud) != NULL);
> if (err)
> return ERR_PTR(err);
>
> - fud = __fuse_get_dev(file);
> - if (!fud)
> - return ERR_PTR(-EPERM);
> -
> return fud;
> }
>
> @@ -2538,10 +2530,10 @@ void fuse_wait_aborted(struct fuse_conn *fc)
>
> int fuse_dev_release(struct inode *inode, struct file *file)
> {
> - struct fuse_dev *fud = __fuse_get_dev(file);
> + struct fuse_dev *fud = fuse_file_to_fud(file);
> + struct fuse_conn *fc = fuse_dev_fc_get(fud);
>
> - if (fud) {
> - struct fuse_conn *fc = fud->fc;
> + if (fc) {
> struct fuse_pqueue *fpq = &fud->pq;
> LIST_HEAD(to_end);
> unsigned int i;
> @@ -2559,8 +2551,8 @@ int fuse_dev_release(struct inode *inode, struct file *file)
> WARN_ON(fc->iq.fasync != NULL);
> fuse_abort_conn(fc);
> }
> - fuse_dev_free(fud);
> }
> + fuse_dev_free(fud);
> return 0;
> }
> EXPORT_SYMBOL_GPL(fuse_dev_release);
> @@ -2578,16 +2570,12 @@ static int fuse_dev_fasync(int fd, struct file *file, int on)
>
> static int fuse_device_clone(struct fuse_conn *fc, struct file *new)
> {
> - struct fuse_dev *fud;
> + struct fuse_dev *new_fud = fuse_file_to_fud(new);
>
> - if (__fuse_get_dev(new))
> + if (fuse_dev_fc_get(new_fud))
> return -EINVAL;
>
> - fud = fuse_dev_alloc_install(fc);
> - if (!fud)
> - return -ENOMEM;
> -
> - new->private_data = fud;
> + fuse_dev_install(new_fud, fc);
> atomic_inc(&fc->dev_count);
>
> return 0;
> @@ -2661,10 +2649,11 @@ static long fuse_dev_ioctl_backing_close(struct file *file, __u32 __user *argp)
> static long fuse_dev_ioctl_sync_init(struct file *file)
> {
> int err = -EINVAL;
> + struct fuse_dev *fud = fuse_file_to_fud(file);
>
> mutex_lock(&fuse_mutex);
> - if (!__fuse_get_dev(file)) {
> - WRITE_ONCE(file->private_data, FUSE_DEV_SYNC_INIT);
> + if (!fuse_dev_fc_get(fud)) {
> + fud->sync_init = true;
> err = 0;
> }
> mutex_unlock(&fuse_mutex);
> diff --git a/fs/fuse/fuse_dev_i.h b/fs/fuse/fuse_dev_i.h
> index 134bf44aff0d..522b2012cd1f 100644
> --- a/fs/fuse/fuse_dev_i.h
> +++ b/fs/fuse/fuse_dev_i.h
> @@ -39,18 +39,35 @@ struct fuse_copy_state {
> } ring;
> };
>
> -#define FUSE_DEV_SYNC_INIT ((struct fuse_dev *) 1)
> -#define FUSE_DEV_PTR_MASK (~1UL)
> +/*
> + * Lockless access is OK, because fud->fc is set once during mount and is valid
> + * until the file is released.
> + */
> +static inline struct fuse_conn *fuse_dev_fc_get(struct fuse_dev *fud)
> +{
> + /* Pairs with smp_store_release() in fuse_dev_fc_set() */
> + return smp_load_acquire(&fud->fc);
> +}
> +
> +static inline void fuse_dev_fc_set(struct fuse_dev *fud, struct fuse_conn *fc)
> +{
> + /* Pairs with smp_load_acquire() in fuse_dev_fc_get() */
> + smp_store_release(&fud->fc, fc);
> +}
> +
> +static inline struct fuse_dev *fuse_file_to_fud(struct file *file)
> +{
> + return file->private_data;
> +}
>
> static inline struct fuse_dev *__fuse_get_dev(struct file *file)
> {
> - /*
> - * Lockless access is OK, because file->private data is set
> - * once during mount and is valid until the file is released.
> - */
> - struct fuse_dev *fud = READ_ONCE(file->private_data);
> + struct fuse_dev *fud = fuse_file_to_fud(file);
> +
> + if (!fuse_dev_fc_get(fud))
> + return NULL;
>
> - return (typeof(fud)) ((unsigned long) fud & FUSE_DEV_PTR_MASK);
> + return fud;
> }
>
> struct fuse_dev *fuse_get_dev(struct file *file);
> diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
> index 23a241f18623..94b49384a2f7 100644
> --- a/fs/fuse/fuse_i.h
> +++ b/fs/fuse/fuse_i.h
> @@ -577,6 +577,9 @@ struct fuse_pqueue {
> * Fuse device instance
> */
> struct fuse_dev {
> + /** Issue FUSE_INIT synchronously */
> + bool sync_init;
> +
> /** Fuse connection for this device */
> struct fuse_conn *fc;
>
> @@ -623,9 +626,6 @@ struct fuse_fs_context {
>
> /* DAX device, may be NULL */
> struct dax_device *dax_dev;
> -
> - /* fuse_dev pointer to fill in, should contain NULL on entry */
> - void **fudptr;
> };
>
> struct fuse_sync_bucket {
> diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
> index 84f78fb89d35..c2d1184d5ba5 100644
> --- a/fs/fuse/inode.c
> +++ b/fs/fuse/inode.c
> @@ -1638,7 +1638,7 @@ EXPORT_SYMBOL_GPL(fuse_dev_alloc);
>
> void fuse_dev_install(struct fuse_dev *fud, struct fuse_conn *fc)
> {
> - fud->fc = fuse_conn_get(fc);
> + fuse_dev_fc_set(fud, fuse_conn_get(fc));
> spin_lock(&fc->lock);
> list_add_tail(&fud->entry, &fc->devices);
> spin_unlock(&fc->lock);
> @@ -1660,7 +1660,7 @@ EXPORT_SYMBOL_GPL(fuse_dev_alloc_install);
>
> void fuse_dev_free(struct fuse_dev *fud)
> {
> - struct fuse_conn *fc = fud->fc;
> + struct fuse_conn *fc = fuse_dev_fc_get(fud);
>
> if (fc) {
> spin_lock(&fc->lock);
> @@ -1823,7 +1823,7 @@ EXPORT_SYMBOL_GPL(fuse_init_fs_context_submount);
>
> int fuse_fill_super_common(struct super_block *sb, struct fuse_fs_context *ctx)
> {
> - struct fuse_dev *fud = NULL;
> + struct fuse_dev *fud = ctx->file ? fuse_file_to_fud(ctx->file) : NULL;
> struct fuse_mount *fm = get_fuse_mount_super(sb);
> struct fuse_conn *fc = fm->fc;
> struct inode *root;
> @@ -1857,18 +1857,11 @@ int fuse_fill_super_common(struct super_block *sb, struct fuse_fs_context *ctx)
> goto err;
> }
>
> - if (ctx->fudptr) {
> - err = -ENOMEM;
> - fud = fuse_dev_alloc_install(fc);
> - if (!fud)
> - goto err_free_dax;
> - }
> -
> fc->dev = sb->s_dev;
> fm->sb = sb;
> err = fuse_bdi_init(fc, sb);
> if (err)
> - goto err_dev_free;
> + goto err_free_dax;
>
> /* Handle umasking inside the fuse code */
> if (sb->s_flags & SB_POSIXACL)
> @@ -1890,15 +1883,15 @@ int fuse_fill_super_common(struct super_block *sb, struct fuse_fs_context *ctx)
> set_default_d_op(sb, &fuse_dentry_operations);
> root_dentry = d_make_root(root);
> if (!root_dentry)
> - goto err_dev_free;
> + goto err_free_dax;
>
> mutex_lock(&fuse_mutex);
> err = -EINVAL;
> - if (ctx->fudptr && *ctx->fudptr) {
> - if (*ctx->fudptr == FUSE_DEV_SYNC_INIT)
> - fc->sync_init = 1;
> - else
> + if (fud) {
> + if (fuse_dev_fc_get(fud))
> goto err_unlock;
> + if (fud->sync_init)
> + fc->sync_init = 1;
> }
>
> err = fuse_ctl_add_conn(fc);
> @@ -1907,8 +1900,8 @@ int fuse_fill_super_common(struct super_block *sb, struct fuse_fs_context *ctx)
>
> list_add_tail(&fc->entry, &fuse_conn_list);
> sb->s_root = root_dentry;
> - if (ctx->fudptr) {
> - *ctx->fudptr = fud;
> + if (fud) {
> + fuse_dev_install(fud, fc);
> wake_up_all(&fuse_dev_waitq);
> }
> mutex_unlock(&fuse_mutex);
> @@ -1917,9 +1910,6 @@ int fuse_fill_super_common(struct super_block *sb, struct fuse_fs_context *ctx)
> err_unlock:
> mutex_unlock(&fuse_mutex);
> dput(root_dentry);
> - err_dev_free:
> - if (fud)
> - fuse_dev_free(fud);
> err_free_dax:
> if (IS_ENABLED(CONFIG_FUSE_DAX))
> fuse_dax_conn_free(fc);
> @@ -1945,13 +1935,10 @@ static int fuse_fill_super(struct super_block *sb, struct fs_context *fsc)
> if ((ctx->file->f_op != &fuse_dev_operations) ||
> (ctx->file->f_cred->user_ns != sb->s_user_ns))
> return -EINVAL;
> - ctx->fudptr = &ctx->file->private_data;
>
> err = fuse_fill_super_common(sb, ctx);
> if (err)
> return err;
> - /* file->private_data shall be visible on all CPUs after this */
> - smp_mb();
>
> fm = get_fuse_mount_super(sb);
>
> diff --git a/fs/fuse/virtio_fs.c b/fs/fuse/virtio_fs.c
> index 2f7485ffac52..f685916754ad 100644
> --- a/fs/fuse/virtio_fs.c
> +++ b/fs/fuse/virtio_fs.c
> @@ -1590,8 +1590,6 @@ static int virtio_fs_fill_super(struct super_block *sb, struct fs_context *fsc)
> goto err_free_fuse_devs;
> }
>
> - /* virtiofs allocates and installs its own fuse devices */
> - ctx->fudptr = NULL;
> if (ctx->dax_mode != FUSE_DAX_NEVER) {
> if (ctx->dax_mode == FUSE_DAX_ALWAYS && !fs->dax_dev) {
> err = -EINVAL;
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [PATCH v3 2/7] fuse: create fuse_dev on /dev/fuse open instead of mount
2026-03-17 21:35 ` Bernd Schubert
@ 2026-03-18 9:39 ` Miklos Szeredi
0 siblings, 0 replies; 25+ messages in thread
From: Miklos Szeredi @ 2026-03-18 9:39 UTC (permalink / raw)
To: Bernd Schubert; +Cc: Miklos Szeredi, linux-fsdevel, Darrick J. Wong
On Tue, 17 Mar 2026 at 22:38, Bernd Schubert <bernd@bsbernd.com> wrote:
>
>
>
> On 3/16/26 17:53, Miklos Szeredi wrote:
> > Allocate struct fuse_dev when opening the device. This means that unlike
> > before, ->private_data is always set to a valid pointer.
> >
> > The use of USE_DEV_SYNC_INIT magic pointer for the private_data is now
> > replaced with a simple bool sync_init member.
> >
> > If sync INIT is not set, I/O on the device returns error before mount.
> > Keep this behavior by checking for the ->fc member. If fud->fc is set, the
> > mount has succeeded. Testing this used READ_ONCE(file->private_data) and
> > smp_mb() to try and provide the necessary semantics. Switch this to
> > smp_store_release() and smp_load_acquire().
> >
> > Setting fud->fc is protected by fuse_mutex, this is unchanged.
> >
> > Will need this later so the /dev/fuse open file reference is not held
> > during FSCONFIG_CMD_CREATE.
> >
> > Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
> > Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>
> > ---
> > fs/fuse/dev.c | 47 +++++++++++++++++---------------------------
> > fs/fuse/fuse_dev_i.h | 33 +++++++++++++++++++++++--------
> > fs/fuse/fuse_i.h | 6 +++---
> > fs/fuse/inode.c | 35 +++++++++++----------------------
> > fs/fuse/virtio_fs.c | 2 --
> > 5 files changed, 57 insertions(+), 66 deletions(-)
> >
> > diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c
> > index f0631c48abef..fe453634897b 100644
> > --- a/fs/fuse/dev.c
> > +++ b/fs/fuse/dev.c
> > @@ -1546,32 +1546,24 @@ static ssize_t fuse_dev_do_read(struct fuse_dev *fud, struct file *file,
>
> I have a question about fuse_dev_do_read(), is there any risk that the
> compiler might re-order and put the
>
> struct fuse_conn *fc = fud->fc;
>
> before the "struct fuse_dev *fud = fuse_get_dev(file);" line in the
> callers? In order to be absolutely sure, wouldn't it make sense to have
> a fuse_get_dev_fc() function, that also returns the fc that was acquired
> anyway and then add this to fuse_dev_do_read()?
It does not matter. The sml_load_acquire() means no memory operations
can be reordered before it, see Documentation/memory-barriers.txt.
Thanks,
Miklos
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v3 3/7] fuse: add refcount to fuse_dev
2026-03-16 16:53 [PATCH v3 0/7] fuse: fix hang with sync init Miklos Szeredi
2026-03-16 16:53 ` [PATCH v3 1/7] fuse: abort on fatal signal during " Miklos Szeredi
2026-03-16 16:53 ` [PATCH v3 2/7] fuse: create fuse_dev on /dev/fuse open instead of mount Miklos Szeredi
@ 2026-03-16 16:53 ` Miklos Szeredi
2026-03-17 22:13 ` Bernd Schubert
2026-03-16 16:53 ` [PATCH v3 4/7] fuse: don't require /dev/fuse fd to be kept open during mount Miklos Szeredi
` (3 subsequent siblings)
6 siblings, 1 reply; 25+ messages in thread
From: Miklos Szeredi @ 2026-03-16 16:53 UTC (permalink / raw)
To: linux-fsdevel; +Cc: Bernd Schubert
This will make it possible to grab the fuse_dev and subsequently release
the file that it came from.
In the above case, fud->fc will be set to FUSE_DEV_FC_DISCONNECTED to
indicate that this is no longer a functional device.
When trying to assign an fc to such a disconnected fuse_dev, the fc is set
to the disconnected state.
Use atomic operations xchg() and cmpxchg() to prevent races.
Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
---
fs/fuse/cuse.c | 2 +-
fs/fuse/dev.c | 9 +++++++--
fs/fuse/fuse_dev_i.h | 15 ++++++++-------
fs/fuse/fuse_i.h | 7 +++++--
fs/fuse/inode.c | 44 ++++++++++++++++++++++++++++++++++++--------
fs/fuse/virtio_fs.c | 2 +-
6 files changed, 58 insertions(+), 21 deletions(-)
diff --git a/fs/fuse/cuse.c b/fs/fuse/cuse.c
index dfcb98a654d8..174333633471 100644
--- a/fs/fuse/cuse.c
+++ b/fs/fuse/cuse.c
@@ -527,7 +527,7 @@ static int cuse_channel_open(struct inode *inode, struct file *file)
cc->fc.initialized = 1;
rc = cuse_send_init(cc);
if (rc) {
- fuse_dev_free(fud);
+ fuse_dev_put(fud);
return rc;
}
file->private_data = fud;
diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c
index fe453634897b..4c926c9c4f39 100644
--- a/fs/fuse/dev.c
+++ b/fs/fuse/dev.c
@@ -2531,7 +2531,8 @@ void fuse_wait_aborted(struct fuse_conn *fc)
int fuse_dev_release(struct inode *inode, struct file *file)
{
struct fuse_dev *fud = fuse_file_to_fud(file);
- struct fuse_conn *fc = fuse_dev_fc_get(fud);
+ /* Pairs with cmpxchg() in fuse_dev_install() */
+ struct fuse_conn *fc = xchg(&fud->fc, FUSE_DEV_FC_DISCONNECTED);
if (fc) {
struct fuse_pqueue *fpq = &fud->pq;
@@ -2551,8 +2552,12 @@ int fuse_dev_release(struct inode *inode, struct file *file)
WARN_ON(fc->iq.fasync != NULL);
fuse_abort_conn(fc);
}
+ spin_lock(&fc->lock);
+ list_del(&fud->entry);
+ spin_unlock(&fc->lock);
+ fuse_conn_put(fc);
}
- fuse_dev_free(fud);
+ fuse_dev_put(fud);
return 0;
}
EXPORT_SYMBOL_GPL(fuse_dev_release);
diff --git a/fs/fuse/fuse_dev_i.h b/fs/fuse/fuse_dev_i.h
index 522b2012cd1f..910f883cd090 100644
--- a/fs/fuse/fuse_dev_i.h
+++ b/fs/fuse/fuse_dev_i.h
@@ -39,22 +39,23 @@ struct fuse_copy_state {
} ring;
};
+/* fud->fc gets assigned to this value when /dev/fuse is closed */
+#define FUSE_DEV_FC_DISCONNECTED ((struct fuse_conn *) 1)
+
/*
* Lockless access is OK, because fud->fc is set once during mount and is valid
* until the file is released.
+ *
+ * fud->fc is set to FUSE_DEV_FC_DISCONNECTED only after the containing file is
+ * released, so result is safe to dereference in most cases. Exceptions are:
+ * fuse_dev_put() and fuse_fill_super_common().
*/
static inline struct fuse_conn *fuse_dev_fc_get(struct fuse_dev *fud)
{
- /* Pairs with smp_store_release() in fuse_dev_fc_set() */
+ /* Pairs with xchg() in fuse_dev_install() */
return smp_load_acquire(&fud->fc);
}
-static inline void fuse_dev_fc_set(struct fuse_dev *fud, struct fuse_conn *fc)
-{
- /* Pairs with smp_load_acquire() in fuse_dev_fc_get() */
- smp_store_release(&fud->fc, fc);
-}
-
static inline struct fuse_dev *fuse_file_to_fud(struct file *file)
{
return file->private_data;
diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
index 94b49384a2f7..230201dc3f90 100644
--- a/fs/fuse/fuse_i.h
+++ b/fs/fuse/fuse_i.h
@@ -577,6 +577,9 @@ struct fuse_pqueue {
* Fuse device instance
*/
struct fuse_dev {
+ /** Reference count of this object */
+ refcount_t ref;
+
/** Issue FUSE_INIT synchronously */
bool sync_init;
@@ -1343,8 +1346,8 @@ void fuse_conn_put(struct fuse_conn *fc);
struct fuse_dev *fuse_dev_alloc_install(struct fuse_conn *fc);
struct fuse_dev *fuse_dev_alloc(void);
-void fuse_dev_install(struct fuse_dev *fud, struct fuse_conn *fc);
-void fuse_dev_free(struct fuse_dev *fud);
+bool fuse_dev_install(struct fuse_dev *fud, struct fuse_conn *fc);
+void fuse_dev_put(struct fuse_dev *fud);
int fuse_send_init(struct fuse_mount *fm);
/**
diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
index c2d1184d5ba5..0e848e3a13c2 100644
--- a/fs/fuse/inode.c
+++ b/fs/fuse/inode.c
@@ -1623,6 +1623,7 @@ struct fuse_dev *fuse_dev_alloc(void)
if (!fud)
return NULL;
+ refcount_set(&fud->ref, 1);
pq = kzalloc_objs(struct list_head, FUSE_PQ_HASH_SIZE);
if (!pq) {
kfree(fud);
@@ -1636,12 +1637,32 @@ struct fuse_dev *fuse_dev_alloc(void)
}
EXPORT_SYMBOL_GPL(fuse_dev_alloc);
-void fuse_dev_install(struct fuse_dev *fud, struct fuse_conn *fc)
+bool fuse_dev_install(struct fuse_dev *fud, struct fuse_conn *fc)
{
- fuse_dev_fc_set(fud, fuse_conn_get(fc));
+ struct fuse_conn *old_fc;
+
+ fuse_conn_get(fc);
spin_lock(&fc->lock);
+ /*
+ * Pairs with:
+ * - xchg() in fuse_dev_release()
+ * - smp_load_acquire() in fuse_dev_fc_get()
+ */
+ old_fc = cmpxchg(&fud->fc, NULL, fc);
+ if (old_fc) {
+ /*
+ * failed to set fud->fc because
+ * - it was already set to a different fc
+ * - it was set to disconneted
+ */
+ spin_unlock(&fc->lock);
+ fuse_conn_put(fc);
+ return false;
+ }
list_add_tail(&fud->entry, &fc->devices);
spin_unlock(&fc->lock);
+
+ return true;
}
EXPORT_SYMBOL_GPL(fuse_dev_install);
@@ -1658,11 +1679,16 @@ struct fuse_dev *fuse_dev_alloc_install(struct fuse_conn *fc)
}
EXPORT_SYMBOL_GPL(fuse_dev_alloc_install);
-void fuse_dev_free(struct fuse_dev *fud)
+void fuse_dev_put(struct fuse_dev *fud)
{
- struct fuse_conn *fc = fuse_dev_fc_get(fud);
+ struct fuse_conn *fc;
- if (fc) {
+ if (!refcount_dec_and_test(&fud->ref))
+ return;
+
+ fc = fuse_dev_fc_get(fud);
+ if (fc && fc != FUSE_DEV_FC_DISCONNECTED) {
+ /* This is the virtiofs case (fuse_dev_release() not called) */
spin_lock(&fc->lock);
list_del(&fud->entry);
spin_unlock(&fc->lock);
@@ -1672,7 +1698,7 @@ void fuse_dev_free(struct fuse_dev *fud)
kfree(fud->pq.processing);
kfree(fud);
}
-EXPORT_SYMBOL_GPL(fuse_dev_free);
+EXPORT_SYMBOL_GPL(fuse_dev_put);
static void fuse_fill_attr_from_inode(struct fuse_attr *attr,
const struct fuse_inode *fi)
@@ -1901,8 +1927,10 @@ int fuse_fill_super_common(struct super_block *sb, struct fuse_fs_context *ctx)
list_add_tail(&fc->entry, &fuse_conn_list);
sb->s_root = root_dentry;
if (fud) {
- fuse_dev_install(fud, fc);
- wake_up_all(&fuse_dev_waitq);
+ if (!fuse_dev_install(fud, fc))
+ fc->connected = 0; /* device file got closed */
+ else
+ wake_up_all(&fuse_dev_waitq);
}
mutex_unlock(&fuse_mutex);
return 0;
diff --git a/fs/fuse/virtio_fs.c b/fs/fuse/virtio_fs.c
index f685916754ad..12300651a0f1 100644
--- a/fs/fuse/virtio_fs.c
+++ b/fs/fuse/virtio_fs.c
@@ -486,7 +486,7 @@ static void virtio_fs_free_devs(struct virtio_fs *fs)
if (!fsvq->fud)
continue;
- fuse_dev_free(fsvq->fud);
+ fuse_dev_put(fsvq->fud);
fsvq->fud = NULL;
}
}
--
2.53.0
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH v3 3/7] fuse: add refcount to fuse_dev
2026-03-16 16:53 ` [PATCH v3 3/7] fuse: add refcount to fuse_dev Miklos Szeredi
@ 2026-03-17 22:13 ` Bernd Schubert
2026-03-18 9:50 ` Miklos Szeredi
0 siblings, 1 reply; 25+ messages in thread
From: Bernd Schubert @ 2026-03-17 22:13 UTC (permalink / raw)
To: Miklos Szeredi, linux-fsdevel
On 3/16/26 17:53, Miklos Szeredi wrote:
> This will make it possible to grab the fuse_dev and subsequently release
> the file that it came from.
>
> In the above case, fud->fc will be set to FUSE_DEV_FC_DISCONNECTED to
> indicate that this is no longer a functional device.
>
> When trying to assign an fc to such a disconnected fuse_dev, the fc is set
> to the disconnected state.
>
> Use atomic operations xchg() and cmpxchg() to prevent races.
So the actual motivation is in fuse_fill_super_common() which checks if
the file descriptor got closed during the mount? I.e. one thread mounts
and another thread closes the fd for the mount?
>
> Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
> ---
> fs/fuse/cuse.c | 2 +-
> fs/fuse/dev.c | 9 +++++++--
> fs/fuse/fuse_dev_i.h | 15 ++++++++-------
> fs/fuse/fuse_i.h | 7 +++++--
> fs/fuse/inode.c | 44 ++++++++++++++++++++++++++++++++++++--------
> fs/fuse/virtio_fs.c | 2 +-
> 6 files changed, 58 insertions(+), 21 deletions(-)
>
> diff --git a/fs/fuse/cuse.c b/fs/fuse/cuse.c
> index dfcb98a654d8..174333633471 100644
> --- a/fs/fuse/cuse.c
> +++ b/fs/fuse/cuse.c
> @@ -527,7 +527,7 @@ static int cuse_channel_open(struct inode *inode, struct file *file)
> cc->fc.initialized = 1;
> rc = cuse_send_init(cc);
> if (rc) {
> - fuse_dev_free(fud);
> + fuse_dev_put(fud);
> return rc;
> }
> file->private_data = fud;
> diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c
> index fe453634897b..4c926c9c4f39 100644
> --- a/fs/fuse/dev.c
> +++ b/fs/fuse/dev.c
> @@ -2531,7 +2531,8 @@ void fuse_wait_aborted(struct fuse_conn *fc)
> int fuse_dev_release(struct inode *inode, struct file *file)
> {
> struct fuse_dev *fud = fuse_file_to_fud(file);
> - struct fuse_conn *fc = fuse_dev_fc_get(fud);
> + /* Pairs with cmpxchg() in fuse_dev_install() */
> + struct fuse_conn *fc = xchg(&fud->fc, FUSE_DEV_FC_DISCONNECTED);
>
> if (fc) {
> struct fuse_pqueue *fpq = &fud->pq;
> @@ -2551,8 +2552,12 @@ int fuse_dev_release(struct inode *inode, struct file *file)
> WARN_ON(fc->iq.fasync != NULL);
> fuse_abort_conn(fc);
> }
> + spin_lock(&fc->lock);
> + list_del(&fud->entry);
> + spin_unlock(&fc->lock);
> + fuse_conn_put(fc);
> }
> - fuse_dev_free(fud);
> + fuse_dev_put(fud);
> return 0;
> }
> EXPORT_SYMBOL_GPL(fuse_dev_release);
> diff --git a/fs/fuse/fuse_dev_i.h b/fs/fuse/fuse_dev_i.h
> index 522b2012cd1f..910f883cd090 100644
> --- a/fs/fuse/fuse_dev_i.h
> +++ b/fs/fuse/fuse_dev_i.h
> @@ -39,22 +39,23 @@ struct fuse_copy_state {
> } ring;
> };
>
> +/* fud->fc gets assigned to this value when /dev/fuse is closed */
> +#define FUSE_DEV_FC_DISCONNECTED ((struct fuse_conn *) 1)
Hmm, these things always give me a headache, because now all callers
need to check for FUSE_DEV_FC_DISCONNECTED? Main protection is then vfs,
which disallows device release when there are competing operations?
> +
> /*
> * Lockless access is OK, because fud->fc is set once during mount and is valid
> * until the file is released.
> + *
> + * fud->fc is set to FUSE_DEV_FC_DISCONNECTED only after the containing file is
> + * released, so result is safe to dereference in most cases. Exceptions are:
> + * fuse_dev_put() and fuse_fill_super_common().
> */
> static inline struct fuse_conn *fuse_dev_fc_get(struct fuse_dev *fud)
> {
> - /* Pairs with smp_store_release() in fuse_dev_fc_set() */
> + /* Pairs with xchg() in fuse_dev_install() */
> return smp_load_acquire(&fud->fc);
> }
Minor nit: Pairs with cmpxchg?
>
> -static inline void fuse_dev_fc_set(struct fuse_dev *fud, struct fuse_conn *fc)
> -{
> - /* Pairs with smp_load_acquire() in fuse_dev_fc_get() */
> - smp_store_release(&fud->fc, fc);
> -}
> -
> static inline struct fuse_dev *fuse_file_to_fud(struct file *file)
> {
> return file->private_data;
> diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
> index 94b49384a2f7..230201dc3f90 100644
> --- a/fs/fuse/fuse_i.h
> +++ b/fs/fuse/fuse_i.h
> @@ -577,6 +577,9 @@ struct fuse_pqueue {
> * Fuse device instance
> */
> struct fuse_dev {
> + /** Reference count of this object */
> + refcount_t ref;
> +
> /** Issue FUSE_INIT synchronously */
> bool sync_init;
>
> @@ -1343,8 +1346,8 @@ void fuse_conn_put(struct fuse_conn *fc);
>
> struct fuse_dev *fuse_dev_alloc_install(struct fuse_conn *fc);
> struct fuse_dev *fuse_dev_alloc(void);
> -void fuse_dev_install(struct fuse_dev *fud, struct fuse_conn *fc);
> -void fuse_dev_free(struct fuse_dev *fud);
> +bool fuse_dev_install(struct fuse_dev *fud, struct fuse_conn *fc);
> +void fuse_dev_put(struct fuse_dev *fud);
> int fuse_send_init(struct fuse_mount *fm);
>
> /**
> diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
> index c2d1184d5ba5..0e848e3a13c2 100644
> --- a/fs/fuse/inode.c
> +++ b/fs/fuse/inode.c
> @@ -1623,6 +1623,7 @@ struct fuse_dev *fuse_dev_alloc(void)
> if (!fud)
> return NULL;
>
> + refcount_set(&fud->ref, 1);
> pq = kzalloc_objs(struct list_head, FUSE_PQ_HASH_SIZE);
> if (!pq) {
> kfree(fud);
> @@ -1636,12 +1637,32 @@ struct fuse_dev *fuse_dev_alloc(void)
> }
> EXPORT_SYMBOL_GPL(fuse_dev_alloc);
>
> -void fuse_dev_install(struct fuse_dev *fud, struct fuse_conn *fc)
> +bool fuse_dev_install(struct fuse_dev *fud, struct fuse_conn *fc)
> {
> - fuse_dev_fc_set(fud, fuse_conn_get(fc));
> + struct fuse_conn *old_fc;
> +
> + fuse_conn_get(fc);
> spin_lock(&fc->lock);
> + /*
> + * Pairs with:
> + * - xchg() in fuse_dev_release()
> + * - smp_load_acquire() in fuse_dev_fc_get()
> + */
> + old_fc = cmpxchg(&fud->fc, NULL, fc);
> + if (old_fc) {
> + /*
> + * failed to set fud->fc because
> + * - it was already set to a different fc
> + * - it was set to disconneted
> + */
> + spin_unlock(&fc->lock);
> + fuse_conn_put(fc);
> + return false;
> + }
> list_add_tail(&fud->entry, &fc->devices);
> spin_unlock(&fc->lock);
> +
> + return true;
> }
> EXPORT_SYMBOL_GPL(fuse_dev_install);
>
> @@ -1658,11 +1679,16 @@ struct fuse_dev *fuse_dev_alloc_install(struct fuse_conn *fc)
Shouldn't all callers, i.e. also fuse_dev_alloc_install() and
fuse_device_clone() check for success?
> }
> EXPORT_SYMBOL_GPL(fuse_dev_alloc_install);
>
> -void fuse_dev_free(struct fuse_dev *fud)
> +void fuse_dev_put(struct fuse_dev *fud)
> {
> - struct fuse_conn *fc = fuse_dev_fc_get(fud);
> + struct fuse_conn *fc;
>
> - if (fc) {
> + if (!refcount_dec_and_test(&fud->ref))
> + return;
> +
> + fc = fuse_dev_fc_get(fud);
> + if (fc && fc != FUSE_DEV_FC_DISCONNECTED) {
> + /* This is the virtiofs case (fuse_dev_release() not called) */
> spin_lock(&fc->lock);
> list_del(&fud->entry);
> spin_unlock(&fc->lock);
> @@ -1672,7 +1698,7 @@ void fuse_dev_free(struct fuse_dev *fud)
> kfree(fud->pq.processing);
> kfree(fud);
> }
> -EXPORT_SYMBOL_GPL(fuse_dev_free);
> +EXPORT_SYMBOL_GPL(fuse_dev_put);
>
> static void fuse_fill_attr_from_inode(struct fuse_attr *attr,
> const struct fuse_inode *fi)
> @@ -1901,8 +1927,10 @@ int fuse_fill_super_common(struct super_block *sb, struct fuse_fs_context *ctx)
> list_add_tail(&fc->entry, &fuse_conn_list);
> sb->s_root = root_dentry;
> if (fud) {
> - fuse_dev_install(fud, fc);
> - wake_up_all(&fuse_dev_waitq);
> + if (!fuse_dev_install(fud, fc))
> + fc->connected = 0; /* device file got closed */
> + else
> + wake_up_all(&fuse_dev_waitq);
> }
> mutex_unlock(&fuse_mutex);
> return 0;
> diff --git a/fs/fuse/virtio_fs.c b/fs/fuse/virtio_fs.c
> index f685916754ad..12300651a0f1 100644
> --- a/fs/fuse/virtio_fs.c
> +++ b/fs/fuse/virtio_fs.c
> @@ -486,7 +486,7 @@ static void virtio_fs_free_devs(struct virtio_fs *fs)
> if (!fsvq->fud)
> continue;
>
> - fuse_dev_free(fsvq->fud);
> + fuse_dev_put(fsvq->fud);
> fsvq->fud = NULL;
> }
> }
Thanks,
Bernd
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [PATCH v3 3/7] fuse: add refcount to fuse_dev
2026-03-17 22:13 ` Bernd Schubert
@ 2026-03-18 9:50 ` Miklos Szeredi
0 siblings, 0 replies; 25+ messages in thread
From: Miklos Szeredi @ 2026-03-18 9:50 UTC (permalink / raw)
To: Bernd Schubert; +Cc: Miklos Szeredi, linux-fsdevel
On Tue, 17 Mar 2026 at 23:16, Bernd Schubert <bernd@bsbernd.com> wrote:
>
>
>
> On 3/16/26 17:53, Miklos Szeredi wrote:
> > This will make it possible to grab the fuse_dev and subsequently release
> > the file that it came from.
> >
> > In the above case, fud->fc will be set to FUSE_DEV_FC_DISCONNECTED to
> > indicate that this is no longer a functional device.
> >
> > When trying to assign an fc to such a disconnected fuse_dev, the fc is set
> > to the disconnected state.
> >
> > Use atomic operations xchg() and cmpxchg() to prevent races.
>
> So the actual motivation is in fuse_fill_super_common() which checks if
> the file descriptor got closed during the mount? I.e. one thread mounts
> and another thread closes the fd for the mount?
Yes.
> > +/* fud->fc gets assigned to this value when /dev/fuse is closed */
> > +#define FUSE_DEV_FC_DISCONNECTED ((struct fuse_conn *) 1)
>
> Hmm, these things always give me a headache, because now all callers
> need to check for FUSE_DEV_FC_DISCONNECTED? Main protection is then vfs,
> which disallows device release when there are competing operations?
This value gets set on release, so all callers that hold a reference
to the device file can not see this value.
Only mount can, which will take a ref to struct fuse_dev and release
struct file. In that case if the file is closed, then fud->fc will be
set to
FUSE_DEV_FC_DISCONNECTED, mount will check if fud->fc is set anyway
(since it could have been set by another mount or FUSE_DEV_IOC_CLONE)
and will return an error in that case (either -EINVAL or -ENOTCONN
depending on when excatly the dev fd was closed).
So in fact the actual value need not be checked anywhere, the only
place it *is* checked is fuse_dev_put(), but we could probably rework
virtiofs so that it cleans up the fc before calling fuse_dev_put().
Thanks,
Miklos
>
>
> > +
> > /*
> > * Lockless access is OK, because fud->fc is set once during mount and is valid
> > * until the file is released.
> > + *
> > + * fud->fc is set to FUSE_DEV_FC_DISCONNECTED only after the containing file is
> > + * released, so result is safe to dereference in most cases. Exceptions are:
> > + * fuse_dev_put() and fuse_fill_super_common().
> > */
> > static inline struct fuse_conn *fuse_dev_fc_get(struct fuse_dev *fud)
> > {
> > - /* Pairs with smp_store_release() in fuse_dev_fc_set() */
> > + /* Pairs with xchg() in fuse_dev_install() */
> > return smp_load_acquire(&fud->fc);
> > }
>
> Minor nit: Pairs with cmpxchg?
>
> >
> > -static inline void fuse_dev_fc_set(struct fuse_dev *fud, struct fuse_conn *fc)
> > -{
> > - /* Pairs with smp_load_acquire() in fuse_dev_fc_get() */
> > - smp_store_release(&fud->fc, fc);
> > -}
> > -
> > static inline struct fuse_dev *fuse_file_to_fud(struct file *file)
> > {
> > return file->private_data;
> > diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
> > index 94b49384a2f7..230201dc3f90 100644
> > --- a/fs/fuse/fuse_i.h
> > +++ b/fs/fuse/fuse_i.h
> > @@ -577,6 +577,9 @@ struct fuse_pqueue {
> > * Fuse device instance
> > */
> > struct fuse_dev {
> > + /** Reference count of this object */
> > + refcount_t ref;
> > +
> > /** Issue FUSE_INIT synchronously */
> > bool sync_init;
> >
> > @@ -1343,8 +1346,8 @@ void fuse_conn_put(struct fuse_conn *fc);
> >
> > struct fuse_dev *fuse_dev_alloc_install(struct fuse_conn *fc);
> > struct fuse_dev *fuse_dev_alloc(void);
> > -void fuse_dev_install(struct fuse_dev *fud, struct fuse_conn *fc);
> > -void fuse_dev_free(struct fuse_dev *fud);
> > +bool fuse_dev_install(struct fuse_dev *fud, struct fuse_conn *fc);
> > +void fuse_dev_put(struct fuse_dev *fud);
> > int fuse_send_init(struct fuse_mount *fm);
> >
> > /**
> > diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
> > index c2d1184d5ba5..0e848e3a13c2 100644
> > --- a/fs/fuse/inode.c
> > +++ b/fs/fuse/inode.c
> > @@ -1623,6 +1623,7 @@ struct fuse_dev *fuse_dev_alloc(void)
> > if (!fud)
> > return NULL;
> >
> > + refcount_set(&fud->ref, 1);
> > pq = kzalloc_objs(struct list_head, FUSE_PQ_HASH_SIZE);
> > if (!pq) {
> > kfree(fud);
> > @@ -1636,12 +1637,32 @@ struct fuse_dev *fuse_dev_alloc(void)
> > }
> > EXPORT_SYMBOL_GPL(fuse_dev_alloc);
> >
> > -void fuse_dev_install(struct fuse_dev *fud, struct fuse_conn *fc)
> > +bool fuse_dev_install(struct fuse_dev *fud, struct fuse_conn *fc)
> > {
> > - fuse_dev_fc_set(fud, fuse_conn_get(fc));
> > + struct fuse_conn *old_fc;
> > +
> > + fuse_conn_get(fc);
> > spin_lock(&fc->lock);
> > + /*
> > + * Pairs with:
> > + * - xchg() in fuse_dev_release()
> > + * - smp_load_acquire() in fuse_dev_fc_get()
> > + */
> > + old_fc = cmpxchg(&fud->fc, NULL, fc);
> > + if (old_fc) {
> > + /*
> > + * failed to set fud->fc because
> > + * - it was already set to a different fc
> > + * - it was set to disconneted
> > + */
> > + spin_unlock(&fc->lock);
> > + fuse_conn_put(fc);
> > + return false;
> > + }
> > list_add_tail(&fud->entry, &fc->devices);
> > spin_unlock(&fc->lock);
> > +
> > + return true;
> > }
> > EXPORT_SYMBOL_GPL(fuse_dev_install);
> >
> > @@ -1658,11 +1679,16 @@ struct fuse_dev *fuse_dev_alloc_install(struct fuse_conn *fc)
>
> Shouldn't all callers, i.e. also fuse_dev_alloc_install() and
> fuse_device_clone() check for success?
>
>
> > }
> > EXPORT_SYMBOL_GPL(fuse_dev_alloc_install);
> >
> > -void fuse_dev_free(struct fuse_dev *fud)
> > +void fuse_dev_put(struct fuse_dev *fud)
> > {
> > - struct fuse_conn *fc = fuse_dev_fc_get(fud);
> > + struct fuse_conn *fc;
> >
> > - if (fc) {
> > + if (!refcount_dec_and_test(&fud->ref))
> > + return;
> > +
> > + fc = fuse_dev_fc_get(fud);
> > + if (fc && fc != FUSE_DEV_FC_DISCONNECTED) {
> > + /* This is the virtiofs case (fuse_dev_release() not called) */
> > spin_lock(&fc->lock);
> > list_del(&fud->entry);
> > spin_unlock(&fc->lock);
> > @@ -1672,7 +1698,7 @@ void fuse_dev_free(struct fuse_dev *fud)
> > kfree(fud->pq.processing);
> > kfree(fud);
> > }
> > -EXPORT_SYMBOL_GPL(fuse_dev_free);
> > +EXPORT_SYMBOL_GPL(fuse_dev_put);
> >
> > static void fuse_fill_attr_from_inode(struct fuse_attr *attr,
> > const struct fuse_inode *fi)
> > @@ -1901,8 +1927,10 @@ int fuse_fill_super_common(struct super_block *sb, struct fuse_fs_context *ctx)
> > list_add_tail(&fc->entry, &fuse_conn_list);
> > sb->s_root = root_dentry;
> > if (fud) {
> > - fuse_dev_install(fud, fc);
> > - wake_up_all(&fuse_dev_waitq);
> > + if (!fuse_dev_install(fud, fc))
> > + fc->connected = 0; /* device file got closed */
> > + else
> > + wake_up_all(&fuse_dev_waitq);
> > }
> > mutex_unlock(&fuse_mutex);
> > return 0;
> > diff --git a/fs/fuse/virtio_fs.c b/fs/fuse/virtio_fs.c
> > index f685916754ad..12300651a0f1 100644
> > --- a/fs/fuse/virtio_fs.c
> > +++ b/fs/fuse/virtio_fs.c
> > @@ -486,7 +486,7 @@ static void virtio_fs_free_devs(struct virtio_fs *fs)
> > if (!fsvq->fud)
> > continue;
> >
> > - fuse_dev_free(fsvq->fud);
> > + fuse_dev_put(fsvq->fud);
> > fsvq->fud = NULL;
> > }
> > }
>
> Thanks,
> Bernd
>
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v3 4/7] fuse: don't require /dev/fuse fd to be kept open during mount
2026-03-16 16:53 [PATCH v3 0/7] fuse: fix hang with sync init Miklos Szeredi
` (2 preceding siblings ...)
2026-03-16 16:53 ` [PATCH v3 3/7] fuse: add refcount to fuse_dev Miklos Szeredi
@ 2026-03-16 16:53 ` Miklos Szeredi
2026-03-16 19:56 ` Joanne Koong
2026-03-16 16:53 ` [PATCH v3 5/7] fuse: clean up device cloning Miklos Szeredi
` (2 subsequent siblings)
6 siblings, 1 reply; 25+ messages in thread
From: Miklos Szeredi @ 2026-03-16 16:53 UTC (permalink / raw)
To: linux-fsdevel; +Cc: Bernd Schubert
With the new mount API the sequence of syscalls would be:
fs_fd = fsopen("fuse", 0);
snprintf(opt, sizeof(opt), "%i", devfd);
fsconfig(fs_fd, FSCONFIG_SET_STRING, "fd", opt, 0);
/* ... */
fsconfig(fs_fd, FSCONFIG_CMD_CREATE, 0, 0, 0);
Current mount code just stores the value of devfd in the fs_context and
uses it in during FSCONFIG_CMD_CREATE, which is inelegant.
Instead grab a reference to the underlying fuse_dev, and use that during
the filesystem creation.
Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
---
fs/fuse/fuse_i.h | 4 +---
fs/fuse/inode.c | 54 +++++++++++++++++++++++++++---------------------
2 files changed, 31 insertions(+), 27 deletions(-)
diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
index 230201dc3f90..4a591a436324 100644
--- a/fs/fuse/fuse_i.h
+++ b/fs/fuse/fuse_i.h
@@ -606,13 +606,11 @@ static inline bool fuse_is_inode_dax_mode(enum fuse_dax_mode mode)
}
struct fuse_fs_context {
- int fd;
- struct file *file;
+ struct fuse_dev *fud;
unsigned int rootmode;
kuid_t user_id;
kgid_t group_id;
bool is_bdev:1;
- bool fd_present:1;
bool rootmode_present:1;
bool user_id_present:1;
bool group_id_present:1;
diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
index 0e848e3a13c2..314fb37f272f 100644
--- a/fs/fuse/inode.c
+++ b/fs/fuse/inode.c
@@ -800,6 +800,26 @@ static const struct fs_parameter_spec fuse_fs_parameters[] = {
{}
};
+static int fuse_opt_fd(struct fs_context *fsc, int fd)
+{
+ struct file *file __free(fput) = fget(fd);
+ struct fuse_fs_context *ctx = fsc->fs_private;
+
+ if (file->f_op != &fuse_dev_operations)
+ return invalfc(fsc, "fd is not a fuse device");
+ /*
+ * Require mount to happen from the same user namespace which
+ * opened /dev/fuse to prevent potential attacks.
+ */
+ if (file->f_cred->user_ns != fsc->user_ns)
+ return invalfc(fsc, "wrong user namespace for fuse device");
+
+ ctx->fud = file->private_data;
+ refcount_inc(&ctx->fud->ref);
+
+ return 0;
+}
+
static int fuse_parse_param(struct fs_context *fsc, struct fs_parameter *param)
{
struct fs_parse_result result;
@@ -839,9 +859,7 @@ static int fuse_parse_param(struct fs_context *fsc, struct fs_parameter *param)
return 0;
case OPT_FD:
- ctx->fd = result.uint_32;
- ctx->fd_present = true;
- break;
+ return fuse_opt_fd(fsc, result.uint_32);
case OPT_ROOTMODE:
if (!fuse_valid_type(result.uint_32))
@@ -904,6 +922,8 @@ static void fuse_free_fsc(struct fs_context *fsc)
struct fuse_fs_context *ctx = fsc->fs_private;
if (ctx) {
+ if (ctx->fud)
+ fuse_dev_put(ctx->fud);
kfree(ctx->subtype);
kfree(ctx);
}
@@ -1849,7 +1869,7 @@ EXPORT_SYMBOL_GPL(fuse_init_fs_context_submount);
int fuse_fill_super_common(struct super_block *sb, struct fuse_fs_context *ctx)
{
- struct fuse_dev *fud = ctx->file ? fuse_file_to_fud(ctx->file) : NULL;
+ struct fuse_dev *fud = ctx->fud;
struct fuse_mount *fm = get_fuse_mount_super(sb);
struct fuse_conn *fc = fm->fc;
struct inode *root;
@@ -1952,18 +1972,10 @@ static int fuse_fill_super(struct super_block *sb, struct fs_context *fsc)
struct fuse_mount *fm;
int err;
- if (!ctx->file || !ctx->rootmode_present ||
+ if (!ctx->fud || !ctx->rootmode_present ||
!ctx->user_id_present || !ctx->group_id_present)
return -EINVAL;
- /*
- * Require mount to happen from the same user namespace which
- * opened /dev/fuse to prevent potential attacks.
- */
- if ((ctx->file->f_op != &fuse_dev_operations) ||
- (ctx->file->f_cred->user_ns != sb->s_user_ns))
- return -EINVAL;
-
err = fuse_fill_super_common(sb, ctx);
if (err)
return err;
@@ -1991,8 +2003,7 @@ static int fuse_test_super(struct super_block *sb, struct fs_context *fsc)
static int fuse_get_tree(struct fs_context *fsc)
{
struct fuse_fs_context *ctx = fsc->fs_private;
- struct fuse_dev *fud;
- struct fuse_conn *fc;
+ struct fuse_conn *fc, *key;
struct fuse_mount *fm;
struct super_block *sb;
int err;
@@ -2012,9 +2023,6 @@ static int fuse_get_tree(struct fs_context *fsc)
fsc->s_fs_info = fm;
- if (ctx->fd_present)
- ctx->file = fget(ctx->fd);
-
if (IS_ENABLED(CONFIG_BLOCK) && ctx->is_bdev) {
err = get_tree_bdev(fsc, fuse_fill_super);
goto out;
@@ -2024,16 +2032,16 @@ static int fuse_get_tree(struct fs_context *fsc)
* (found by device name), normal fuse mounts can't
*/
err = -EINVAL;
- if (!ctx->file)
+ if (!ctx->fud)
goto out;
/*
* Allow creating a fuse mount with an already initialized fuse
* connection
*/
- fud = __fuse_get_dev(ctx->file);
- if (ctx->file->f_op == &fuse_dev_operations && fud) {
- fsc->sget_key = fud->fc;
+ key = fuse_dev_fc_get(ctx->fud);
+ if (key) {
+ fsc->sget_key = key;
sb = sget_fc(fsc, fuse_test_super, fuse_set_no_super);
err = PTR_ERR_OR_ZERO(sb);
if (!IS_ERR(sb))
@@ -2044,8 +2052,6 @@ static int fuse_get_tree(struct fs_context *fsc)
out:
if (fsc->s_fs_info)
fuse_mount_destroy(fm);
- if (ctx->file)
- fput(ctx->file);
return err;
}
--
2.53.0
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH v3 4/7] fuse: don't require /dev/fuse fd to be kept open during mount
2026-03-16 16:53 ` [PATCH v3 4/7] fuse: don't require /dev/fuse fd to be kept open during mount Miklos Szeredi
@ 2026-03-16 19:56 ` Joanne Koong
2026-03-17 9:35 ` Miklos Szeredi
0 siblings, 1 reply; 25+ messages in thread
From: Joanne Koong @ 2026-03-16 19:56 UTC (permalink / raw)
To: Miklos Szeredi; +Cc: linux-fsdevel, Bernd Schubert
On Mon, Mar 16, 2026 at 9:53 AM Miklos Szeredi <mszeredi@redhat.com> wrote:
>
> With the new mount API the sequence of syscalls would be:
>
> fs_fd = fsopen("fuse", 0);
> snprintf(opt, sizeof(opt), "%i", devfd);
> fsconfig(fs_fd, FSCONFIG_SET_STRING, "fd", opt, 0);
> /* ... */
> fsconfig(fs_fd, FSCONFIG_CMD_CREATE, 0, 0, 0);
>
> Current mount code just stores the value of devfd in the fs_context and
> uses it in during FSCONFIG_CMD_CREATE, which is inelegant.
>
> Instead grab a reference to the underlying fuse_dev, and use that during
> the filesystem creation.
>
> Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
> ---
> fs/fuse/fuse_i.h | 4 +---
> fs/fuse/inode.c | 54 +++++++++++++++++++++++++++---------------------
> 2 files changed, 31 insertions(+), 27 deletions(-)
>
> diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
> index 230201dc3f90..4a591a436324 100644
> --- a/fs/fuse/fuse_i.h
> +++ b/fs/fuse/fuse_i.h
> @@ -606,13 +606,11 @@ static inline bool fuse_is_inode_dax_mode(enum fuse_dax_mode mode)
> }
>
> struct fuse_fs_context {
> - int fd;
> - struct file *file;
> + struct fuse_dev *fud;
> unsigned int rootmode;
> kuid_t user_id;
> kgid_t group_id;
> bool is_bdev:1;
> - bool fd_present:1;
> bool rootmode_present:1;
> bool user_id_present:1;
> bool group_id_present:1;
> diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
> index 0e848e3a13c2..314fb37f272f 100644
> --- a/fs/fuse/inode.c
> +++ b/fs/fuse/inode.c
> @@ -800,6 +800,26 @@ static const struct fs_parameter_spec fuse_fs_parameters[] = {
> {}
> };
>
> +static int fuse_opt_fd(struct fs_context *fsc, int fd)
> +{
> + struct file *file __free(fput) = fget(fd);
> + struct fuse_fs_context *ctx = fsc->fs_private;
> +
I think we first need a "if (!file)" check here before dereferencing?
As I understand it, userspace can pass in any fd value.
Thanks,
Joanne
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [PATCH v3 4/7] fuse: don't require /dev/fuse fd to be kept open during mount
2026-03-16 19:56 ` Joanne Koong
@ 2026-03-17 9:35 ` Miklos Szeredi
0 siblings, 0 replies; 25+ messages in thread
From: Miklos Szeredi @ 2026-03-17 9:35 UTC (permalink / raw)
To: Joanne Koong; +Cc: Miklos Szeredi, linux-fsdevel, Bernd Schubert
On Mon, 16 Mar 2026 at 20:56, Joanne Koong <joannelkoong@gmail.com> wrote:
> > +static int fuse_opt_fd(struct fs_context *fsc, int fd)
> > +{
> > + struct file *file __free(fput) = fget(fd);
> > + struct fuse_fs_context *ctx = fsc->fs_private;
> > +
>
> I think we first need a "if (!file)" check here before dereferencing?
> As I understand it, userspace can pass in any fd value.
Well spotted.
Thanks,
Miklos
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v3 5/7] fuse: clean up device cloning
2026-03-16 16:53 [PATCH v3 0/7] fuse: fix hang with sync init Miklos Szeredi
` (3 preceding siblings ...)
2026-03-16 16:53 ` [PATCH v3 4/7] fuse: don't require /dev/fuse fd to be kept open during mount Miklos Szeredi
@ 2026-03-16 16:53 ` Miklos Szeredi
2026-03-17 22:51 ` Bernd Schubert
2026-03-16 16:53 ` [PATCH v3 6/7] fuse: alloc pqueue before installing fc Miklos Szeredi
2026-03-16 16:53 ` [PATCH v3 7/7] fuse: support FSCONFIG_SET_FD for "fd" option Miklos Szeredi
6 siblings, 1 reply; 25+ messages in thread
From: Miklos Szeredi @ 2026-03-16 16:53 UTC (permalink / raw)
To: linux-fsdevel; +Cc: Bernd Schubert
- fuse_mutex is not needed for device cloning, because fuse_dev_install()
uses cmpxcg() to set fud->fc, which prevents races between clone/mount
or clone/clone. This makes the logic simpler
- Drop fc->dev_count. This is only used to check in release if the device
is the last clone, but checking list_empty(&fc->devices) is equivalent
after removing the released device from the list. Removing the fuse_dev
before calling fuse_abort_conn() is okay, since the processing and io
lists are now empty for this device.
Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
---
fs/fuse/dev.c | 46 ++++++++++++++++++----------------------------
fs/fuse/fuse_i.h | 3 ---
fs/fuse/inode.c | 1 -
3 files changed, 18 insertions(+), 32 deletions(-)
diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c
index 4c926c9c4f39..3d6578fcb386 100644
--- a/fs/fuse/dev.c
+++ b/fs/fuse/dev.c
@@ -2538,6 +2538,7 @@ int fuse_dev_release(struct inode *inode, struct file *file)
struct fuse_pqueue *fpq = &fud->pq;
LIST_HEAD(to_end);
unsigned int i;
+ bool last;
spin_lock(&fpq->lock);
WARN_ON(!list_empty(&fpq->io));
@@ -2547,14 +2548,16 @@ int fuse_dev_release(struct inode *inode, struct file *file)
fuse_dev_end_requests(&to_end);
+ spin_lock(&fc->lock);
+ list_del(&fud->entry);
/* Are we the last open device? */
- if (atomic_dec_and_test(&fc->dev_count)) {
+ last = list_empty(&fc->devices);
+ spin_unlock(&fc->lock);
+
+ if (last) {
WARN_ON(fc->iq.fasync != NULL);
fuse_abort_conn(fc);
}
- spin_lock(&fc->lock);
- list_del(&fud->entry);
- spin_unlock(&fc->lock);
fuse_conn_put(fc);
}
fuse_dev_put(fud);
@@ -2573,24 +2576,10 @@ static int fuse_dev_fasync(int fd, struct file *file, int on)
return fasync_helper(fd, file, on, &fud->fc->iq.fasync);
}
-static int fuse_device_clone(struct fuse_conn *fc, struct file *new)
-{
- struct fuse_dev *new_fud = fuse_file_to_fud(new);
-
- if (fuse_dev_fc_get(new_fud))
- return -EINVAL;
-
- fuse_dev_install(new_fud, fc);
- atomic_inc(&fc->dev_count);
-
- return 0;
-}
-
static long fuse_dev_ioctl_clone(struct file *file, __u32 __user *argp)
{
- int res;
int oldfd;
- struct fuse_dev *fud = NULL;
+ struct fuse_dev *fud, *new_fud;
if (get_user(oldfd, argp))
return -EFAULT;
@@ -2603,17 +2592,18 @@ static long fuse_dev_ioctl_clone(struct file *file, __u32 __user *argp)
* Check against file->f_op because CUSE
* uses the same ioctl handler.
*/
- if (fd_file(f)->f_op == file->f_op)
- fud = __fuse_get_dev(fd_file(f));
+ if (fd_file(f)->f_op != file->f_op)
+ return -EINVAL;
- res = -EINVAL;
- if (fud) {
- mutex_lock(&fuse_mutex);
- res = fuse_device_clone(fud->fc, file);
- mutex_unlock(&fuse_mutex);
- }
+ fud = __fuse_get_dev(fd_file(f));
+ if (!fud)
+ return -EINVAL;
- return res;
+ new_fud = fuse_file_to_fud(file);
+ if (!fuse_dev_install(new_fud, fud->fc))
+ return -EINVAL;
+
+ return 0;
}
static long fuse_dev_ioctl_backing_open(struct file *file,
diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
index 4a591a436324..77f1c7cf24d2 100644
--- a/fs/fuse/fuse_i.h
+++ b/fs/fuse/fuse_i.h
@@ -650,9 +650,6 @@ struct fuse_conn {
/** Refcount */
refcount_t count;
- /** Number of fuse_dev's */
- atomic_t dev_count;
-
/** Current epoch for up-to-date dentries */
atomic_t epoch;
diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
index 314fb37f272f..7065614d02c9 100644
--- a/fs/fuse/inode.c
+++ b/fs/fuse/inode.c
@@ -995,7 +995,6 @@ void fuse_conn_init(struct fuse_conn *fc, struct fuse_mount *fm,
spin_lock_init(&fc->bg_lock);
init_rwsem(&fc->killsb);
refcount_set(&fc->count, 1);
- atomic_set(&fc->dev_count, 1);
atomic_set(&fc->epoch, 1);
INIT_WORK(&fc->epoch_work, fuse_epoch_work);
init_waitqueue_head(&fc->blocked_waitq);
--
2.53.0
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH v3 5/7] fuse: clean up device cloning
2026-03-16 16:53 ` [PATCH v3 5/7] fuse: clean up device cloning Miklos Szeredi
@ 2026-03-17 22:51 ` Bernd Schubert
2026-03-17 23:43 ` Joanne Koong
0 siblings, 1 reply; 25+ messages in thread
From: Bernd Schubert @ 2026-03-17 22:51 UTC (permalink / raw)
To: Miklos Szeredi, linux-fsdevel
On 3/16/26 17:53, Miklos Szeredi wrote:
> - fuse_mutex is not needed for device cloning, because fuse_dev_install()
> uses cmpxcg() to set fud->fc, which prevents races between clone/mount
> or clone/clone. This makes the logic simpler
>
> - Drop fc->dev_count. This is only used to check in release if the device
> is the last clone, but checking list_empty(&fc->devices) is equivalent
> after removing the released device from the list. Removing the fuse_dev
> before calling fuse_abort_conn() is okay, since the processing and io
> lists are now empty for this device.
>
> Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
> ---
> fs/fuse/dev.c | 46 ++++++++++++++++++----------------------------
> fs/fuse/fuse_i.h | 3 ---
> fs/fuse/inode.c | 1 -
> 3 files changed, 18 insertions(+), 32 deletions(-)
>
> diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c
> index 4c926c9c4f39..3d6578fcb386 100644
> --- a/fs/fuse/dev.c
> +++ b/fs/fuse/dev.c
> @@ -2538,6 +2538,7 @@ int fuse_dev_release(struct inode *inode, struct file *file)
> struct fuse_pqueue *fpq = &fud->pq;
> LIST_HEAD(to_end);
> unsigned int i;
> + bool last;
>
> spin_lock(&fpq->lock);
> WARN_ON(!list_empty(&fpq->io));
> @@ -2547,14 +2548,16 @@ int fuse_dev_release(struct inode *inode, struct file *file)
>
> fuse_dev_end_requests(&to_end);
>
> + spin_lock(&fc->lock);
> + list_del(&fud->entry);
> /* Are we the last open device? */
> - if (atomic_dec_and_test(&fc->dev_count)) {
> + last = list_empty(&fc->devices);
> + spin_unlock(&fc->lock);
> +
> + if (last) {
> WARN_ON(fc->iq.fasync != NULL);
> fuse_abort_conn(fc);
> }
> - spin_lock(&fc->lock);
> - list_del(&fud->entry);
> - spin_unlock(&fc->lock);
> fuse_conn_put(fc);
> }
> fuse_dev_put(fud);
> @@ -2573,24 +2576,10 @@ static int fuse_dev_fasync(int fd, struct file *file, int on)
> return fasync_helper(fd, file, on, &fud->fc->iq.fasync);
> }
>
> -static int fuse_device_clone(struct fuse_conn *fc, struct file *new)
> -{
> - struct fuse_dev *new_fud = fuse_file_to_fud(new);
> -
> - if (fuse_dev_fc_get(new_fud))
> - return -EINVAL;
> -
> - fuse_dev_install(new_fud, fc);
> - atomic_inc(&fc->dev_count);
> -
> - return 0;
> -}
> -
> static long fuse_dev_ioctl_clone(struct file *file, __u32 __user *argp)
> {
> - int res;
> int oldfd;
> - struct fuse_dev *fud = NULL;
> + struct fuse_dev *fud, *new_fud;
>
> if (get_user(oldfd, argp))
> return -EFAULT;
> @@ -2603,17 +2592,18 @@ static long fuse_dev_ioctl_clone(struct file *file, __u32 __user *argp)
> * Check against file->f_op because CUSE
> * uses the same ioctl handler.
> */
> - if (fd_file(f)->f_op == file->f_op)
> - fud = __fuse_get_dev(fd_file(f));
Let's say there is crazy daemon with
thread-A tries to mount
thread-B closes the fd during the mount
thread-C does clone in parallel
> + if (fd_file(f)->f_op != file->f_op)
> + return -EINVAL;
>
> - res = -EINVAL;
> - if (fud) {
> - mutex_lock(&fuse_mutex);
> - res = fuse_device_clone(fud->fc, file);
Won't this acceess an invalid fc?
> - mutex_unlock(&fuse_mutex);
> - }
> + fud = __fuse_get_dev(fd_file(f));
> + if (!fud)
> + return -EINVAL;
>
> - return res;
> + new_fud = fuse_file_to_fud(file);
> + if (!fuse_dev_install(new_fud, fud->fc))
> + return -EINVAL;
> +
> + return 0;
> }
>
> static long fuse_dev_ioctl_backing_open(struct file *file,
> diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
> index 4a591a436324..77f1c7cf24d2 100644
> --- a/fs/fuse/fuse_i.h
> +++ b/fs/fuse/fuse_i.h
> @@ -650,9 +650,6 @@ struct fuse_conn {
> /** Refcount */
> refcount_t count;
>
> - /** Number of fuse_dev's */
> - atomic_t dev_count;
> -
> /** Current epoch for up-to-date dentries */
> atomic_t epoch;
>
> diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
> index 314fb37f272f..7065614d02c9 100644
> --- a/fs/fuse/inode.c
> +++ b/fs/fuse/inode.c
> @@ -995,7 +995,6 @@ void fuse_conn_init(struct fuse_conn *fc, struct fuse_mount *fm,
> spin_lock_init(&fc->bg_lock);
> init_rwsem(&fc->killsb);
> refcount_set(&fc->count, 1);
> - atomic_set(&fc->dev_count, 1);
> atomic_set(&fc->epoch, 1);
> INIT_WORK(&fc->epoch_work, fuse_epoch_work);
> init_waitqueue_head(&fc->blocked_waitq);
Thanks,
Bernd
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [PATCH v3 5/7] fuse: clean up device cloning
2026-03-17 22:51 ` Bernd Schubert
@ 2026-03-17 23:43 ` Joanne Koong
0 siblings, 0 replies; 25+ messages in thread
From: Joanne Koong @ 2026-03-17 23:43 UTC (permalink / raw)
To: Bernd Schubert; +Cc: Miklos Szeredi, linux-fsdevel
On Tue, Mar 17, 2026 at 3:51 PM Bernd Schubert <bernd@bsbernd.com> wrote:
>
>
> On 3/16/26 17:53, Miklos Szeredi wrote:
> > - fuse_mutex is not needed for device cloning, because fuse_dev_install()
> > uses cmpxcg() to set fud->fc, which prevents races between clone/mount
> > or clone/clone. This makes the logic simpler
> >
> > - Drop fc->dev_count. This is only used to check in release if the device
> > is the last clone, but checking list_empty(&fc->devices) is equivalent
> > after removing the released device from the list. Removing the fuse_dev
> > before calling fuse_abort_conn() is okay, since the processing and io
> > lists are now empty for this device.
> >
> > Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
> > ---
> > fs/fuse/dev.c | 46 ++++++++++++++++++----------------------------
> > fs/fuse/fuse_i.h | 3 ---
> > fs/fuse/inode.c | 1 -
> > 3 files changed, 18 insertions(+), 32 deletions(-)
> >
> > diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c
> > index 4c926c9c4f39..3d6578fcb386 100644
> > --- a/fs/fuse/dev.c
> > +++ b/fs/fuse/dev.c
> > @@ -2538,6 +2538,7 @@ int fuse_dev_release(struct inode *inode, struct file *file)
> > struct fuse_pqueue *fpq = &fud->pq;
> > LIST_HEAD(to_end);
> > unsigned int i;
> > + bool last;
> >
> > spin_lock(&fpq->lock);
> > WARN_ON(!list_empty(&fpq->io));
> > @@ -2547,14 +2548,16 @@ int fuse_dev_release(struct inode *inode, struct file *file)
> >
> > fuse_dev_end_requests(&to_end);
> >
> > + spin_lock(&fc->lock);
> > + list_del(&fud->entry);
> > /* Are we the last open device? */
> > - if (atomic_dec_and_test(&fc->dev_count)) {
> > + last = list_empty(&fc->devices);
> > + spin_unlock(&fc->lock);
> > +
> > + if (last) {
> > WARN_ON(fc->iq.fasync != NULL);
> > fuse_abort_conn(fc);
> > }
> > - spin_lock(&fc->lock);
> > - list_del(&fud->entry);
> > - spin_unlock(&fc->lock);
> > fuse_conn_put(fc);
> > }
> > fuse_dev_put(fud);
> > @@ -2573,24 +2576,10 @@ static int fuse_dev_fasync(int fd, struct file *file, int on)
> > return fasync_helper(fd, file, on, &fud->fc->iq.fasync);
> > }
> >
> > -static int fuse_device_clone(struct fuse_conn *fc, struct file *new)
> > -{
> > - struct fuse_dev *new_fud = fuse_file_to_fud(new);
> > -
> > - if (fuse_dev_fc_get(new_fud))
> > - return -EINVAL;
> > -
> > - fuse_dev_install(new_fud, fc);
> > - atomic_inc(&fc->dev_count);
> > -
> > - return 0;
> > -}
> > -
> > static long fuse_dev_ioctl_clone(struct file *file, __u32 __user *argp)
> > {
> > - int res;
> > int oldfd;
> > - struct fuse_dev *fud = NULL;
> > + struct fuse_dev *fud, *new_fud;
> >
> > if (get_user(oldfd, argp))
> > return -EFAULT;
> > @@ -2603,17 +2592,18 @@ static long fuse_dev_ioctl_clone(struct file *file, __u32 __user *argp)
> > * Check against file->f_op because CUSE
> > * uses the same ioctl handler.
> > */
> > - if (fd_file(f)->f_op == file->f_op)
> > - fud = __fuse_get_dev(fd_file(f));
>
> Let's say there is crazy daemon with
>
> thread-A tries to mount
> thread-B closes the fd during the mount
> thread-C does clone in parallel
>
>
>
> > + if (fd_file(f)->f_op != file->f_op)
> > + return -EINVAL;
> >
> > - res = -EINVAL;
> > - if (fud) {
> > - mutex_lock(&fuse_mutex);
> > - res = fuse_device_clone(fud->fc, file);
>
> Won't this acceess an invalid fc?
>
if I'm understanding it correctly, the CLASS(fd, f)(oldfd); line a
few lines above does a fdget(oldfd) which holds a refcount on the
file, which means even after thread B closes the fd, the
fuse_dev_release() (and the fud->fc invalidation) doesn't get
triggered.
Thanks,
Joanne
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v3 6/7] fuse: alloc pqueue before installing fc
2026-03-16 16:53 [PATCH v3 0/7] fuse: fix hang with sync init Miklos Szeredi
` (4 preceding siblings ...)
2026-03-16 16:53 ` [PATCH v3 5/7] fuse: clean up device cloning Miklos Szeredi
@ 2026-03-16 16:53 ` Miklos Szeredi
2026-03-23 18:22 ` Darrick J. Wong
2026-03-16 16:53 ` [PATCH v3 7/7] fuse: support FSCONFIG_SET_FD for "fd" option Miklos Szeredi
6 siblings, 1 reply; 25+ messages in thread
From: Miklos Szeredi @ 2026-03-16 16:53 UTC (permalink / raw)
To: linux-fsdevel; +Cc: Bernd Schubert
Prior to this patchset, fuse_dev (containing fuse_pqueue) was allocated on
mount. But now fuse_dev is allocated when opening /dev/fuse, even though
the queues are not needed at that time.
Delay allocation of the pqueue (4k worth of list_head) just before mounting
or cloning a device.
Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
---
fs/fuse/dev.c | 9 ++++++--
fs/fuse/dev_uring.c | 2 +-
fs/fuse/fuse_i.h | 5 +++--
fs/fuse/inode.c | 51 +++++++++++++++++++++++++++++----------------
fs/fuse/virtio_fs.c | 4 ++--
5 files changed, 46 insertions(+), 25 deletions(-)
diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c
index 3d6578fcb386..d18796b1010c 100644
--- a/fs/fuse/dev.c
+++ b/fs/fuse/dev.c
@@ -1546,7 +1546,7 @@ static ssize_t fuse_dev_do_read(struct fuse_dev *fud, struct file *file,
static int fuse_dev_open(struct inode *inode, struct file *file)
{
- struct fuse_dev *fud = fuse_dev_alloc();
+ struct fuse_dev *fud = fuse_dev_alloc(false);
if (!fud)
return -ENOMEM;
@@ -2580,6 +2580,7 @@ static long fuse_dev_ioctl_clone(struct file *file, __u32 __user *argp)
{
int oldfd;
struct fuse_dev *fud, *new_fud;
+ struct list_head *pq;
if (get_user(oldfd, argp))
return -EFAULT;
@@ -2599,8 +2600,12 @@ static long fuse_dev_ioctl_clone(struct file *file, __u32 __user *argp)
if (!fud)
return -EINVAL;
+ pq = fuse_pqueue_alloc();
+ if (!pq)
+ return -ENOMEM;
+
new_fud = fuse_file_to_fud(file);
- if (!fuse_dev_install(new_fud, fud->fc))
+ if (!fuse_dev_install(new_fud, fud->fc, pq))
return -EINVAL;
return 0;
diff --git a/fs/fuse/dev_uring.c b/fs/fuse/dev_uring.c
index 7b9822e8837b..fb4f21c871fb 100644
--- a/fs/fuse/dev_uring.c
+++ b/fs/fuse/dev_uring.c
@@ -277,7 +277,7 @@ static struct fuse_ring_queue *fuse_uring_create_queue(struct fuse_ring *ring,
queue = kzalloc_obj(*queue, GFP_KERNEL_ACCOUNT);
if (!queue)
return NULL;
- pq = kzalloc_objs(struct list_head, FUSE_PQ_HASH_SIZE);
+ pq = fuse_pqueue_alloc();
if (!pq) {
kfree(queue);
return NULL;
diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
index 77f1c7cf24d2..e22d65e5ecff 100644
--- a/fs/fuse/fuse_i.h
+++ b/fs/fuse/fuse_i.h
@@ -1340,8 +1340,9 @@ void fuse_conn_init(struct fuse_conn *fc, struct fuse_mount *fm,
void fuse_conn_put(struct fuse_conn *fc);
struct fuse_dev *fuse_dev_alloc_install(struct fuse_conn *fc);
-struct fuse_dev *fuse_dev_alloc(void);
-bool fuse_dev_install(struct fuse_dev *fud, struct fuse_conn *fc);
+struct list_head *fuse_pqueue_alloc(void);
+struct fuse_dev *fuse_dev_alloc(bool alloc_pq);
+bool fuse_dev_install(struct fuse_dev *fud, struct fuse_conn *fc, struct list_head *pq);
void fuse_dev_put(struct fuse_dev *fud);
int fuse_send_init(struct fuse_mount *fm);
diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
index 7065614d02c9..f388d57fdd8f 100644
--- a/fs/fuse/inode.c
+++ b/fs/fuse/inode.c
@@ -977,15 +977,22 @@ static void fuse_iqueue_init(struct fuse_iqueue *fiq,
void fuse_pqueue_init(struct fuse_pqueue *fpq)
{
- unsigned int i;
-
spin_lock_init(&fpq->lock);
- for (i = 0; i < FUSE_PQ_HASH_SIZE; i++)
- INIT_LIST_HEAD(&fpq->processing[i]);
INIT_LIST_HEAD(&fpq->io);
fpq->connected = 1;
}
+struct list_head *fuse_pqueue_alloc(void)
+{
+ struct list_head *pq = kzalloc_objs(struct list_head, FUSE_PQ_HASH_SIZE);
+
+ if (pq) {
+ for (int i = 0; i < FUSE_PQ_HASH_SIZE; i++)
+ INIT_LIST_HEAD(&pq[i]);
+ }
+ return pq;
+}
+
void fuse_conn_init(struct fuse_conn *fc, struct fuse_mount *fm,
struct user_namespace *user_ns,
const struct fuse_iqueue_ops *fiq_ops, void *fiq_priv)
@@ -1633,33 +1640,38 @@ static int fuse_bdi_init(struct fuse_conn *fc, struct super_block *sb)
return 0;
}
-struct fuse_dev *fuse_dev_alloc(void)
+struct fuse_dev *fuse_dev_alloc(bool alloc_pq)
{
struct fuse_dev *fud;
- struct list_head *pq;
+ struct list_head *pq __free(kfree) = NULL;
+
+ if (alloc_pq) {
+ pq = fuse_pqueue_alloc();
+ if (!pq)
+ return NULL;
+ }
fud = kzalloc_obj(struct fuse_dev);
if (!fud)
return NULL;
refcount_set(&fud->ref, 1);
- pq = kzalloc_objs(struct list_head, FUSE_PQ_HASH_SIZE);
- if (!pq) {
- kfree(fud);
- return NULL;
- }
-
- fud->pq.processing = pq;
+ fud->pq.processing = no_free_ptr(pq);
fuse_pqueue_init(&fud->pq);
return fud;
}
EXPORT_SYMBOL_GPL(fuse_dev_alloc);
-bool fuse_dev_install(struct fuse_dev *fud, struct fuse_conn *fc)
+bool fuse_dev_install(struct fuse_dev *fud, struct fuse_conn *fc, struct list_head *pq)
{
struct fuse_conn *old_fc;
+ if (!pq)
+ WARN_ON(!fud->pq.processing);
+ else if (cmpxchg(&fud->pq.processing, NULL, pq))
+ kfree(pq);
+
fuse_conn_get(fc);
spin_lock(&fc->lock);
/*
@@ -1687,13 +1699,12 @@ EXPORT_SYMBOL_GPL(fuse_dev_install);
struct fuse_dev *fuse_dev_alloc_install(struct fuse_conn *fc)
{
- struct fuse_dev *fud;
+ struct fuse_dev *fud = fuse_dev_alloc(true);
- fud = fuse_dev_alloc();
if (!fud)
return NULL;
- fuse_dev_install(fud, fc);
+ fuse_dev_install(fud, fc, NULL);
return fud;
}
EXPORT_SYMBOL_GPL(fuse_dev_alloc_install);
@@ -1871,10 +1882,14 @@ int fuse_fill_super_common(struct super_block *sb, struct fuse_fs_context *ctx)
struct fuse_dev *fud = ctx->fud;
struct fuse_mount *fm = get_fuse_mount_super(sb);
struct fuse_conn *fc = fm->fc;
+ struct list_head *pq __free(kfree) = fuse_pqueue_alloc();
struct inode *root;
struct dentry *root_dentry;
int err;
+ if (!pq)
+ return -ENOMEM;
+
err = -EINVAL;
if (sb->s_flags & SB_MANDLOCK)
goto err;
@@ -1946,7 +1961,7 @@ int fuse_fill_super_common(struct super_block *sb, struct fuse_fs_context *ctx)
list_add_tail(&fc->entry, &fuse_conn_list);
sb->s_root = root_dentry;
if (fud) {
- if (!fuse_dev_install(fud, fc))
+ if (!fuse_dev_install(fud, fc, no_free_ptr(pq)))
fc->connected = 0; /* device file got closed */
else
wake_up_all(&fuse_dev_waitq);
diff --git a/fs/fuse/virtio_fs.c b/fs/fuse/virtio_fs.c
index 12300651a0f1..cc6426992ecd 100644
--- a/fs/fuse/virtio_fs.c
+++ b/fs/fuse/virtio_fs.c
@@ -1585,7 +1585,7 @@ static int virtio_fs_fill_super(struct super_block *sb, struct fs_context *fsc)
for (i = 0; i < fs->nvqs; i++) {
struct virtio_fs_vq *fsvq = &fs->vqs[i];
- fsvq->fud = fuse_dev_alloc();
+ fsvq->fud = fuse_dev_alloc(true);
if (!fsvq->fud)
goto err_free_fuse_devs;
}
@@ -1606,7 +1606,7 @@ static int virtio_fs_fill_super(struct super_block *sb, struct fs_context *fsc)
for (i = 0; i < fs->nvqs; i++) {
struct virtio_fs_vq *fsvq = &fs->vqs[i];
- fuse_dev_install(fsvq->fud, fc);
+ fuse_dev_install(fsvq->fud, fc, NULL);
}
/* Previous unmount will stop all queues. Start these again */
--
2.53.0
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH v3 6/7] fuse: alloc pqueue before installing fc
2026-03-16 16:53 ` [PATCH v3 6/7] fuse: alloc pqueue before installing fc Miklos Szeredi
@ 2026-03-23 18:22 ` Darrick J. Wong
2026-03-23 18:33 ` Bernd Schubert
0 siblings, 1 reply; 25+ messages in thread
From: Darrick J. Wong @ 2026-03-23 18:22 UTC (permalink / raw)
To: Miklos Szeredi; +Cc: linux-fsdevel, Bernd Schubert
On Mon, Mar 16, 2026 at 05:53:17PM +0100, Miklos Szeredi wrote:
> Prior to this patchset, fuse_dev (containing fuse_pqueue) was allocated on
> mount. But now fuse_dev is allocated when opening /dev/fuse, even though
> the queues are not needed at that time.
>
> Delay allocation of the pqueue (4k worth of list_head) just before mounting
> or cloning a device.
>
> Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
I wonder if this is worth the extra complexity to defer a memory
allocation? If the fuse server actually mount()s then we need the
pqueues, right? How common is it for a fuse server to open the
/dev/fuse when the kernel is so low on memory that the open() would fail
with ENOMEM on account of the pqueue allocation? And is it likely that
a lot of memory would be freed up by the time we get to mount/clone?
I suppose if you had a very slow mounting fuse server this could happen,
but now everyone has to understand that fud->pq.processing can be NULL.
--D
> ---
> fs/fuse/dev.c | 9 ++++++--
> fs/fuse/dev_uring.c | 2 +-
> fs/fuse/fuse_i.h | 5 +++--
> fs/fuse/inode.c | 51 +++++++++++++++++++++++++++++----------------
> fs/fuse/virtio_fs.c | 4 ++--
> 5 files changed, 46 insertions(+), 25 deletions(-)
>
> diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c
> index 3d6578fcb386..d18796b1010c 100644
> --- a/fs/fuse/dev.c
> +++ b/fs/fuse/dev.c
> @@ -1546,7 +1546,7 @@ static ssize_t fuse_dev_do_read(struct fuse_dev *fud, struct file *file,
>
> static int fuse_dev_open(struct inode *inode, struct file *file)
> {
> - struct fuse_dev *fud = fuse_dev_alloc();
> + struct fuse_dev *fud = fuse_dev_alloc(false);
>
> if (!fud)
> return -ENOMEM;
> @@ -2580,6 +2580,7 @@ static long fuse_dev_ioctl_clone(struct file *file, __u32 __user *argp)
> {
> int oldfd;
> struct fuse_dev *fud, *new_fud;
> + struct list_head *pq;
>
> if (get_user(oldfd, argp))
> return -EFAULT;
> @@ -2599,8 +2600,12 @@ static long fuse_dev_ioctl_clone(struct file *file, __u32 __user *argp)
> if (!fud)
> return -EINVAL;
>
> + pq = fuse_pqueue_alloc();
> + if (!pq)
> + return -ENOMEM;
> +
> new_fud = fuse_file_to_fud(file);
> - if (!fuse_dev_install(new_fud, fud->fc))
> + if (!fuse_dev_install(new_fud, fud->fc, pq))
> return -EINVAL;
>
> return 0;
> diff --git a/fs/fuse/dev_uring.c b/fs/fuse/dev_uring.c
> index 7b9822e8837b..fb4f21c871fb 100644
> --- a/fs/fuse/dev_uring.c
> +++ b/fs/fuse/dev_uring.c
> @@ -277,7 +277,7 @@ static struct fuse_ring_queue *fuse_uring_create_queue(struct fuse_ring *ring,
> queue = kzalloc_obj(*queue, GFP_KERNEL_ACCOUNT);
> if (!queue)
> return NULL;
> - pq = kzalloc_objs(struct list_head, FUSE_PQ_HASH_SIZE);
> + pq = fuse_pqueue_alloc();
> if (!pq) {
> kfree(queue);
> return NULL;
> diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
> index 77f1c7cf24d2..e22d65e5ecff 100644
> --- a/fs/fuse/fuse_i.h
> +++ b/fs/fuse/fuse_i.h
> @@ -1340,8 +1340,9 @@ void fuse_conn_init(struct fuse_conn *fc, struct fuse_mount *fm,
> void fuse_conn_put(struct fuse_conn *fc);
>
> struct fuse_dev *fuse_dev_alloc_install(struct fuse_conn *fc);
> -struct fuse_dev *fuse_dev_alloc(void);
> -bool fuse_dev_install(struct fuse_dev *fud, struct fuse_conn *fc);
> +struct list_head *fuse_pqueue_alloc(void);
> +struct fuse_dev *fuse_dev_alloc(bool alloc_pq);
> +bool fuse_dev_install(struct fuse_dev *fud, struct fuse_conn *fc, struct list_head *pq);
> void fuse_dev_put(struct fuse_dev *fud);
> int fuse_send_init(struct fuse_mount *fm);
>
> diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
> index 7065614d02c9..f388d57fdd8f 100644
> --- a/fs/fuse/inode.c
> +++ b/fs/fuse/inode.c
> @@ -977,15 +977,22 @@ static void fuse_iqueue_init(struct fuse_iqueue *fiq,
>
> void fuse_pqueue_init(struct fuse_pqueue *fpq)
> {
> - unsigned int i;
> -
> spin_lock_init(&fpq->lock);
> - for (i = 0; i < FUSE_PQ_HASH_SIZE; i++)
> - INIT_LIST_HEAD(&fpq->processing[i]);
> INIT_LIST_HEAD(&fpq->io);
> fpq->connected = 1;
> }
>
> +struct list_head *fuse_pqueue_alloc(void)
> +{
> + struct list_head *pq = kzalloc_objs(struct list_head, FUSE_PQ_HASH_SIZE);
> +
> + if (pq) {
> + for (int i = 0; i < FUSE_PQ_HASH_SIZE; i++)
> + INIT_LIST_HEAD(&pq[i]);
> + }
> + return pq;
> +}
> +
> void fuse_conn_init(struct fuse_conn *fc, struct fuse_mount *fm,
> struct user_namespace *user_ns,
> const struct fuse_iqueue_ops *fiq_ops, void *fiq_priv)
> @@ -1633,33 +1640,38 @@ static int fuse_bdi_init(struct fuse_conn *fc, struct super_block *sb)
> return 0;
> }
>
> -struct fuse_dev *fuse_dev_alloc(void)
> +struct fuse_dev *fuse_dev_alloc(bool alloc_pq)
> {
> struct fuse_dev *fud;
> - struct list_head *pq;
> + struct list_head *pq __free(kfree) = NULL;
> +
> + if (alloc_pq) {
> + pq = fuse_pqueue_alloc();
> + if (!pq)
> + return NULL;
> + }
>
> fud = kzalloc_obj(struct fuse_dev);
> if (!fud)
> return NULL;
>
> refcount_set(&fud->ref, 1);
> - pq = kzalloc_objs(struct list_head, FUSE_PQ_HASH_SIZE);
> - if (!pq) {
> - kfree(fud);
> - return NULL;
> - }
> -
> - fud->pq.processing = pq;
> + fud->pq.processing = no_free_ptr(pq);
> fuse_pqueue_init(&fud->pq);
>
> return fud;
> }
> EXPORT_SYMBOL_GPL(fuse_dev_alloc);
>
> -bool fuse_dev_install(struct fuse_dev *fud, struct fuse_conn *fc)
> +bool fuse_dev_install(struct fuse_dev *fud, struct fuse_conn *fc, struct list_head *pq)
> {
> struct fuse_conn *old_fc;
>
> + if (!pq)
> + WARN_ON(!fud->pq.processing);
> + else if (cmpxchg(&fud->pq.processing, NULL, pq))
> + kfree(pq);
> +
> fuse_conn_get(fc);
> spin_lock(&fc->lock);
> /*
> @@ -1687,13 +1699,12 @@ EXPORT_SYMBOL_GPL(fuse_dev_install);
>
> struct fuse_dev *fuse_dev_alloc_install(struct fuse_conn *fc)
> {
> - struct fuse_dev *fud;
> + struct fuse_dev *fud = fuse_dev_alloc(true);
>
> - fud = fuse_dev_alloc();
> if (!fud)
> return NULL;
>
> - fuse_dev_install(fud, fc);
> + fuse_dev_install(fud, fc, NULL);
> return fud;
> }
> EXPORT_SYMBOL_GPL(fuse_dev_alloc_install);
> @@ -1871,10 +1882,14 @@ int fuse_fill_super_common(struct super_block *sb, struct fuse_fs_context *ctx)
> struct fuse_dev *fud = ctx->fud;
> struct fuse_mount *fm = get_fuse_mount_super(sb);
> struct fuse_conn *fc = fm->fc;
> + struct list_head *pq __free(kfree) = fuse_pqueue_alloc();
> struct inode *root;
> struct dentry *root_dentry;
> int err;
>
> + if (!pq)
> + return -ENOMEM;
> +
> err = -EINVAL;
> if (sb->s_flags & SB_MANDLOCK)
> goto err;
> @@ -1946,7 +1961,7 @@ int fuse_fill_super_common(struct super_block *sb, struct fuse_fs_context *ctx)
> list_add_tail(&fc->entry, &fuse_conn_list);
> sb->s_root = root_dentry;
> if (fud) {
> - if (!fuse_dev_install(fud, fc))
> + if (!fuse_dev_install(fud, fc, no_free_ptr(pq)))
> fc->connected = 0; /* device file got closed */
> else
> wake_up_all(&fuse_dev_waitq);
> diff --git a/fs/fuse/virtio_fs.c b/fs/fuse/virtio_fs.c
> index 12300651a0f1..cc6426992ecd 100644
> --- a/fs/fuse/virtio_fs.c
> +++ b/fs/fuse/virtio_fs.c
> @@ -1585,7 +1585,7 @@ static int virtio_fs_fill_super(struct super_block *sb, struct fs_context *fsc)
> for (i = 0; i < fs->nvqs; i++) {
> struct virtio_fs_vq *fsvq = &fs->vqs[i];
>
> - fsvq->fud = fuse_dev_alloc();
> + fsvq->fud = fuse_dev_alloc(true);
> if (!fsvq->fud)
> goto err_free_fuse_devs;
> }
> @@ -1606,7 +1606,7 @@ static int virtio_fs_fill_super(struct super_block *sb, struct fs_context *fsc)
> for (i = 0; i < fs->nvqs; i++) {
> struct virtio_fs_vq *fsvq = &fs->vqs[i];
>
> - fuse_dev_install(fsvq->fud, fc);
> + fuse_dev_install(fsvq->fud, fc, NULL);
> }
>
> /* Previous unmount will stop all queues. Start these again */
> --
> 2.53.0
>
>
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [PATCH v3 6/7] fuse: alloc pqueue before installing fc
2026-03-23 18:22 ` Darrick J. Wong
@ 2026-03-23 18:33 ` Bernd Schubert
2026-03-23 18:45 ` Darrick J. Wong
0 siblings, 1 reply; 25+ messages in thread
From: Bernd Schubert @ 2026-03-23 18:33 UTC (permalink / raw)
To: Darrick J. Wong, Miklos Szeredi; +Cc: linux-fsdevel
On 3/23/26 19:22, Darrick J. Wong wrote:
> On Mon, Mar 16, 2026 at 05:53:17PM +0100, Miklos Szeredi wrote:
>> Prior to this patchset, fuse_dev (containing fuse_pqueue) was allocated on
>> mount. But now fuse_dev is allocated when opening /dev/fuse, even though
>> the queues are not needed at that time.
>>
>> Delay allocation of the pqueue (4k worth of list_head) just before mounting
>> or cloning a device.
>>
>> Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
>
> I wonder if this is worth the extra complexity to defer a memory
> allocation? If the fuse server actually mount()s then we need the
> pqueues, right? How common is it for a fuse server to open the
> /dev/fuse when the kernel is so low on memory that the open() would fail
> with ENOMEM on account of the pqueue allocation? And is it likely that
> a lot of memory would be freed up by the time we get to mount/clone?
>
> I suppose if you had a very slow mounting fuse server this could happen,
> but now everyone has to understand that fud->pq.processing can be NULL.
I guess the issue is that people might abuse it and open /dev/fuse for fun?
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v3 6/7] fuse: alloc pqueue before installing fc
2026-03-23 18:33 ` Bernd Schubert
@ 2026-03-23 18:45 ` Darrick J. Wong
0 siblings, 0 replies; 25+ messages in thread
From: Darrick J. Wong @ 2026-03-23 18:45 UTC (permalink / raw)
To: Bernd Schubert; +Cc: Miklos Szeredi, linux-fsdevel
On Mon, Mar 23, 2026 at 07:33:10PM +0100, Bernd Schubert wrote:
>
>
> On 3/23/26 19:22, Darrick J. Wong wrote:
> > On Mon, Mar 16, 2026 at 05:53:17PM +0100, Miklos Szeredi wrote:
> >> Prior to this patchset, fuse_dev (containing fuse_pqueue) was allocated on
> >> mount. But now fuse_dev is allocated when opening /dev/fuse, even though
> >> the queues are not needed at that time.
> >>
> >> Delay allocation of the pqueue (4k worth of list_head) just before mounting
> >> or cloning a device.
> >>
> >> Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
> >
> > I wonder if this is worth the extra complexity to defer a memory
> > allocation? If the fuse server actually mount()s then we need the
> > pqueues, right? How common is it for a fuse server to open the
> > /dev/fuse when the kernel is so low on memory that the open() would fail
> > with ENOMEM on account of the pqueue allocation? And is it likely that
> > a lot of memory would be freed up by the time we get to mount/clone?
> >
> > I suppose if you had a very slow mounting fuse server this could happen,
> > but now everyone has to understand that fud->pq.processing can be NULL.
>
> I guess the issue is that people might abuse it and open /dev/fuse for fun?
Ah, right, I forget that /dev/fuse is world-writable now. Yeah, we
shouldn't let any idiot pin a bunch of kernel memory. :)
I think the commit message should include that:
"Prior to this patchset, fuse_dev (containing fuse_pqueue) was allocated
on mount. But now fuse_dev is allocated when opening /dev/fuse, even
though the queues are not needed at that time.
"Delay allocation of the pqueue (4k worth of list_head) just before
mounting or cloning a device.
"Various distributions (e.g. Debian/Fedora) configure /dev/fuse as world
writable, so the pqueue allocation should be deferred to a privileged
operation (mount) to prevent unprivileged userspace from consuming
pinned kernel memory."
With that justification,
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>
--D
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v3 7/7] fuse: support FSCONFIG_SET_FD for "fd" option
2026-03-16 16:53 [PATCH v3 0/7] fuse: fix hang with sync init Miklos Szeredi
` (5 preceding siblings ...)
2026-03-16 16:53 ` [PATCH v3 6/7] fuse: alloc pqueue before installing fc Miklos Szeredi
@ 2026-03-16 16:53 ` Miklos Szeredi
2026-03-19 3:22 ` Lai, Yi
6 siblings, 1 reply; 25+ messages in thread
From: Miklos Szeredi @ 2026-03-16 16:53 UTC (permalink / raw)
To: linux-fsdevel; +Cc: Bernd Schubert, Darrick J. Wong
This is not only cleaner to use in userspace (no need to sprintf the fd to
a string) but also allows userspace to detect that the devfd can be closed
after the fsconfig call.
Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>
---
fs/fuse/inode.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
index f388d57fdd8f..e53153463e10 100644
--- a/fs/fuse/inode.c
+++ b/fs/fuse/inode.c
@@ -788,7 +788,7 @@ enum {
static const struct fs_parameter_spec fuse_fs_parameters[] = {
fsparam_string ("source", OPT_SOURCE),
- fsparam_u32 ("fd", OPT_FD),
+ fsparam_fd ("fd", OPT_FD),
fsparam_u32oct ("rootmode", OPT_ROOTMODE),
fsparam_uid ("user_id", OPT_USER_ID),
fsparam_gid ("group_id", OPT_GROUP_ID),
@@ -800,9 +800,8 @@ static const struct fs_parameter_spec fuse_fs_parameters[] = {
{}
};
-static int fuse_opt_fd(struct fs_context *fsc, int fd)
+static int fuse_opt_fd(struct fs_context *fsc, struct file *file)
{
- struct file *file __free(fput) = fget(fd);
struct fuse_fs_context *ctx = fsc->fs_private;
if (file->f_op != &fuse_dev_operations)
@@ -859,7 +858,12 @@ static int fuse_parse_param(struct fs_context *fsc, struct fs_parameter *param)
return 0;
case OPT_FD:
- return fuse_opt_fd(fsc, result.uint_32);
+ if (param->type == fs_value_is_file) {
+ return fuse_opt_fd(fsc, param->file);
+ } else {
+ struct file *file __free(fput) = fget(result.uint_32);
+ return fuse_opt_fd(fsc, file);
+ }
case OPT_ROOTMODE:
if (!fuse_valid_type(result.uint_32))
--
2.53.0
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH v3 7/7] fuse: support FSCONFIG_SET_FD for "fd" option
2026-03-16 16:53 ` [PATCH v3 7/7] fuse: support FSCONFIG_SET_FD for "fd" option Miklos Szeredi
@ 2026-03-19 3:22 ` Lai, Yi
0 siblings, 0 replies; 25+ messages in thread
From: Lai, Yi @ 2026-03-19 3:22 UTC (permalink / raw)
To: Miklos Szeredi; +Cc: linux-fsdevel, Bernd Schubert, Darrick J. Wong
On Mon, Mar 16, 2026 at 05:53:18PM +0100, Miklos Szeredi wrote:
> This is not only cleaner to use in userspace (no need to sprintf the fd to
> a string) but also allows userspace to detect that the devfd can be closed
> after the fsconfig call.
>
> Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
> Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>
> ---
> fs/fuse/inode.c | 12 ++++++++----
> 1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
> index f388d57fdd8f..e53153463e10 100644
> --- a/fs/fuse/inode.c
> +++ b/fs/fuse/inode.c
> @@ -788,7 +788,7 @@ enum {
>
> static const struct fs_parameter_spec fuse_fs_parameters[] = {
> fsparam_string ("source", OPT_SOURCE),
> - fsparam_u32 ("fd", OPT_FD),
> + fsparam_fd ("fd", OPT_FD),
> fsparam_u32oct ("rootmode", OPT_ROOTMODE),
> fsparam_uid ("user_id", OPT_USER_ID),
> fsparam_gid ("group_id", OPT_GROUP_ID),
> @@ -800,9 +800,8 @@ static const struct fs_parameter_spec fuse_fs_parameters[] = {
> {}
> };
>
> -static int fuse_opt_fd(struct fs_context *fsc, int fd)
> +static int fuse_opt_fd(struct fs_context *fsc, struct file *file)
> {
> - struct file *file __free(fput) = fget(fd);
> struct fuse_fs_context *ctx = fsc->fs_private;
>
> if (file->f_op != &fuse_dev_operations)
> @@ -859,7 +858,12 @@ static int fuse_parse_param(struct fs_context *fsc, struct fs_parameter *param)
> return 0;
>
> case OPT_FD:
> - return fuse_opt_fd(fsc, result.uint_32);
> + if (param->type == fs_value_is_file) {
> + return fuse_opt_fd(fsc, param->file);
> + } else {
> + struct file *file __free(fput) = fget(result.uint_32);
> + return fuse_opt_fd(fsc, file);
> + }
>
> case OPT_ROOTMODE:
> if (!fuse_valid_type(result.uint_32))
> --
> 2.53.0
>
Hi Miklos Szeredi,
Greetings!
I used Syzkaller and found that there is general protection fault in fuse_opt_fd in linux-next next-20260317.
After bisection and the first bad commit is:
"
6dcceeb72856 fuse: support FSCONFIG_SET_FD for "fd" option
"
All detailed into can be found at:
https://github.com/laifryiee/syzkaller_logs/tree/main/260319_020043_fuse_opt_fd
Syzkaller repro code:
https://github.com/laifryiee/syzkaller_logs/tree/main/260319_020043_fuse_opt_fd/repro.c
Syzkaller repro syscall steps:
https://github.com/laifryiee/syzkaller_logs/tree/main/260319_020043_fuse_opt_fd/repro.prog
Syzkaller report:
https://github.com/laifryiee/syzkaller_logs/tree/main/260319_020043_fuse_opt_fd/repro.report
Kconfig(make olddefconfig):
https://github.com/laifryiee/syzkaller_logs/tree/main/260319_020043_fuse_opt_fd/kconfig_origin
Bisect info:
https://github.com/laifryiee/syzkaller_logs/tree/main/260319_020043_fuse_opt_fd/bisect_info.log
bzImage:
https://github.com/laifryiee/syzkaller_logs/raw/refs/heads/main/260319_020043_fuse_opt_fd/bzImage_8e5a478b6d6a5bb0a3d52147862b15e4d826af19
Issue dmesg:
https://github.com/laifryiee/syzkaller_logs/blob/main/260319_020043_fuse_opt_fd/8e5a478b6d6a5bb0a3d52147862b15e4d826af19_dmesg.log
"
[ 19.543111] Oops: general protection fault, probably for non-canonical address 0xdffffc0000000009: 0000 [#1] SMP KASAN NOPTI
[ 19.543731] KASAN: null-ptr-deref in range [0x0000000000000048-0x000000000000004f]
[ 19.544132] CPU: 1 UID: 0 PID: 739 Comm: repro Not tainted 7.0.0-rc4-next-20260317-8e5a478b6d6a #1 PREEMPT(lazy)
[ 19.544657] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.16.0-0-gd239552ce722-prebuilt.qemu.org 04/01/2014
[ 19.545222] RIP: 0010:fuse_opt_fd+0x5e/0x340
[ 19.545481] Code: c1 ea 03 80 3c 02 00 0f 85 91 02 00 00 49 8d 7c 24 48 49 8b 9e 98 00 00 00 48 b8 00 00 00 00 00 fc ff df 48 89 fa 48 c1 ea 03 <80> 3c 02 00 0f 85 60 02 00 00 49 81 7c 24 48 20 cf 2e 86 0f 85 d8
[ 19.546420] RSP: 0018:ffff888012c879f8 EFLAGS: 00010206
[ 19.546703] RAX: dffffc0000000000 RBX: ffff88801fa83380 RCX: ffff888012c8796c
[ 19.547073] RDX: 0000000000000009 RSI: ffffffff82986cbf RDI: 0000000000000048
[ 19.547443] RBP: ffff888012c87a20 R08: 0000000000000000 R09: 0000000000000001
[ 19.547816] R10: 0000000000000001 R11: ffff8880141a8eb8 R12: 0000000000000000
[ 19.548185] R13: 0000000000000000 R14: ffff888010e3cc00 R15: ffff888012c87aa8
[ 19.548557] FS: 00007fbea1acf600(0000) GS:ffff8880e30aa000(0000) knlGS:0000000000000000
[ 19.548976] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 19.549279] CR2: 0000200000001000 CR3: 000000000ef6a006 CR4: 0000000000770ef0
[ 19.549650] PKRU: 55555554
[ 19.549805] Call Trace:
[ 19.549947] <TASK>
[ 19.550073] fuse_parse_param+0x810/0xcc0
[ 19.550296] ? __pfx_fuse_parse_param+0x10/0x10
[ 19.550545] ? __pfx___sanitizer_cov_trace_const_cmp2+0x10/0x10
[ 19.550876] ? static_key_count+0x69/0x80
[ 19.551107] ? __sanitizer_cov_trace_const_cmp8+0x1c/0x30
[ 19.551395] ? security_fs_context_parse_param+0x121/0x150
[ 19.551698] ? __pfx_fuse_parse_param+0x10/0x10
[ 19.551942] vfs_parse_fs_param+0x21e/0x3e0
[ 19.552188] vfs_parse_fs_qstr+0x15d/0x1e0
[ 19.552418] ? __pfx_vfs_parse_fs_qstr+0x10/0x10
[ 19.552676] ? kasan_save_track+0x18/0x40
[ 19.552909] ? kasan_save_alloc_info+0x3c/0x50
[ 19.553160] ? __pfx_vfs_parse_comma_sep+0x10/0x10
[ 19.553425] vfs_parse_monolithic_sep+0x1ab/0x230
[ 19.553687] ? __pfx_vfs_parse_monolithic_sep+0x10/0x10
[ 19.553975] ? fuse_init_fs_context+0x179/0x1f0
[ 19.554220] ? __sanitizer_cov_trace_const_cmp4+0x1a/0x20
[ 19.554512] ? __pfx_generic_parse_monolithic+0x10/0x10
[ 19.554801] generic_parse_monolithic+0x2e/0x40
[ 19.555052] parse_monolithic_mount_data+0x75/0xa0
[ 19.555319] path_mount+0x707/0x2060
[ 19.555526] ? lockdep_hardirqs_on+0x85/0x110
[ 19.555790] ? __pfx_path_mount+0x10/0x10
[ 19.556010] ? __kasan_slab_free+0x59/0x70
[ 19.556236] ? kmem_cache_free+0x251/0x5c0
[ 19.556463] ? putname+0xc6/0x130
[ 19.556655] ? putname+0xcb/0x130
[ 19.556845] __x64_sys_mount+0x2c3/0x340
[ 19.557060] ? __x64_sys_mount+0x2c3/0x340
[ 19.557286] ? __pfx___x64_sys_mount+0x10/0x10
[ 19.557530] ? __audit_syscall_entry+0x393/0x4f0
[ 19.557794] x64_sys_call+0x2b8/0x21c0
[ 19.558009] do_syscall_64+0xc1/0x1130
[ 19.558219] entry_SYSCALL_64_after_hwframe+0x76/0x7e
[ 19.558490] RIP: 0033:0x7fbea183ee5d
[ 19.558699] Code: ff c3 66 2e 0f 1f 84 00 00 00 00 00 90 f3 0f 1e fa 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 93 af 1b 00 f7 d8 64 89 01 48
[ 19.559633] RSP: 002b:00007ffd9393a858 EFLAGS: 00000202 ORIG_RAX: 00000000000000a5
[ 19.560035] RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 00007fbea183ee5d
[ 19.560405] RDX: 0000200000002100 RSI: 00002000000020c0 RDI: 0000000000000000
[ 19.560777] RBP: 00007ffd9393a870 R08: 0000200000000080 R09: 0000000000000800
[ 19.561155] R10: 0000000000000000 R11: 0000000000000202 R12: 00007ffd9393a988
[ 19.561526] R13: 0000000000401156 R14: 0000000000403e08 R15: 00007fbea1b18000
[ 19.561910] </TASK>
[ 19.562038] Modules linked in:
[ 19.562271] ---[ end trace 0000000000000000 ]---
[ 19.562525] RIP: 0010:fuse_opt_fd+0x5e/0x340
[ 19.562762] Code: c1 ea 03 80 3c 02 00 0f 85 91 02 00 00 49 8d 7c 24 48 49 8b 9e 98 00 00 00 48 b8 00 00 00 00 00 fc ff df 48 89 fa 48 c1 ea 03 <80> 3c 02 00 0f 85 60 02 00 00 49 81 7c 24 48 20 cf 2e 86 0f 85 d8
[ 19.563698] RSP: 0018:ffff888012c879f8 EFLAGS: 00010206
[ 19.563974] RAX: dffffc0000000000 RBX: ffff88801fa83380 RCX: ffff888012c8796c
[ 19.564342] RDX: 0000000000000009 RSI: ffffffff82986cbf RDI: 0000000000000048
[ 19.564712] RBP: ffff888012c87a20 R08: 0000000000000000 R09: 0000000000000001
[ 19.565083] R10: 0000000000000001 R11: ffff8880141a8eb8 R12: 0000000000000000
[ 19.565457] R13: 0000000000000000 R14: ffff888010e3cc00 R15: ffff888012c87aa8
[ 19.565849] FS: 00007fbea1acf600(0000) GS:ffff8880e30aa000(0000) knlGS:0000000000000000
[ 19.566266] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 19.566571] CR2: 0000200000001000 CR3: 000000000ef6a006 CR4: 0000000000770ef0
[ 19.566946] PKRU: 55555554
[ 19.571014] Oops: general protection fault, probably for non-canonical address 0xdffffc0000000009: 0000 [#2] SMP KASAN NOPTI
[ 19.571708] KASAN: null-ptr-deref in range [0x0000000000000048-0x000000000000004f]
[ 19.572103] CPU: 1 UID: 0 PID: 740 Comm: repro Tainted: G D 7.0.0-rc4-next-20260317-8e5a478b6d6a #1 PREEMPT(lazy)
[ 19.572706] Tainted: [D]=DIE
[ 19.572871] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.16.0-0-gd239552ce722-prebuilt.qemu.org 04/01/2014
[ 19.573447] RIP: 0010:fuse_opt_fd+0x5e/0x340
[ 19.573683] Code: c1 ea 03 80 3c 02 00 0f 85 91 02 00 00 49 8d 7c 24 48 49 8b 9e 98 00 00 00 48 b8 00 00 00 00 00 fc ff df 48 89 fa 48 c1 ea 03 <80> 3c 02 00 0f 85 60 02 00 00 49 81 7c 24 48 20 cf 2e 86 0f 85 d8
[ 19.574613] RSP: 0018:ffff888019a8f9f8 EFLAGS: 00010206
[ 19.574894] RAX: dffffc0000000000 RBX: ffff88801fa83c00 RCX: 1ffffffff0e3262d
[ 19.575264] RDX: 0000000000000009 RSI: ffffffff82986cbf RDI: 0000000000000048
[ 19.575635] RBP: ffff888019a8fa20 R08: 0000000000000000 R09: 0000000000000000
[ 19.576003] R10: 0000000000000000 R11: 000000000000000f R12: 0000000000000000
[ 19.576373] R13: 0000000000000000 R14: ffff888010e3ca00 R15: ffff888019a8faa8
[ 19.576746] FS: 00007f4b7fb2b600(0000) GS:ffff8880e30aa000(0000) knlGS:0000000000000000
[ 19.577161] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 19.577463] CR2: 0000200000001000 CR3: 0000000011acd003 CR4: 0000000000770ef0
[ 19.577841] PKRU: 55555554
[ 19.577992] Call Trace:
[ 19.578134] <TASK>
[ 19.578267] fuse_parse_param+0x810/0xcc0
[ 19.578488] ? __pfx_fuse_parse_param+0x10/0x10
[ 19.578737] ? __pfx___sanitizer_cov_trace_const_cmp2+0x10/0x10
[ 19.579055] ? static_key_count+0x69/0x80
[ 19.579278] ? __sanitizer_cov_trace_const_cmp8+0x1c/0x30
[ 19.579577] ? security_fs_context_parse_param+0x121/0x150
[ 19.579881] ? __pfx_fuse_parse_param+0x10/0x10
[ 19.580132] vfs_parse_fs_param+0x21e/0x3e0
[ 19.580368] vfs_parse_fs_qstr+0x15d/0x1e0
[ 19.580597] ? __pfx_vfs_parse_fs_qstr+0x10/0x10
[ 19.580861] ? kasan_save_track+0x18/0x40
[ 19.581087] ? kasan_save_alloc_info+0x3c/0x50
[ 19.581334] ? __pfx_vfs_parse_comma_sep+0x10/0x10
[ 19.581598] vfs_parse_monolithic_sep+0x1ab/0x230
[ 19.581862] ? __pfx_vfs_parse_monolithic_sep+0x10/0x10
[ 19.582151] ? fuse_init_fs_context+0x179/0x1f0
[ 19.582399] ? __sanitizer_cov_trace_const_cmp4+0x1a/0x20
[ 19.582693] ? __pfx_generic_parse_monolithic+0x10/0x10
[ 19.582978] generic_parse_monolithic+0x2e/0x40
[ 19.583235] parse_monolithic_mount_data+0x75/0xa0
[ 19.583504] path_mount+0x707/0x2060
[ 19.583709] ? __pfx_path_mount+0x10/0x10
[ 19.583931] ? __kasan_slab_free+0x59/0x70
[ 19.584159] ? kmem_cache_free+0x251/0x5c0
[ 19.584383] ? putname+0xc6/0x130
[ 19.584574] ? putname+0xcb/0x130
[ 19.584765] __x64_sys_mount+0x2c3/0x340
[ 19.584986] ? __x64_sys_mount+0x2c3/0x340
[ 19.585210] ? __pfx___x64_sys_mount+0x10/0x10
[ 19.585456] ? __audit_syscall_entry+0x393/0x4f0
[ 19.585717] x64_sys_call+0x2b8/0x21c0
[ 19.585928] do_syscall_64+0xc1/0x1130
[ 19.586138] entry_SYSCALL_64_after_hwframe+0x76/0x7e
[ 19.586407] RIP: 0033:0x7f4b7f83ee5d
[ 19.586613] Code: ff c3 66 2e 0f 1f 84 00 00 00 00 00 90 f3 0f 1e fa 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 93 af 1b 00 f7 d8 64 89 01 48
[ 19.587548] RSP: 002b:00007ffd113e09a8 EFLAGS: 00000202 ORIG_RAX: 00000000000000a5
[ 19.587946] RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 00007f4b7f83ee5d
[ 19.588319] RDX: 0000200000002100 RSI: 00002000000020c0 RDI: 0000000000000000
[ 19.588702] RBP: 00007ffd113e09c0 R08: 0000200000000080 R09: 0000000000000800
[ 19.589069] R10: 0000000000000000 R11: 0000000000000202 R12: 00007ffd113e0ad8
[ 19.589438] R13: 0000000000401156 R14: 0000000000403e08 R15: 00007f4b7fb74000
[ 19.589825] </TASK>
[ 19.589952] Modules linked in:
[ 19.590475] ---[ end trace 0000000000000000 ]---
"
Hope this cound be insightful to you.
Regards,
Yi Lai
---
If you don't need the following environment to reproduce the problem or if you
already have one reproduced environment, please ignore the following information.
How to reproduce:
git clone https://gitlab.com/xupengfe/repro_vm_env.git
cd repro_vm_env
tar -xvf repro_vm_env.tar.gz
cd repro_vm_env; ./start3.sh // it needs qemu-system-x86_64 and I used v7.1.0
// start3.sh will load bzImage_2241ab53cbb5cdb08a6b2d4688feb13971058f65 v6.2-rc5 kernel
// You could change the bzImage_xxx as you want
// Maybe you need to remove line "-drive if=pflash,format=raw,readonly=on,file=./OVMF_CODE.fd \" for different qemu version
You could use below command to log in, there is no password for root.
ssh -p 10023 root@localhost
After login vm(virtual machine) successfully, you could transfer reproduced
binary to the vm by below way, and reproduce the problem in vm:
gcc -pthread -o repro repro.c
scp -P 10023 repro root@localhost:/root/
Get the bzImage for target kernel:
Please use target kconfig and copy it to kernel_src/.config
make olddefconfig
make -jx bzImage //x should equal or less than cpu num your pc has
Fill the bzImage file into above start3.sh to load the target kernel in vm.
Tips:
If you already have qemu-system-x86_64, please ignore below info.
If you want to install qemu v7.1.0 version:
git clone https://github.com/qemu/qemu.git
cd qemu
git checkout -f v7.1.0
mkdir build
cd build
yum install -y ninja-build.x86_64
yum -y install libslirp-devel.x86_64
../configure --target-list=x86_64-softmmu --enable-kvm --enable-vnc --enable-gtk --enable-sdl --enable-usb-redir --enable-slirp
make
make install
^ permalink raw reply [flat|nested] 25+ messages in thread