linux-kernel - Re: [PATCH 3/3] vdpa_sim_blk: call vringh_complete

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CACGkMEv1Y_rKSc799qi2=EkrQfm4E_d9aMWefXyz5h4vdXjxgA@mail.gmail.com>
Date:   Thu, 23 Jun 2022 11:52:19 +0800
From:   Jason Wang <jasowang@...hat.com>
To:     Stefano Garzarella <sgarzare@...hat.com>
Cc:     virtualization <virtualization@...ts.linux-foundation.org>,
        "Michael S. Tsirkin" <mst@...hat.com>,
        linux-kernel <linux-kernel@...r.kernel.org>,
        Eugenio Pérez <eperezma@...hat.com>
Subject: Re: [PATCH 3/3] vdpa_sim_blk: call vringh_complete_iotlb() also in
 the error path

On Wed, Jun 22, 2022 at 12:13 AM Stefano Garzarella <sgarzare@...hat.com> wrote:
>
> Call vringh_complete_iotlb() even when we encounter a serious error
> that prevents us from writing the status in the "in" header
> (e.g. the header length is incorrect, etc.).
>
> The guest is misbehaving, so maybe the ring is in a bad state, but
> let's avoid making things worse.
>
> Signed-off-by: Stefano Garzarella <sgarzare@...hat.com>

Acked-by: Jason Wang <jasowang@...hat.com>

> ---
>  drivers/vdpa/vdpa_sim/vdpa_sim_blk.c | 14 +++++++++-----
>  1 file changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c b/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c
> index ac86478845b6..de9cd9843143 100644
> --- a/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c
> +++ b/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c
> @@ -63,6 +63,7 @@ static bool vdpasim_blk_handle_req(struct vdpasim *vdpasim,
>  {
>         size_t pushed = 0, to_pull, to_push;
>         struct virtio_blk_outhdr hdr;
> +       bool handled = false;
>         ssize_t bytes;
>         loff_t offset;
>         u64 sector;
> @@ -78,12 +79,12 @@ static bool vdpasim_blk_handle_req(struct vdpasim *vdpasim,
>         if (vq->out_iov.used < 1 || vq->in_iov.used < 1) {
>                 dev_dbg(&vdpasim->vdpa.dev, "missing headers - out_iov: %u in_iov %u\n",
>                         vq->out_iov.used, vq->in_iov.used);
> -               return false;
> +               goto err;
>         }
>
>         if (vq->in_iov.iov[vq->in_iov.used - 1].iov_len < 1) {
>                 dev_dbg(&vdpasim->vdpa.dev, "request in header too short\n");
> -               return false;
> +               goto err;
>         }
>
>         /* The last byte is the status and we checked if the last iov has
> @@ -97,7 +98,7 @@ static bool vdpasim_blk_handle_req(struct vdpasim *vdpasim,
>                                       sizeof(hdr));
>         if (bytes != sizeof(hdr)) {
>                 dev_dbg(&vdpasim->vdpa.dev, "request out header too short\n");
> -               return false;
> +               goto err;
>         }
>
>         to_pull -= bytes;
> @@ -182,16 +183,19 @@ static bool vdpasim_blk_handle_req(struct vdpasim *vdpasim,
>         /* Last byte is the status */
>         bytes = vringh_iov_push_iotlb(&vq->vring, &vq->in_iov, &status, 1);
>         if (bytes != 1)
> -               return false;
> +               goto err;
>
>         pushed += bytes;
>
>         /* Make sure data is wrote before advancing index */
>         smp_wmb();
>
> +       handled = true;
> +
> +err:
>         vringh_complete_iotlb(&vq->vring, vq->head, pushed);
>
> -       return true;
> +       return handled;
>  }
>
>  static void vdpasim_blk_work(struct work_struct *work)
> --
> 2.36.1
>