Re: [PATCH v4 1/2] fs/aio: Restrict kiocb_set_cancel_fn() to I/O submitted via libaio

[Eric Biggers <ebiggers@kernel.org>]

On Thu, Feb 15, 2024 at 12:47:38PM -0800, Bart Van Assche wrote:
> void kiocb_set_cancel_fn(struct kiocb *iocb, kiocb_cancel_fn *cancel)
> {
>	struct aio_kiocb *req = container_of(iocb, struct aio_kiocb, rw);
>	struct kioctx *ctx = req->ki_ctx;
>	unsigned long flags;
>  
> +	/*
> +	 * kiocb didn't come from aio or is neither a read nor a write, hence
> +	 * ignore it.
> +	 */
> +	if (!(iocb->ki_flags & IOCB_AIO_RW))
> +		return;
> +
>  	if (WARN_ON_ONCE(!list_empty(&req->ki_list)))
>  		return;
>  

If I understand correctly, this patch is supposed to fix a memory safety bug
when kiocb_set_cancel_fn() is called on a kiocb that is owned by io_uring
instead of legacy AIO.  However, the kiocb still gets accessed as an aio_kiocb
at the very beginning of the function, so it's still broken:

	struct aio_kiocb *req = container_of(iocb, struct aio_kiocb, rw);
	struct kioctx *ctx = req->ki_ctx;

I'm also wondering why "ignore" is the right fix.  The USB gadget driver sees
that it has asynchronous I/O (kiocb::ki_complete != NULL) and then tries to set
a cancellation function.  What is the expected behavior when the I/O is owned by
io_uring?  Should it perhaps call into io_uring to set a cancellation function
with io_uring?  Or is the concept of cancellation functions indeed specific to
legacy AIO, and nothing should be done with io_uring I/O?

- Eric