Re: [PATCH 2/3] usb: gadget: f_fs: register cancellation handler only after request queueing

[Ingo Rohloff <ingo.rohloff@lauterbach.com>]

Hello Neill,

> On Fri, 6 Nov 2026 22:47:48 +0000
> Neill Kapron <nkapron@google.com> wrote:
> 
> > On Tue, Sep 01, 2026 at 02:09:07PM +0200, Ingo Rohloff wrote:  
> > > Call kiocb_set_cancel_fn() only after usb_ep_queue() completes
> > > successfully.
> > > ...  
> >
> > Unfortunately, I don't think this is safe. The cancel handler has to be
> > set before the request is queued.  
> ...
> You should **NOT** call "kiocb_set_cancel_fn(...)",
> before the request is successfully queued via "usb_ep_queue()".

I just had another look at the code and I am even more convinced than
before that commit  ("usb: gadget: f_fs: Fix Use-After-Free in
AIO error path") is wrong. Especially this code part

	if (res == -EIOCBQUEUED)
		return res;
	/* Comment by Ingo: Code here menans there was an error */
	if (p->aio) {
		kiocb->ki_complete(kiocb, res);
		mmdrop(p->mm);
		kfree(p);
		return -EIOCBQUEUED;
	}

Returning "-EIOCBQUEUED" when in fact nothing has been queued
(because of an error) doesn't make any sense to me.

As far as I can tell, the whole point of returning "-EIOCBQUEUED" is  to tell
aio.c that something is "in progress"; which is exactly  the opposite of
what's happening here.

Returning -EIOCBQUEUED is just a hack here, to prevent
"aio_rw_done(...)" (in aio.c) from calling "kiocb->ki_complete(...)" after 
ffs_epfile_write_iter/ffs_epfile_read_iter
has returned.

with best regards
  Ingo

On Mon, 9 Nov 2026 12:31:40 +0100
Ingo Rohloff <ingo.rohloff@lauterbach.com> wrote:

> Hello Neill,
> 
> On Fri, 6 Nov 2026 22:47:48 +0000
> Neill Kapron <nkapron@google.com> wrote:
> 
> > On Tue, Sep 01, 2026 at 02:09:07PM +0200, Ingo Rohloff wrote:  
> > > Call kiocb_set_cancel_fn() only after usb_ep_queue() completes
> > > successfully.
> > > ...  
> >
> > Unfortunately, I don't think this is safe. The cancel handler has to be
> > set before the request is queued.   
> 
> Why do you think, that the cancel handler has to be set before 
> the request is queued?
> 
> I can't find any reason for this.
> 
> I will describe this in terms of an AIO write (read is the same),
> referring to the current kernel (without my patch):
> 
> The request enters the kernel via
> SYSCALL_DEFINE3(io_submit
> ...
>   io_submit_one(ctx, user_iocb, false);
>     __io_submit_one(ctx, &iocb, user_iocb, req, compat);   
>       ...
>         case IOCB_CMD_PWRITE:
>           return aio_write(&req->rw, iocb, false, compat);
>       aio_write
>           ...
>           aio_rw_done(req, file->f_op->write_iter(req, &iter));
>             file->f_op->write_iter == ffs_epfile_write_iter
>         ffs_epfile_write_iter(struct kiocb *kiocb, struct iov_iter *from)
>             kiocb_set_cancel_fn(...)
>             ffs_epfile_io(...)
>               ret = usb_ep_queue(ep->ep, req, GFP_ATOMIC);
> 
> 
> I can't see why you should need to call "kiocb_set_cancel_fn(...)"
> before you call "usb_ep_queue()".
> In fact I think that's exactly wrong:
> You should **NOT** call "kiocb_set_cancel_fn(...)",
> before the request is successfully queued via "usb_ep_queue()".
> 
> Unfortunately f_fs.c seems the only place where "kiocb_set_cancel_fn()"
> is used, so there is no other code to compare to.
> 
> If "usb_ep_queue()" fails, then "ffs_epfile_io()"
> will **NOT** return -EIOCBQUEUED.
> 
> This in turn means the code in "aio_rw_done(...)"
> will call "req->ki_complete(req, ret)", because that's
> what "aio_rw_done(...)" does, if you **DO NOT** return -EIOCBQUEUED.
> 
> As far as I can tell, this is exactly what we want:
> The request is completed with an error,
> which indicates that the request was not queued.
> 
> Because the request is completed, why should you ever
> set a cancel handler for it?
> So why should you call kiocb_set_cancel_fn before the
> request is queued successfully?
> 
> with best regards
>   Ingo
> 



-- 

-------------------------------------------------------------------------
Dipl.-Inform.
Ingo ROHLOFF
Senior Staff Engineer
phone +49 8102 9876-142 - ingo.rohloff@lauterbach.com

Lauterbach Engineering GmbH & Co. KG
Altlaufstrasse 40, 85635 Hoehenkirchen-Siegertsbrunn, GERMANY
www.lauterbach.com

Registered Office: Hoehenkirchen-Siegertsbrunn, Germany,
Local Court: Munich, HRA 87406, VAT ID: DE246770537, 
Managing Directors: Stephan Lauterbach, Dr. Thomas Ullmann

-------------------------------------------------------------------------