Re: Re: Re: [PATCH 0/2] fuse: Fix possible memleak at startup with immediate teardown

[Horst Birthelmer <horst@birthelmer.de>]

On Fri, Apr 10, 2026 at 10:09:36AM -0700, Joanne Koong wrote:
> On Fri, Apr 10, 2026 at 12:21 AM Horst Birthelmer <horst@birthelmer.de> wrote:
> >
> > On Thu, Apr 09, 2026 at 04:09:53PM -0700, Joanne Koong wrote:
> > > On Thu, Apr 9, 2026 at 4:02 AM Bernd Schubert <bernd@bsbernd.com> wrote:
> > > >
> > > >
> > > >
> > > > On 10/21/25 23:33, Bernd Schubert wrote:
> > > > > Do not merge yet, the current series has not been tested yet.
> > > >
> > > > I'm glad that that I was hesitating to apply it, the DDN branch had it
> > > > for ages and this patch actually introduced a possible fc->num_waiting
> > > > issue, because fc->uring->queue_refs might go down to 0 though
> > > > fuse_uring_cancel() and then fuse_uring_abort() would never stop and
> > > > flush the queues without another addition.
> > > >
> > >
> > > Hi Bernd and Jian,
> > >
> > > For some reason the "[PATCH 2/2] fs/fuse: fix potential memory leak
> > > from fuse_uring_cancel" email was never delivered to my inbox, so I am
> > > just going to write my reply to that patch here instead, hope that's
> > > ok.
> > >
> > > Just to summarize, the race is that during unmount, fuse_abort() ->
> > > fuse_uring_abort() -> ... -> fuse_uring_teardown_entries() -> ... ->
> > > fuse_uring_entry_teardown() gets run but there may still be sqes that
> > > are being registered, which results in new ents that are created (and
> > > leaked) after the teardown logic has finished and the queues are
> > > stopped/dead. The async teardown work (fuse_uring_async_stop_queues())
> > > never gets scheduled because at the time of teardown, queue->refs is 0
> > > as those sqes have not fully created the ents and grabbed refs yet.
> > > fuse_uring_destruct() runs during unmount, but this doesn't clean up
> > > the created ents because those registered ents got put on the
> > > ent_in_userspace list which fuse_uring_destruct() doesn't go through
> > > to free, resulting in those ents being leaked.
> > >
> > > The root cause of the race is that ents are being registered even when
> > > the queue is already stopped/dead. I think if we at registration time
> > > check the queue state before calling fuse_uring_prepare_cancel(), we
> > > eliminate the race altogether. If we see that the abort path has
> > > already triggered (eg queue->stopped == true), we manually free the
> > > ent and return an error instead of adding it to a list, eg
> >
> > In my case (Bernd mentioned that I was investigating a hang during umount)
> > there were a lot of requests created during teardown, so what happened
> > was very similar, but for exact the opposite reason.
> > In fuse_uring_abort() queue_refs was already 0 due to an optimization
> > where the ring teardown ran before fuse_abort_conn().
> 
> Hi Horst,
> 
> Just to clarify, is this with running locally patched changes on your
> ddn kernel? In the upstream code I'm seeing that teardown is only
> called by the abort path, eg fuse_abort_conn() -> fuse_uring_abort()
> -> fuse_uring_stop_queues() -> teardown logic, so I'm not seeing how
> it's possible for teardown to run before fuse_abort_conn(). Is there
> something I'm missing?

Yes and no ... ;-)
The original patch this whole discussion was started by had a call to
the teardown of the entries and I had that applied.
But even without that the problem can still occur that queue_refs is 0
by the time fuse_abort_conn() is called.

> 
> > Thus the queue->stopped was never set.
> >
> > How do we make sure that fuse_uring_teardown_entries() has not been
> > called by fuse_uring_async_stop_queues()?
> 
> If i'm understanding your question correctly, your question is what
> ensures the teardown logic in fuse_uring_async_stop_queues() hasn't
> already executed by the time we drop the queue lock after checking if
> the queue has been stopped? In fuse_uring_async_stop_queues(), the
> async teardown work gets continuously rescheduled so long as
> queue_refs > 0. The ent holds a reference on the queue, so when the
> queue lock is dropped that async teardown work will be continuously
> running until it cleans up that (and any other) ents.
> 

You understand correctly.
If the fuse_async_stop_queues() runs there is still a window where
we have queue_refs == 0. If in that window fuse_abort_conn() runs
we never actually stop the queues and we can accept requests which
will never be processed.

I have never seen this happen without the patch mentioned above,
but with that 'optimization' it happens regularly when you are able to
kill the fuse server and the application using the file system more or
less at the same time e.g. by an OOM event, when the kernel tries to
free resources.

To me this looks like nothing will stop this from happening, though,
but maybe I'm just not familiar enough with the uring code ...

> >
> > Maybe I'm missing something?
> >
> > My fix was to remove the check for queue_refs > 0 in fuse_uring_abort()
> > and make sure that even if the teardown was complete nothing bad happens
> > in fuse_uring_abort_end_requests() and fuse_uring_stop_queues().
> 
> I'll look more at this path today.
> 
> Thanks,
> Joanne

Thanks,
Horst