On Fri, Feb 09, 2024 at 11:30:26AM -0800, Bart Van Assche wrote:
> Originally io_cancel() only supported cancelling USB reads and writes.
> If I/O was cancelled successfully, information about the cancelled I/O
> operation was copied to the data structure the io_cancel() 'result'
> argument points at. Commit 63b05203af57 ("[PATCH] AIO: retry
> infrastructure fixes and enhancements") changed the io_cancel() behavior
> from reporting status information via the 'result' argument into
> reporting status information on the completion ring. Commit 41003a7bcfed
> ("aio: remove retry-based AIO") accidentally changed the behavior into
> not reporting a completion event on the completion ring for cancelled
> requests. This is a bug because successful cancellation leads to an iocb
> leak in user space. Since this bug was introduced more than ten years
> ago and since nobody has complained since then, remove support for I/O
> cancellation. Keep support for cancellation of IOCB_CMD_POLL requests.
>
> Calling kiocb_set_cancel_fn() without knowing whether the caller
> submitted a struct kiocb or a struct aio_kiocb is unsafe. The
> following call trace illustrates that without this patch an
> out-of-bounds write happens if I/O is submitted by io_uring (from a
> phone with an ARM CPU and kernel 6.1):
>
> WARNING: CPU: 3 PID: 368 at fs/aio.c:598 kiocb_set_cancel_fn+0x9c/0xa8
> Call trace:
> kiocb_set_cancel_fn+0x9c/0xa8
> ffs_epfile_read_iter+0x144/0x1d0
> io_read+0x19c/0x498
> io_issue_sqe+0x118/0x27c
> io_submit_sqes+0x25c/0x5fc
> __arm64_sys_io_uring_enter+0x104/0xab0
> invoke_syscall+0x58/0x11c
> el0_svc_common+0xb4/0xf4
> do_el0_svc+0x2c/0xb0
> el0_svc+0x2c/0xa4
> el0t_64_sync_handler+0x68/0xb4
> el0t_64_sync+0x1a4/0x1a8
>
> Cc: Christoph Hellwig <hch@xxxxxx>
> Cc: Avi Kivity <avi@xxxxxxxxxxxx>
> Cc: Sandeep Dhavale <dhavale@xxxxxxxxxx>
> Cc: Jens Axboe <axboe@xxxxxxxxx>
> Cc: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
> Fixes: 63b05203af57 ("[PATCH] AIO: retry infrastructure fixes and enhancements")
> Cc: <stable@xxxxxxxxxxxxxxx>
> Signed-off-by: Bart Van Assche <bvanassche@xxxxxxx>
> ---
Great,
Reviewed-by: Christian Brauner <brauner@xxxxxxxxxx>
