* [PATCH v2 1/3] fuse: fix missing barrier when checking io-uring readiness [not found] <20260715174305.336261-1-joannelkoong@gmail.com> @ 2026-07-15 17:43 ` Joanne Koong 2026-07-15 22:26 ` Bernd Schubert 2026-07-15 17:43 ` [PATCH v2 3/3] fuse: publish io-uring queues with release semantics Joanne Koong 1 sibling, 1 reply; 6+ messages in thread From: Joanne Koong @ 2026-07-15 17:43 UTC (permalink / raw) To: miklos, bernd; +Cc: fuse-devel, stable fuse_block_alloc() reads fch->initialized and then fch->io_uring. fch->io_uring is set before fch->initialized, ordered by the smp_wmb() in fuse_chan_set_intialized(), but fuse_block_alloc() has no matching read barrier between the two loads. This may lead a CPU to observe fch->initialized=1 but fch->io_uring=0, and skip the check that blocks request allocation until the io-uring queues are ready. This can reintroduce the lock-order inversion deadlock that commit 3393ff964e0f prevents. Add an smp_rmb() barrier to pair with the smp_wmb() in fuse_chan_set_initialized() to prevent this. Fixes: 3393ff964e0f ("fuse: block request allocation until io-uring init is complete") Cc: stable@vger.kernel.org Signed-off-by: Joanne Koong <joannelkoong@gmail.com> --- fs/fuse/dev.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c index 5763a7cd3b37..b70c536d7e25 100644 --- a/fs/fuse/dev.c +++ b/fs/fuse/dev.c @@ -85,7 +85,13 @@ void fuse_chan_set_initialized(struct fuse_chan *fch, struct fuse_chan_param *pa static bool fuse_block_alloc(struct fuse_chan *fch, bool for_background) { - return !fch->initialized || (for_background && fch->blocked) || + if (!fch->initialized) + return true; + + /* Pairs with smp_wmb() in fuse_chan_set_initialized() */ + smp_rmb(); + + return (for_background && fch->blocked) || (fch->io_uring && fch->connected && !fuse_uring_ready(fch)); } -- 2.52.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2 1/3] fuse: fix missing barrier when checking io-uring readiness 2026-07-15 17:43 ` [PATCH v2 1/3] fuse: fix missing barrier when checking io-uring readiness Joanne Koong @ 2026-07-15 22:26 ` Bernd Schubert 2026-07-16 18:17 ` Joanne Koong 0 siblings, 1 reply; 6+ messages in thread From: Bernd Schubert @ 2026-07-15 22:26 UTC (permalink / raw) To: Joanne Koong, miklos; +Cc: fuse-devel, stable On 7/15/26 19:43, Joanne Koong wrote: > fuse_block_alloc() reads fch->initialized and then fch->io_uring. > fch->io_uring is set before fch->initialized, ordered by the smp_wmb() > in fuse_chan_set_intialized(), but fuse_block_alloc() has no matching > read barrier between the two loads. > > This may lead a CPU to observe fch->initialized=1 but fch->io_uring=0, > and skip the check that blocks request allocation until the io-uring > queues are ready. This can reintroduce the lock-order inversion deadlock > that commit 3393ff964e0f prevents. > > Add an smp_rmb() barrier to pair with the smp_wmb() in > fuse_chan_set_initialized() to prevent this. > > Fixes: 3393ff964e0f ("fuse: block request allocation until io-uring init is complete") > Cc: stable@vger.kernel.org > Signed-off-by: Joanne Koong <joannelkoong@gmail.com> > --- > fs/fuse/dev.c | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c > index 5763a7cd3b37..b70c536d7e25 100644 > --- a/fs/fuse/dev.c > +++ b/fs/fuse/dev.c > @@ -85,7 +85,13 @@ void fuse_chan_set_initialized(struct fuse_chan *fch, struct fuse_chan_param *pa > > static bool fuse_block_alloc(struct fuse_chan *fch, bool for_background) > { > - return !fch->initialized || (for_background && fch->blocked) || > + if (!fch->initialized) > + return true; > + > + /* Pairs with smp_wmb() in fuse_chan_set_initialized() */ > + smp_rmb(); > + > + return (for_background && fch->blocked) || > (fch->io_uring && fch->connected && !fuse_uring_ready(fch)); > } > I wonder if we could remove smp_rmb() in fuse_get_req(), it follows fuse_block_alloc(). Otherwise, Reviewed-by: Bernd Schubert <bernd@bsbernd.com> ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 1/3] fuse: fix missing barrier when checking io-uring readiness 2026-07-15 22:26 ` Bernd Schubert @ 2026-07-16 18:17 ` Joanne Koong 0 siblings, 0 replies; 6+ messages in thread From: Joanne Koong @ 2026-07-16 18:17 UTC (permalink / raw) To: Bernd Schubert; +Cc: miklos, fuse-devel, stable On Wed, Jul 15, 2026 at 3:26 PM Bernd Schubert <bernd@bsbernd.com> wrote: > > On 7/15/26 19:43, Joanne Koong wrote: > > fuse_block_alloc() reads fch->initialized and then fch->io_uring. > > fch->io_uring is set before fch->initialized, ordered by the smp_wmb() > > in fuse_chan_set_intialized(), but fuse_block_alloc() has no matching > > read barrier between the two loads. > > > > This may lead a CPU to observe fch->initialized=1 but fch->io_uring=0, > > and skip the check that blocks request allocation until the io-uring > > queues are ready. This can reintroduce the lock-order inversion deadlock > > that commit 3393ff964e0f prevents. > > > > Add an smp_rmb() barrier to pair with the smp_wmb() in > > fuse_chan_set_initialized() to prevent this. > > > > Fixes: 3393ff964e0f ("fuse: block request allocation until io-uring init is complete") > > Cc: stable@vger.kernel.org > > Signed-off-by: Joanne Koong <joannelkoong@gmail.com> > > --- > > fs/fuse/dev.c | 8 +++++++- > > 1 file changed, 7 insertions(+), 1 deletion(-) > > > > diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c > > index 5763a7cd3b37..b70c536d7e25 100644 > > --- a/fs/fuse/dev.c > > +++ b/fs/fuse/dev.c > > @@ -85,7 +85,13 @@ void fuse_chan_set_initialized(struct fuse_chan *fch, struct fuse_chan_param *pa > > > > static bool fuse_block_alloc(struct fuse_chan *fch, bool for_background) > > { > > - return !fch->initialized || (for_background && fch->blocked) || > > + if (!fch->initialized) > > + return true; > > + > > + /* Pairs with smp_wmb() in fuse_chan_set_initialized() */ > > + smp_rmb(); > > + > > + return (for_background && fch->blocked) || > > (fch->io_uring && fch->connected && !fuse_uring_ready(fch)); > > } > > > > I wonder if we could remove smp_rmb() in fuse_get_req(), it follows > fuse_block_alloc(). Nicely spotted. This is removed in the 2nd patch ("fuse: use release/acquire for fch->initialized") but I can move deleting that line into this patch. Thanks, Joanne ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2 3/3] fuse: publish io-uring queues with release semantics [not found] <20260715174305.336261-1-joannelkoong@gmail.com> 2026-07-15 17:43 ` [PATCH v2 1/3] fuse: fix missing barrier when checking io-uring readiness Joanne Koong @ 2026-07-15 17:43 ` Joanne Koong 2026-07-15 22:54 ` Bernd Schubert 1 sibling, 1 reply; 6+ messages in thread From: Joanne Koong @ 2026-07-15 17:43 UTC (permalink / raw) To: miklos, bernd; +Cc: fuse-devel, stable fuse_uring_create_queue() initializes a fuse_ring_queue and then publishes the pointer into ring->queues[qid] with WRITE_ONCE() under the fch->lock. There are several readers that may concurrently be fetching that pointer locklessly and then deferencing it. WRITE_ONCE() doesn't ensure ordering of the queue's field initialization before the ring->queues[qid] pointer assignment. The queue must be published with smp_store_release() so the field initialization is guaranteed to happen before. Readers in paths where the read may happen concurrently with the store need to use READ_ONCE() because any race involving a plain access is undefined. Fixes: 24fe962c86f5 ("fuse: {io-uring} Handle SQEs - register commands") Cc: stable@vger.kernel.org Signed-off-by: Joanne Koong <joannelkoong@gmail.com> --- fs/fuse/dev_uring.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/fs/fuse/dev_uring.c b/fs/fuse/dev_uring.c index 51f985154aa1..bf9d51f2a508 100644 --- a/fs/fuse/dev_uring.c +++ b/fs/fuse/dev_uring.c @@ -321,9 +321,11 @@ static struct fuse_ring_queue *fuse_uring_create_queue(struct fuse_ring *ring, } /* - * write_once and lock as the caller mostly doesn't take the lock at all + * fch->lock serializes concurrent creators for this qid. + * smp_store_release() are for the lockless readers who must see a + * fully initialized queue after &ring->queues[qid] is set */ - WRITE_ONCE(ring->queues[qid], queue); + smp_store_release(&ring->queues[qid], queue); spin_unlock(&fch->lock); return queue; @@ -434,7 +436,7 @@ static void fuse_uring_log_ent_state(struct fuse_ring *ring) struct fuse_ring_ent *ent; for (qid = 0; qid < ring->nr_queues; qid++) { - struct fuse_ring_queue *queue = ring->queues[qid]; + struct fuse_ring_queue *queue = READ_ONCE(ring->queues[qid]); if (!queue) continue; @@ -967,7 +969,7 @@ static int fuse_uring_commit_fetch(struct io_uring_cmd *cmd, int issue_flags, if (qid >= ring->nr_queues) return -EINVAL; - queue = ring->queues[qid]; + queue = READ_ONCE(ring->queues[qid]); if (!queue) return err; fpq = &queue->fpq; @@ -1035,7 +1037,7 @@ static bool is_ring_ready(struct fuse_ring *ring, int current_qid) if (current_qid == qid) continue; - queue = ring->queues[qid]; + queue = READ_ONCE(ring->queues[qid]); if (!queue) { ready = false; break; @@ -1191,7 +1193,7 @@ static int fuse_uring_register(struct io_uring_cmd *cmd, return -EINVAL; } - queue = ring->queues[qid]; + queue = READ_ONCE(ring->queues[qid]); if (!queue) { queue = fuse_uring_create_queue(ring, qid); if (!queue) -- 2.52.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2 3/3] fuse: publish io-uring queues with release semantics 2026-07-15 17:43 ` [PATCH v2 3/3] fuse: publish io-uring queues with release semantics Joanne Koong @ 2026-07-15 22:54 ` Bernd Schubert 2026-07-16 18:21 ` Joanne Koong 0 siblings, 1 reply; 6+ messages in thread From: Bernd Schubert @ 2026-07-15 22:54 UTC (permalink / raw) To: Joanne Koong, miklos; +Cc: fuse-devel, stable On 7/15/26 19:43, Joanne Koong wrote: > fuse_uring_create_queue() initializes a fuse_ring_queue and then > publishes the pointer into ring->queues[qid] with WRITE_ONCE() under the > fch->lock. There are several readers that may concurrently be fetching > that pointer locklessly and then deferencing it. > > WRITE_ONCE() doesn't ensure ordering of the queue's field > initialization before the ring->queues[qid] pointer assignment. The > queue must be published with smp_store_release() so the field > initialization is guaranteed to happen before. > > Readers in paths where the read may happen concurrently with the store > need to use READ_ONCE() because any race involving a plain access is > undefined. > > Fixes: 24fe962c86f5 ("fuse: {io-uring} Handle SQEs - register commands") > Cc: stable@vger.kernel.org > Signed-off-by: Joanne Koong <joannelkoong@gmail.com> > --- > fs/fuse/dev_uring.c | 14 ++++++++------ > 1 file changed, 8 insertions(+), 6 deletions(-) > > diff --git a/fs/fuse/dev_uring.c b/fs/fuse/dev_uring.c > index 51f985154aa1..bf9d51f2a508 100644 > --- a/fs/fuse/dev_uring.c > +++ b/fs/fuse/dev_uring.c > @@ -321,9 +321,11 @@ static struct fuse_ring_queue *fuse_uring_create_queue(struct fuse_ring *ring, > } > > /* > - * write_once and lock as the caller mostly doesn't take the lock at all > + * fch->lock serializes concurrent creators for this qid. > + * smp_store_release() are for the lockless readers who must see a > + * fully initialized queue after &ring->queues[qid] is set > */ > - WRITE_ONCE(ring->queues[qid], queue); > + smp_store_release(&ring->queues[qid], queue); > spin_unlock(&fch->lock); > > return queue; > @@ -434,7 +436,7 @@ static void fuse_uring_log_ent_state(struct fuse_ring *ring) > struct fuse_ring_ent *ent; > > for (qid = 0; qid < ring->nr_queues; qid++) { > - struct fuse_ring_queue *queue = ring->queues[qid]; > + struct fuse_ring_queue *queue = READ_ONCE(ring->queues[qid]); > > if (!queue) > continue; > @@ -967,7 +969,7 @@ static int fuse_uring_commit_fetch(struct io_uring_cmd *cmd, int issue_flags, > if (qid >= ring->nr_queues) > return -EINVAL; > > - queue = ring->queues[qid]; > + queue = READ_ONCE(ring->queues[qid]); > if (!queue) > return err; > fpq = &queue->fpq; > @@ -1035,7 +1037,7 @@ static bool is_ring_ready(struct fuse_ring *ring, int current_qid) > if (current_qid == qid) > continue; > > - queue = ring->queues[qid]; > + queue = READ_ONCE(ring->queues[qid]); > if (!queue) { > ready = false; > break; > @@ -1191,7 +1193,7 @@ static int fuse_uring_register(struct io_uring_cmd *cmd, > return -EINVAL; > } > > - queue = ring->queues[qid]; > + queue = READ_ONCE(ring->queues[qid]); > if (!queue) { > queue = fuse_uring_create_queue(ring, qid); > if (!queue) For consistency, maybe READ_ONCE/WRITE_ONCE in fuse_uring_destruct? Otherwise, Reviewed-by: Bernd Schubert <bernd@bsbernd.com> ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 3/3] fuse: publish io-uring queues with release semantics 2026-07-15 22:54 ` Bernd Schubert @ 2026-07-16 18:21 ` Joanne Koong 0 siblings, 0 replies; 6+ messages in thread From: Joanne Koong @ 2026-07-16 18:21 UTC (permalink / raw) To: Bernd Schubert; +Cc: miklos, fuse-devel, stable On Wed, Jul 15, 2026 at 3:54 PM Bernd Schubert <bernd@bsbernd.com> wrote: > > > > On 7/15/26 19:43, Joanne Koong wrote: > > fuse_uring_create_queue() initializes a fuse_ring_queue and then > > publishes the pointer into ring->queues[qid] with WRITE_ONCE() under the > > fch->lock. There are several readers that may concurrently be fetching > > that pointer locklessly and then deferencing it. > > > > WRITE_ONCE() doesn't ensure ordering of the queue's field > > initialization before the ring->queues[qid] pointer assignment. The > > queue must be published with smp_store_release() so the field > > initialization is guaranteed to happen before. > > > > Readers in paths where the read may happen concurrently with the store > > need to use READ_ONCE() because any race involving a plain access is > > undefined. > > > > Fixes: 24fe962c86f5 ("fuse: {io-uring} Handle SQEs - register commands") > > Cc: stable@vger.kernel.org > > Signed-off-by: Joanne Koong <joannelkoong@gmail.com> > > --- > > fs/fuse/dev_uring.c | 14 ++++++++------ > > 1 file changed, 8 insertions(+), 6 deletions(-) > > > > diff --git a/fs/fuse/dev_uring.c b/fs/fuse/dev_uring.c > > index 51f985154aa1..bf9d51f2a508 100644 > > --- a/fs/fuse/dev_uring.c > > +++ b/fs/fuse/dev_uring.c > > @@ -321,9 +321,11 @@ static struct fuse_ring_queue *fuse_uring_create_queue(struct fuse_ring *ring, > > } > > > > /* > > - * write_once and lock as the caller mostly doesn't take the lock at all > > + * fch->lock serializes concurrent creators for this qid. > > + * smp_store_release() are for the lockless readers who must see a > > + * fully initialized queue after &ring->queues[qid] is set > > */ > > - WRITE_ONCE(ring->queues[qid], queue); > > + smp_store_release(&ring->queues[qid], queue); > > spin_unlock(&fch->lock); > > > > return queue; > > @@ -434,7 +436,7 @@ static void fuse_uring_log_ent_state(struct fuse_ring *ring) > > struct fuse_ring_ent *ent; > > > > for (qid = 0; qid < ring->nr_queues; qid++) { > > - struct fuse_ring_queue *queue = ring->queues[qid]; > > + struct fuse_ring_queue *queue = READ_ONCE(ring->queues[qid]); > > > > if (!queue) > > continue; > > @@ -967,7 +969,7 @@ static int fuse_uring_commit_fetch(struct io_uring_cmd *cmd, int issue_flags, > > if (qid >= ring->nr_queues) > > return -EINVAL; > > > > - queue = ring->queues[qid]; > > + queue = READ_ONCE(ring->queues[qid]); > > if (!queue) > > return err; > > fpq = &queue->fpq; > > @@ -1035,7 +1037,7 @@ static bool is_ring_ready(struct fuse_ring *ring, int current_qid) > > if (current_qid == qid) > > continue; > > > > - queue = ring->queues[qid]; > > + queue = READ_ONCE(ring->queues[qid]); > > if (!queue) { > > ready = false; > > break; > > @@ -1191,7 +1193,7 @@ static int fuse_uring_register(struct io_uring_cmd *cmd, > > return -EINVAL; > > } > > > > - queue = ring->queues[qid]; > > + queue = READ_ONCE(ring->queues[qid]); > > if (!queue) { > > queue = fuse_uring_create_queue(ring, qid); > > if (!queue) > > > For consistency, maybe READ_ONCE/WRITE_ONCE in fuse_uring_destruct? I didn't add it because I don't think it's necessary (because fuse_uring_destruct can only run after the queue has been created), but I can add it to make things more uniform, if you have a preference for one way or the other. I'll send out v3 with this change. Thanks, Joanne ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-07-16 18:21 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20260715174305.336261-1-joannelkoong@gmail.com>
2026-07-15 17:43 ` [PATCH v2 1/3] fuse: fix missing barrier when checking io-uring readiness Joanne Koong
2026-07-15 22:26 ` Bernd Schubert
2026-07-16 18:17 ` Joanne Koong
2026-07-15 17:43 ` [PATCH v2 3/3] fuse: publish io-uring queues with release semantics Joanne Koong
2026-07-15 22:54 ` Bernd Schubert
2026-07-16 18:21 ` Joanne Koong
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox