Although the support of VIRTIO_F_ANY_LAYOUT + VIRTIO_F_VERSION_1 was
signaled by the commit 664ed90e621c ("vhost/scsi: Set
VIRTIO_F_ANY_LAYOUT + VIRTIO_F_VERSION_1 feature bits"),
vhost_scsi_send_bad_target() still assumes the response in a single
descriptor.
Similar issue in vhost_scsi_send_bad_target() has been fixed in previous
commit.
Fixes: 3ca51662f818 ("vhost-scsi: Add better resource allocation failure handling")
Signed-off-by: Dongli Zhang <dongli.zhang@oracle.com>
---
Changed since v1:
- New patch to fix vhost_scsi_send_status().
drivers/vhost/scsi.c | 18 +++++++++++-------
1 file changed, 11 insertions(+), 7 deletions(-)
diff --git a/drivers/vhost/scsi.c b/drivers/vhost/scsi.c
index 59d907b94c5e..26bcf3a7f70c 100644
--- a/drivers/vhost/scsi.c
+++ b/drivers/vhost/scsi.c
@@ -999,18 +999,22 @@ static void vhost_scsi_target_queue_cmd(struct vhost_scsi_nexus *nexus,
static void
vhost_scsi_send_status(struct vhost_scsi *vs, struct vhost_virtqueue *vq,
- int head, unsigned int out, u8 status)
+ struct vhost_scsi_ctx *vc, u8 status)
{
- struct virtio_scsi_cmd_resp __user *resp;
struct virtio_scsi_cmd_resp rsp;
+ struct iov_iter iov_iter;
int ret;
memset(&rsp, 0, sizeof(rsp));
rsp.status = status;
- resp = vq->iov[out].iov_base;
- ret = __copy_to_user(resp, &rsp, sizeof(rsp));
- if (!ret)
- vhost_add_used_and_signal(&vs->dev, vq, head, 0);
+
+ iov_iter_init(&iov_iter, ITER_DEST, &vq->iov[vc->out], vc->in,
+ sizeof(rsp));
+
+ ret = copy_to_iter(&rsp, sizeof(rsp), &iov_iter);
+
+ if (likely(ret == sizeof(rsp)))
+ vhost_add_used_and_signal(&vs->dev, vq, vc->head, 0);
else
pr_err("Faulted on virtio_scsi_cmd_resp\n");
}
@@ -1420,7 +1424,7 @@ vhost_scsi_handle_vq(struct vhost_scsi *vs, struct vhost_virtqueue *vq)
else if (ret == -EIO)
vhost_scsi_send_bad_target(vs, vq, &vc, TYPE_IO_CMD);
else if (ret == -ENOMEM)
- vhost_scsi_send_status(vs, vq, vc.head, vc.out,
+ vhost_scsi_send_status(vs, vq, &vc,
SAM_STAT_TASK_SET_FULL);
} while (likely(!vhost_exceeds_weight(vq, ++c, 0)));
out:
--
2.39.3On 3/17/25 6:55 PM, Dongli Zhang wrote:
> Although the support of VIRTIO_F_ANY_LAYOUT + VIRTIO_F_VERSION_1 was
> signaled by the commit 664ed90e621c ("vhost/scsi: Set
> VIRTIO_F_ANY_LAYOUT + VIRTIO_F_VERSION_1 feature bits"),
> vhost_scsi_send_bad_target() still assumes the response in a single
> descriptor.
>
> Similar issue in vhost_scsi_send_bad_target() has been fixed in previous
> commit.
>
> Fixes: 3ca51662f818 ("vhost-scsi: Add better resource allocation failure handling")
> Signed-off-by: Dongli Zhang <dongli.zhang@oracle.com>
Reviewed-by: Mike Christie <michael.christie@oracle.com>
On Tue, Mar 18, 2025 at 7:52 AM Dongli Zhang <dongli.zhang@oracle.com> wrote:
>
> Although the support of VIRTIO_F_ANY_LAYOUT + VIRTIO_F_VERSION_1 was
> signaled by the commit 664ed90e621c ("vhost/scsi: Set
> VIRTIO_F_ANY_LAYOUT + VIRTIO_F_VERSION_1 feature bits"),
> vhost_scsi_send_bad_target() still assumes the response in a single
> descriptor.
>
> Similar issue in vhost_scsi_send_bad_target() has been fixed in previous
> commit.
>
> Fixes: 3ca51662f818 ("vhost-scsi: Add better resource allocation failure handling")
And
6dd88fd59da84631b5fe5c8176931c38cfa3b265 ("vhost-scsi: unbreak any
layout for response")
> Signed-off-by: Dongli Zhang <dongli.zhang@oracle.com>
> ---
> Changed since v1:
> - New patch to fix vhost_scsi_send_status().
>
> drivers/vhost/scsi.c | 18 +++++++++++-------
> 1 file changed, 11 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/vhost/scsi.c b/drivers/vhost/scsi.c
> index 59d907b94c5e..26bcf3a7f70c 100644
> --- a/drivers/vhost/scsi.c
> +++ b/drivers/vhost/scsi.c
> @@ -999,18 +999,22 @@ static void vhost_scsi_target_queue_cmd(struct vhost_scsi_nexus *nexus,
>
> static void
> vhost_scsi_send_status(struct vhost_scsi *vs, struct vhost_virtqueue *vq,
> - int head, unsigned int out, u8 status)
> + struct vhost_scsi_ctx *vc, u8 status)
> {
> - struct virtio_scsi_cmd_resp __user *resp;
> struct virtio_scsi_cmd_resp rsp;
> + struct iov_iter iov_iter;
> int ret;
>
> memset(&rsp, 0, sizeof(rsp));
> rsp.status = status;
> - resp = vq->iov[out].iov_base;
> - ret = __copy_to_user(resp, &rsp, sizeof(rsp));
> - if (!ret)
> - vhost_add_used_and_signal(&vs->dev, vq, head, 0);
> +
> + iov_iter_init(&iov_iter, ITER_DEST, &vq->iov[vc->out], vc->in,
> + sizeof(rsp));
> +
> + ret = copy_to_iter(&rsp, sizeof(rsp), &iov_iter);
> +
> + if (likely(ret == sizeof(rsp)))
> + vhost_add_used_and_signal(&vs->dev, vq, vc->head, 0);
> else
> pr_err("Faulted on virtio_scsi_cmd_resp\n");
> }
> @@ -1420,7 +1424,7 @@ vhost_scsi_handle_vq(struct vhost_scsi *vs, struct vhost_virtqueue *vq)
> else if (ret == -EIO)
> vhost_scsi_send_bad_target(vs, vq, &vc, TYPE_IO_CMD);
> else if (ret == -ENOMEM)
> - vhost_scsi_send_status(vs, vq, vc.head, vc.out,
> + vhost_scsi_send_status(vs, vq, &vc,
> SAM_STAT_TASK_SET_FULL);
> } while (likely(!vhost_exceeds_weight(vq, ++c, 0)));
> out:
> --
> 2.39.3
>
Acked-by: Jason Wang <jasowang@redhat.com>
Thanks
Hi Jason,
On 3/17/25 5:48 PM, Jason Wang wrote:
> On Tue, Mar 18, 2025 at 7:52 AM Dongli Zhang <dongli.zhang@oracle.com> wrote:
>>
>> Although the support of VIRTIO_F_ANY_LAYOUT + VIRTIO_F_VERSION_1 was
>> signaled by the commit 664ed90e621c ("vhost/scsi: Set
>> VIRTIO_F_ANY_LAYOUT + VIRTIO_F_VERSION_1 feature bits"),
>> vhost_scsi_send_bad_target() still assumes the response in a single
>> descriptor.
>>
>> Similar issue in vhost_scsi_send_bad_target() has been fixed in previous
>> commit.
>>
>> Fixes: 3ca51662f818 ("vhost-scsi: Add better resource allocation failure handling")
>
> And
>
> 6dd88fd59da84631b5fe5c8176931c38cfa3b265 ("vhost-scsi: unbreak any
> layout for response")
>
Would suggest add the commit to Fixes?
vhost_scsi_send_status() has been introduced by the most recent patch.
It isn't related to that commit. That commit is to fix
vhost_scsi_complete_cmd_work()
Or would you suggest mention it as part of "Similar issue has been fixed in
previous commit."?
Thank you very much!
Dongli Zhang
On Thu, Mar 20, 2025 at 1:54 AM Dongli Zhang <dongli.zhang@oracle.com> wrote:
>
> Hi Jason,
>
> On 3/17/25 5:48 PM, Jason Wang wrote:
> > On Tue, Mar 18, 2025 at 7:52 AM Dongli Zhang <dongli.zhang@oracle.com> wrote:
> >>
> >> Although the support of VIRTIO_F_ANY_LAYOUT + VIRTIO_F_VERSION_1 was
> >> signaled by the commit 664ed90e621c ("vhost/scsi: Set
> >> VIRTIO_F_ANY_LAYOUT + VIRTIO_F_VERSION_1 feature bits"),
> >> vhost_scsi_send_bad_target() still assumes the response in a single
> >> descriptor.
> >>
> >> Similar issue in vhost_scsi_send_bad_target() has been fixed in previous
> >> commit.
> >>
> >> Fixes: 3ca51662f818 ("vhost-scsi: Add better resource allocation failure handling")
> >
> > And
> >
> > 6dd88fd59da84631b5fe5c8176931c38cfa3b265 ("vhost-scsi: unbreak any
> > layout for response")
> >
>
> Would suggest add the commit to Fixes?
>
> vhost_scsi_send_status() has been introduced by the most recent patch.
>
> It isn't related to that commit. That commit is to fix
> vhost_scsi_complete_cmd_work()
>
> Or would you suggest mention it as part of "Similar issue has been fixed in
> previous commit."?
I'm fine with either, they are all fixes for any header layout anyhow.
Thanks
>
> Thank you very much!
>
> Dongli Zhang
>
© 2016 - 2025 Red Hat, Inc.