Re: [PATCH 3/4] export/fuse: Safe termination for FUSE-uring

[Stefan Hajnoczi <stefanha@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2457 bytes --]

On Tue, Sep 09, 2025 at 04:51:32PM -0400, Brian Song wrote:
> 
> 
> On 9/9/25 3:33 PM, Stefan Hajnoczi wrote:
> > On Fri, Aug 29, 2025 at 10:50:24PM -0400, Brian Song wrote:
> > > @@ -901,24 +941,15 @@ static void fuse_export_shutdown(BlockExport *blk_exp)
> > >            */
> > >           g_hash_table_remove(exports, exp->mountpoint);
> > >       }
> > > -}
> > > -
> > > -static void fuse_export_delete(BlockExport *blk_exp)
> > > -{
> > > -    FuseExport *exp = container_of(blk_exp, FuseExport, common);
> > > -    for (int i = 0; i < exp->num_queues; i++) {
> > > +    for (size_t i = 0; i < exp->num_queues; i++) {
> > >           FuseQueue *q = &exp->queues[i];
> > >           /* Queue 0's FD belongs to the FUSE session */
> > >           if (i > 0 && q->fuse_fd >= 0) {
> > >               close(q->fuse_fd);
> > 
> > This changes the behavior of the non-io_uring code. Now all fuse fds and
> > fuse_session are closed while requests are potentially still being
> > processed.
> > 
> > There is a race condition: if an IOThread is processing a request here
> > then it may invoke a system call on q->fuse_fd just after it has been
> > closed but not set to -1. If another thread has also opened a new file
> > then the fd could be reused, resulting in an accidental write(2) to the
> > new file. I'm not sure whether there is a way to trigger this in
> > practice, but it looks like a problem waiting to happen.
> > 
> > Simply setting q->fuse_fd to -1 here doesn't fix the race. It would be
> > necessary to stop processing fuse_fd in the thread before closing it
> > here or to schedule a BH in each thread so that fuse_fd can be closed
> > in the thread that uses the fd.
> 
> I get what you mean. This newly introduced cleanup code was originally in
> the deletion section, after the reconf counter decreased to 0, and it was
> meant to cancel the pending SQEs. But now we've moved it to the shutdown
> section, which may introduce a potential problem. How do you think we should
> fix it? This is the last week of GSoC, I'm already busy on weekdays since
> the new term has started.

Hi Brian,
Two issues:
1. Change of behavior for non-io_uring code. It would be safer to keep
   the old behavior for non-io_uring code.
2. The race condition. Schedule a BH in each queue's IOThread and call
   close(fuse_fd) from the BH function. That way there is no race
   between threads.

Stefan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]