ublk: clear server ownership before aborting in-flight requests

[PATCH] ublk: clear server ownership before aborting in-flight requests

Posted by ZhengYuan Huang 1 month ago

[BUG]
A stale UBLK_IO_COMMIT_AND_FETCH_REQ can reach the normal completion path
after ublk has already aborted the in-flight request, leading to a
use-after-free in map/unmap mode:

BUG: KASAN: use-after-free in ublk_copy_io_pages drivers/block/ublk_drv.c:946 [inline]
BUG: KASAN: use-after-free in ublk_copy_user_pages+0x83c/0xcc0 drivers/block/ublk_drv.c:1013
Write of size 4096 at addr ffff88800ce2a000 by task ublk.fsfuzz/275

Call Trace:
 ...
 ublk_copy_io_pages drivers/block/ublk_drv.c:946 [inline]
 ublk_copy_user_pages+0x83c/0xcc0 drivers/block/ublk_drv.c:1013
 ublk_unmap_io+0x2bb/0x350 drivers/block/ublk_drv.c:1076
 __ublk_complete_rq drivers/block/ublk_drv.c:1188 [inline]
 ublk_ch_uring_cmd_local+0x157c/0x2180 drivers/block/ublk_drv.c:2477
 ublk_ch_uring_cmd+0x42/0x640 drivers/block/ublk_drv.c:2561
 io_uring_cmd+0x26f/0x570 io_uring/uring_cmd.c:263
 __io_issue_sqe+0xc2/0x760 io_uring/io_uring.c:1826
 io_issue_sqe+0xdd/0x11e0 io_uring/io_uring.c:1849
 io_queue_sqe io_uring/io_uring.c:2076 [inline]
 io_submit_sqe io_uring/io_uring.c:2336 [inline]
 io_submit_sqes+0x806/0x2390 io_uring/io_uring.c:2449
 __do_sys_io_uring_enter+0x5c0/0x13a0 io_uring/io_uring.c:3516
 __se_sys_io_uring_enter io_uring/io_uring.c:3455 [inline]
 __x64_sys_io_uring_enter+0xe5/0x1c0 io_uring/io_uring.c:3455
 x64_sys_call+0x2419/0x26a0 arch/x86/include/generated/asm/syscalls_64.h:427
 ...

[CAUSE]
commit e63d2228ef83 ("ublk: simplify aborting ublk request") removed the
abort-only completion state and now __ublk_fail_req() fails or requeues
the request without first revoking UBLK_IO_FLAG_OWNED_BY_SRV. That leaves
the tag looking as if it is still owned by the ublk server, so a stale
COMMIT_AND_FETCH_REQ can pass the ownership check, reuse io->req, and
call __ublk_complete_rq() after the request has already been ended. In map
mode that drives ublk_unmap_io() into freed request pages.

[FIX]
Clear UBLK_IO_FLAG_OWNED_BY_SRV as soon as abort starts in
__ublk_fail_req(). Once ownership is revoked, any stale
COMMIT_AND_FETCH_REQ fails before touching io->req, so the completion path
can no longer copy into freed bio pages.

Fixes: e63d2228ef83 ("ublk: simplify aborting ublk request")
Signed-off-by: ZhengYuan Huang <gality369@gmail.com>
---
 drivers/block/ublk_drv.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
index 57ec900f0ce0..4f16a1ce7d2a 100644
--- a/drivers/block/ublk_drv.c
+++ b/drivers/block/ublk_drv.c
@@ -2673,6 +2673,9 @@ static void __ublk_fail_req(struct ublk_device *ub, struct ublk_io *io,
 	WARN_ON_ONCE(!ublk_dev_support_batch_io(ub) &&
 			io->flags & UBLK_IO_FLAG_ACTIVE);
 
+	/* The ublk server no longer owns this request once abort starts. */
+	io->flags &= ~UBLK_IO_FLAG_OWNED_BY_SRV;
+
 	if (ublk_nosrv_should_reissue_outstanding(ub))
 		blk_mq_requeue_request(req, false);
 	else {
-- 
2.43.0

Re: [PATCH] ublk: clear server ownership before aborting in-flight requests

Posted by Caleb Sander Mateos 1 month ago

On Tue, May 12, 2026 at 2:16 AM ZhengYuan Huang <gality369@gmail.com> wrote:
>
> [BUG]
> A stale UBLK_IO_COMMIT_AND_FETCH_REQ can reach the normal completion path
> after ublk has already aborted the in-flight request, leading to a
> use-after-free in map/unmap mode:

How is it possible for UBLK_IO_COMMIT_AND_FETCH_REQ to be issued after
the ublk device has been aborted? __ublk_fail_req() is called from
ublk_abort_queue(), which is called from ublk_ch_release_work_fn().
Issuing UBLK_IO_COMMIT_AND_FETCH_REQ requires a reference to the ublkc
file, so I don't see how it can happen after the last reference to the
file has been released by the ublk server.
Plus, ublk_ch_release_work_fn() will call ublk_reset_ch_dev() ->
ublk_queue_reinit() later anyways, which clears all io->flags except
for UBLK_IO_FLAG_CANCELED.

>
> BUG: KASAN: use-after-free in ublk_copy_io_pages drivers/block/ublk_drv.c:946 [inline]

This function doesn't exist upstream anymore, can you test against the
latest ublk?

Best,
Caleb

> BUG: KASAN: use-after-free in ublk_copy_user_pages+0x83c/0xcc0 drivers/block/ublk_drv.c:1013
> Write of size 4096 at addr ffff88800ce2a000 by task ublk.fsfuzz/275
>
> Call Trace:
>  ...
>  ublk_copy_io_pages drivers/block/ublk_drv.c:946 [inline]
>  ublk_copy_user_pages+0x83c/0xcc0 drivers/block/ublk_drv.c:1013
>  ublk_unmap_io+0x2bb/0x350 drivers/block/ublk_drv.c:1076
>  __ublk_complete_rq drivers/block/ublk_drv.c:1188 [inline]
>  ublk_ch_uring_cmd_local+0x157c/0x2180 drivers/block/ublk_drv.c:2477
>  ublk_ch_uring_cmd+0x42/0x640 drivers/block/ublk_drv.c:2561
>  io_uring_cmd+0x26f/0x570 io_uring/uring_cmd.c:263
>  __io_issue_sqe+0xc2/0x760 io_uring/io_uring.c:1826
>  io_issue_sqe+0xdd/0x11e0 io_uring/io_uring.c:1849
>  io_queue_sqe io_uring/io_uring.c:2076 [inline]
>  io_submit_sqe io_uring/io_uring.c:2336 [inline]
>  io_submit_sqes+0x806/0x2390 io_uring/io_uring.c:2449
>  __do_sys_io_uring_enter+0x5c0/0x13a0 io_uring/io_uring.c:3516
>  __se_sys_io_uring_enter io_uring/io_uring.c:3455 [inline]
>  __x64_sys_io_uring_enter+0xe5/0x1c0 io_uring/io_uring.c:3455
>  x64_sys_call+0x2419/0x26a0 arch/x86/include/generated/asm/syscalls_64.h:427
>  ...
>
> [CAUSE]
> commit e63d2228ef83 ("ublk: simplify aborting ublk request") removed the
> abort-only completion state and now __ublk_fail_req() fails or requeues
> the request without first revoking UBLK_IO_FLAG_OWNED_BY_SRV. That leaves
> the tag looking as if it is still owned by the ublk server, so a stale
> COMMIT_AND_FETCH_REQ can pass the ownership check, reuse io->req, and
> call __ublk_complete_rq() after the request has already been ended. In map
> mode that drives ublk_unmap_io() into freed request pages.
>
> [FIX]
> Clear UBLK_IO_FLAG_OWNED_BY_SRV as soon as abort starts in
> __ublk_fail_req(). Once ownership is revoked, any stale
> COMMIT_AND_FETCH_REQ fails before touching io->req, so the completion path
> can no longer copy into freed bio pages.
>
> Fixes: e63d2228ef83 ("ublk: simplify aborting ublk request")
> Signed-off-by: ZhengYuan Huang <gality369@gmail.com>
> ---
>  drivers/block/ublk_drv.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
> index 57ec900f0ce0..4f16a1ce7d2a 100644
> --- a/drivers/block/ublk_drv.c
> +++ b/drivers/block/ublk_drv.c
> @@ -2673,6 +2673,9 @@ static void __ublk_fail_req(struct ublk_device *ub, struct ublk_io *io,
>         WARN_ON_ONCE(!ublk_dev_support_batch_io(ub) &&
>                         io->flags & UBLK_IO_FLAG_ACTIVE);
>
> +       /* The ublk server no longer owns this request once abort starts. */
> +       io->flags &= ~UBLK_IO_FLAG_OWNED_BY_SRV;
> +
>         if (ublk_nosrv_should_reissue_outstanding(ub))
>                 blk_mq_requeue_request(req, false);
>         else {
> --
> 2.43.0
>