]> git.ipfire.org Git - thirdparty/linux.git/commitdiff
ublk: reset kernel-owned dev_info fields in ublk_ctrl_add_dev()
authorMing Lei <tom.leiming@gmail.com>
Sun, 26 Jul 2026 14:50:25 +0000 (09:50 -0500)
committerJens Axboe <axboe@kernel.dk>
Mon, 27 Jul 2026 17:08:48 +0000 (11:08 -0600)
ublk_ctrl_add_dev() memcpy()s the userspace ublksrv_ctrl_dev_info into
ub->dev_info and then fixes up the fields the driver owns, but misses
->state and ->ublksrv_pid.

A device added with ->state = UBLK_S_DEV_LIVE passes the
"->state != UBLK_S_DEV_DEAD" test that ublk_stop_dev_unlocked() uses as its
proxy for "a disk is attached", while ->ub_disk is still NULL, so DEL_DEV
right after ADD_DEV oopses in del_gendisk().  UBLK_S_DEV_QUIESCED plus
UBLK_F_USER_RECOVERY dies one step earlier, in ublk_force_abort_dev().  A
poisoned ->state also gets START_USER_RECOVERY and the char device
read/write path onto a device that was never started, and wedges START_DEV
at -EEXIST.  A poisoned ->ublksrv_pid just makes GET_DEV_INFO report an
unrelated task as the ublk server.

Reset both after the memcpy(), as ublk_detach_disk() does.  Userspace only
ever reads these back, so correcting them silently breaks nothing.

ADD_DEV has copied ->state in unsanitized since ublk was merged, but back
then it was harmless: the gendisk was allocated during ADD_DEV, and both
teardown and the START_DEV -EEXIST check keyed off disk_live() rather than
->state.  The oops became reachable once the disk allocation moved to
START_DEV and those checks switched to ->state.

Fixes: 6d9e6dfdf3b2 ("ublk: defer disk allocation")
Cc: stable@vger.kernel.org
Signed-off-by: Ming Lei <tom.leiming@gmail.com>
Reviewed-by: Caleb Sander Mateos <csander@purestorage.com>
Link: https://patch.msgid.link/20260726145025.1507383-1-tom.leiming@gmail.com
Signed-off-by: Jens Axboe <axboe@kernel.dk>
drivers/block/ublk_drv.c

index 4ca6ec738c9302e7fa7d93a6a02c65e080ca61a4..2a22f9dc1f2f1540975d527df9375c36379dab8d 100644 (file)
@@ -4764,6 +4764,15 @@ static int ublk_ctrl_add_dev(const struct ublksrv_ctrl_cmd *header)
        /* update device id */
        ub->dev_info.dev_id = ub->ub_number;
 
+       /*
+        * ->state and ->ublksrv_pid are owned by the driver and only read back
+        * by userspace, but they come from the copied-in dev_info, so reset
+        * them. Otherwise a device added with ->state != DEAD looks live while
+        * ->ub_disk is still NULL.
+        */
+       ub->dev_info.state = UBLK_S_DEV_DEAD;
+       ub->dev_info.ublksrv_pid = -1;
+
        /*
         * 64bit flags will be copied back to userspace as feature
         * negotiation result, so have to clear flags which driver