On Fri, Mar 14, 2025 at 3:51 PM Bernd Schubert
<bernd.schubert@xxxxxxxxxxx> wrote:
>
> On 3/14/25 21:50, Joanne Koong wrote:
> > There is a race condition leading to a kernel crash from a null
> > dereference when attemping to access fc->lock in
> > fuse_uring_create_queue(). fc may be NULL in the case where another
> > thread is creating the uring in fuse_uring_create() and has set
> > fc->ring but has not yet set ring->fc when fuse_uring_create_queue()
> > reads ring->fc.
> >
> > This fix passes fc to fuse_uring_create_queue() instead of accessing it
> > through ring->fc.
> >
> > Signed-off-by: Joanne Koong <joannelkoong@xxxxxxxxx>
> > Fixes: 24fe962c86f5 ("fuse: {io-uring} Handle SQEs - register commands")
> > ---
> > fs/fuse/dev_uring.c | 6 +++---
> > 1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/fs/fuse/dev_uring.c b/fs/fuse/dev_uring.c
> > index ab8c26042aa8..64f1ae308dc4 100644
> > --- a/fs/fuse/dev_uring.c
> > +++ b/fs/fuse/dev_uring.c
> > @@ -250,10 +250,10 @@ static struct fuse_ring *fuse_uring_create(struct fuse_conn *fc)
> > return res;
> > }
> >
> > -static struct fuse_ring_queue *fuse_uring_create_queue(struct fuse_ring *ring,
> > +static struct fuse_ring_queue *fuse_uring_create_queue(struct fuse_conn *fc,
> > + struct fuse_ring *ring,
> > int qid)
> > {
> > - struct fuse_conn *fc = ring->fc;
> > struct fuse_ring_queue *queue;
> > struct list_head *pq;
> >
> > @@ -1088,7 +1088,7 @@ static int fuse_uring_register(struct io_uring_cmd *cmd,
> >
> > queue = ring->queues[qid];
> > if (!queue) {
> > - queue = fuse_uring_create_queue(ring, qid);
> > + queue = fuse_uring_create_queue(fc, ring, qid);
> > if (!queue)
> > return err;
> > }
>
> I wonder if we could get more issues,
> fuse_uring_register()
> {
> struct fuse_ring *ring = smp_load_acquire(&fc->ring);
> ...
> if (qid >= ring->nr_queues) {
>
> ...
> }
>
> In fuse_uring_create()
> {
> ...
> fc->ring = ring;
> ring->nr_queues = nr_queues;
> ring->fc = fc;
> ring->max_payload_sz = max_payload_size;
> ...
> }
>
>
> I.e. we cannot trust any of the values assigned after fc->ring?
Seems like it, great point. I think we'll need to do the smp memory
barrier way after all then. I'll send that as v3.
> Sorry about this, I should have noticed before :(
No worries at all, I appreciate all the work you've put getting uring
into fuse :)
Thanks,
Joanne
>
>
> Thanks,
> Bernd
