Reviewed-by: Andrey Zhadchenko <andrey.zhadchenko at virtuozzo.com>
On 6/5/26 19:49, Konstantin Khorenko wrote:
> While reviewing the recent "vhost-blk: stop fetching descriptors on
> VHOST_BLK_SET_BACKEND(-1)" patch I went over the surrounding error paths
> in drivers/vhost/blk.c and found a few latent bugs. None of them are
> introduced by that patch - they all date back to the original vhost-blk
> import (40a5928ec730) - but they share a common theme: a return value or
> a userspace-supplied value is not validated the way the code assumes,
> and the mistake stays invisible during normal operation. This series
> fixes them. No functional change for the well-behaved fast paths.
>
> 1. fix NULL deref on bad fd in VHOST_BLK_SET_BACKEND
> vhost_blk_set_backend() checks fget() with IS_ERR(), but fget()
> signals failure by returning NULL, not an ERR_PTR. IS_ERR(NULL) is
> false, so a bad (non-negative) fd slips through and the next line
> dereferences it (file->f_mapping->host) and oopses. Test for NULL
> and return -EBADF.
>
> 2. fix out-of-bounds req[] access on vhost_get_vq_desc() error
> [most important] vhost_blk_handle_guest_kick() stores the
> vhost_get_vq_desc() return value in a u16 head. On error the
> function returns a negative errno, which truncates to a large
> positive u16, so the "head < 0" check is always false and the bogus
> value is used to index blk_vq->req[head] - a wild out-of-bounds
> access tens of MB past the allocation. Reachable by a guest that
> corrupts its avail ring / descriptor chain. Declare head as int.
>
> 3. re-validate vq index in vhost_blk_setup() (double-fetch)
> vhost_blk_setup() uses a userspace-supplied vq index as an array
> index into blk->vqs[] without bounds checking. It is validated by
> vhost_vring_ioctl() first, but that is a separate read of the same
> userspace memory; a second thread of the same process can rewrite
> the index between the two reads (classic double-fetch / TOCTOU).
> Re-validate against VHOST_BLK_VQ_MAX where it is used. This is
> local hardening on the kernel/userspace boundary - the trigger
> needs an already-privileged fd owner (a host-side VMM, not the
> guest) winning a narrow race - not a guest-triggerable escape.
>
> 4. return int, not size_t, from the bounce-buffer copy helpers
> vhost_blk_move_{req_to_bb,bb_to_req}() are typed size_t but
> "return -EINVAL". It works today only by truncation/non-zeroness
> in the callers; switch the return type to int. No functional
> change - cleanup that removes a future foot-gun.
>
> Patches 1-3 are correctness/memory-safety fixes; patch 4 is cleanup.
> All four are independent and could be applied in any order.
>
> Konstantin Khorenko (4):
> vhost-blk: fix NULL deref on bad fd in VHOST_BLK_SET_BACKEND
> vhost-blk: fix out-of-bounds req[] access on vhost_get_vq_desc() error
> vhost-blk: re-validate vq index in vhost_blk_setup() (double-fetch)
> vhost-blk: return int, not size_t, from the bounce-buffer copy helpers
>
> drivers/vhost/blk.c | 19 ++++++++++++++-----
> 1 file changed, 14 insertions(+), 5 deletions(-)
>