[PATCH] vhost-scsi: fix event queue iov out-of-bounds

[PATCH] vhost-scsi: fix event queue iov out-of-bounds

[Jia Jia]

vhost_scsi_do_evt_work() uses vq->iov[out] after vhost_get_vq_desc()
without checking that the descriptor chain contains an input segment.
vq->iov has UIO_MAXIOV entries (1024).  A chain of 1024 output-only
descriptors yields out == 1024 and in == 0, so the length check reads
one past the end of the array.

With a host-side harness that sets VHOST_SCSI_SET_EVENTS_MISSED and
kicks an event queue filled with 1024 OUT descriptors, UBSAN reports:

  UBSAN: array-index-out-of-bounds in drivers/vhost/scsi.c:611:14
  index 1024 is out of range for type 'iovec [1024]'

  Call Trace:
   <TASK>
   dump_stack_lvl+0x5f/0x90
   dump_stack+0x10/0x18
   ubsan_epilogue+0x9/0x39
   __ubsan_handle_out_of_bounds.cold+0x50/0x55
   vhost_scsi_complete_events+0x55f/0x5a0 [vhost_scsi]
   vhost_scsi_evt_work+0x17/0x30 [vhost_scsi]
   vhost_run_work_list+0x8e/0xd0 [vhost]
   vhost_task_fn+0xe1/0x210
   ret_from_fork+0x348/0x540
   ret_from_fork_asm+0x1a/0x30
   </TASK>

Require at least one input descriptor before indexing iov[out], and
treat a pure-output chain as a missed event like other invalid event
buffers.

Fixes: a6c9af87363c ("tcm_vhost: Add hotplug/hotunplug support")
Signed-off-by: Jia Jia <physicalmtea@gmail.com>
---
 drivers/vhost/scsi.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/vhost/scsi.c b/drivers/vhost/scsi.c
index 9a1253b9d8c5..9b5f07f52b0c 100644
--- a/drivers/vhost/scsi.c
+++ b/drivers/vhost/scsi.c
@@ -608,9 +608,10 @@
 		return;
 	}
 
-	if ((vq->iov[out].iov_len != sizeof(struct virtio_scsi_event))) {
+	if (!in ||
+	    vq->iov[out].iov_len != sizeof(struct virtio_scsi_event)) {
 		vq_err(vq, "Expecting virtio_scsi_event, got %zu bytes\n",
-				vq->iov[out].iov_len);
+		       in ? vq->iov[out].iov_len : 0);
 		vs->vs_events_missed = true;
 		return;
 	}
-- 
2.43.0

Re: [PATCH] vhost-scsi: fix event queue iov out-of-bounds

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Self-deadlock (AA deadlock) in vhost_scsi_send_evt when vhost_vq_work_queue fails.
--

commit c17fff972ec26b541af9ec7b514db052328d64e6
Author: Jia Jia <physicalmtea@gmail.com>

vhost-scsi: fix event queue iov out-of-bounds

This commit fixes an array-index-out-of-bounds issue in vhost_scsi_do_evt_work()
by ensuring there is at least one input descriptor before indexing the iov array.
It treats pure-output chains as missed events to prevent reading past the end
of the array.

> diff --git a/drivers/vhost/scsi.c b/drivers/vhost/scsi.c
> index 7a1f39a327da6..5a0863d560a07 100644
> --- a/drivers/vhost/scsi.c
> +++ b/drivers/vhost/scsi.c
> @@ -608,9 +608,10 @@ vhost_scsi_do_evt_work(struct vhost_scsi *vs, struct vhost_scsi_evt *evt)
>  		return;
>  	}
>  
> -	if ((vq->iov[out].iov_len != sizeof(struct virtio_scsi_event))) {
> +	if (!in ||
> +	    vq->iov[out].iov_len != sizeof(struct virtio_scsi_event)) {

[Severity: High]
This is a pre-existing issue, but while reviewing this event handling code, I
noticed a potential self-deadlock scenario.

If vhost_scsi_send_evt() is called from a path that already holds vq->mutex,
like vhost_scsi_evt_handle_kick():

vhost_scsi_evt_handle_kick()
    mutex_lock(&vq->mutex);
    ...
    if (vs->vs_events_missed)
        vhost_scsi_send_evt(...)

And vhost_scsi_send_evt() falls back to vhost_scsi_complete_events() because
vhost_vq_work_queue() returns false (for instance, if vq->worker is NULL):

vhost_scsi_send_evt()
    if (!vhost_vq_work_queue(vq, &vs->vs_event_work))
        vhost_scsi_complete_events(vs, true);

vhost_scsi_complete_events() then attempts to take the exact same mutex:

vhost_scsi_complete_events()
    mutex_lock(&vq->mutex);

Could this sequence cause a self-deadlock where the kernel thread hangs
indefinitely waiting on the non-recursive vq->mutex, potentially leading to a
denial of service?

>  		vq_err(vq, "Expecting virtio_scsi_event, got %zu bytes\n",
> -				vq->iov[out].iov_len);
> +		       in ? vq->iov[out].iov_len : 0);
>  		vs->vs_events_missed = true;
>  		return;
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260904005240.502787-1-physicalmtea@gmail.com?part=1

Re: [PATCH] vhost-scsi: fix event queue iov out-of-bounds

[Jia Jia]

>
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
>
> Pre-existing issues:
> - [High] Self-deadlock (AA deadlock) in vhost_scsi_send_evt when vhost_vq_work_queue fails.
> --
>
> commit c17fff972ec26b541af9ec7b514db052328d64e6
> Author: Jia Jia <physicalmtea@gmail.com>
>
> vhost-scsi: fix event queue iov out-of-bounds
>
> This commit fixes an array-index-out-of-bounds issue in vhost_scsi_do_evt_work()
> by ensuring there is at least one input descriptor before indexing the iov array.
> It treats pure-output chains as missed events to prevent reading past the end
> of the array.
>
> > diff --git a/drivers/vhost/scsi.c b/drivers/vhost/scsi.c
> > index 7a1f39a327da6..5a0863d560a07 100644
> > --- a/drivers/vhost/scsi.c
> > +++ b/drivers/vhost/scsi.c
> > @@ -608,9 +608,10 @@ vhost_scsi_do_evt_work(struct vhost_scsi *vs, struct vhost_scsi_evt *evt)
> >               return;
> >       }
> >
> > -     if ((vq->iov[out].iov_len != sizeof(struct virtio_scsi_event))) {
> > +     if (!in ||
> > +         vq->iov[out].iov_len != sizeof(struct virtio_scsi_event)) {
>
> [Severity: High]
> This is a pre-existing issue, but while reviewing this event handling code, I
> noticed a potential self-deadlock scenario.
>
> If vhost_scsi_send_evt() is called from a path that already holds vq->mutex,
> like vhost_scsi_evt_handle_kick():
>
> vhost_scsi_evt_handle_kick()
>     mutex_lock(&vq->mutex);
>     ...
>     if (vs->vs_events_missed)
>         vhost_scsi_send_evt(...)
>
> And vhost_scsi_send_evt() falls back to vhost_scsi_complete_events() because
> vhost_vq_work_queue() returns false (for instance, if vq->worker is NULL):
>
> vhost_scsi_send_evt()
>     if (!vhost_vq_work_queue(vq, &vs->vs_event_work))
>         vhost_scsi_complete_events(vs, true);
>
> vhost_scsi_complete_events() then attempts to take the exact same mutex:
>
> vhost_scsi_complete_events()
>     mutex_lock(&vq->mutex);
>
> Could this sequence cause a self-deadlock where the kernel thread hangs
> indefinitely waiting on the non-recursive vq->mutex, potentially leading to a
> denial of service?
>
> >               vq_err(vq, "Expecting virtio_scsi_event, got %zu bytes\n",
> > -                             vq->iov[out].iov_len);
> > +                    in ? vq->iov[out].iov_len : 0);
> >               vs->vs_events_missed = true;
> >               return;
> >       }
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260904005240.502787-1-physicalmtea@gmail.com?part=1

This is a false positive. The current patch does not introduce this
issue, but I will try to reproduce it to verify whether it is a real
issue.