On Mon, Mar 04, 2024 at 01:09:15PM -0700, Jens Axboe wrote: > > 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; > > > Doesn't matter, they are both just pointer math. But it'd look cleaner > if it was below. It dereferences the pointer. > > > 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? > > Because the ->ki_cancel() is a hack, as demonstrated by this issue in > teh first place, which is a gross layering violation. io_uring supports > proper cancelations, invoked from userspace. It would never have worked > with this scheme. Maybe kiocb_set_cancel_fn() should have a comment that explains this? - Eric
