Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Tue, 17 Nov 2020 11:36:36 +0000
From:   Stefan Hajnoczi <stefanha@redhat.com>
To:     Stefano Garzarella <sgarzare@redhat.com>
Cc:     virtualization@lists.linux-foundation.org,
        Laurent Vivier <lvivier@redhat.com>,
        linux-kernel@vger.kernel.org, Eli Cohen <elic@nvidia.com>,
        Jason Wang <jasowang@redhat.com>,
        "Michael S. Tsirkin" <mst@redhat.com>,
        Max Gurtovoy <mgurtovoy@nvidia.com>
Subject: Re: [PATCH RFC 12/12] vdpa_sim_blk: implement ramdisk behaviour
Message-ID: <20201117113636.GG131917@stefanha-x1.localdomain>
References: <20201113134712.69744-1-sgarzare@redhat.com>
 <20201113134712.69744-13-sgarzare@redhat.com>
MIME-Version: 1.0
In-Reply-To: <20201113134712.69744-13-sgarzare@redhat.com>
Content-Type: multipart/signed; micalg=pgp-sha256;
        protocol="application/pgp-signature"; boundary="NPukt5Otb9an/u20"
Content-Disposition: inline
Precedence: bulk

--NPukt5Otb9an/u20
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Fri, Nov 13, 2020 at 02:47:12PM +0100, Stefano Garzarella wrote:
> diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c b/drivers/vdpa/vdpa_sim=
/vdpa_sim_blk.c
> index 8e41b3ab98d5..68e74383322f 100644
> --- a/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c
> +++ b/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c
> @@ -7,6 +7,7 @@
>   */
> =20
>  #include <linux/module.h>
> +#include <linux/blkdev.h>
>  #include <uapi/linux/virtio_blk.h>
> =20
>  #include "vdpa_sim.h"
> @@ -24,10 +25,137 @@
> =20
>  static struct vdpasim *vdpasim_blk_dev;
> =20
> +static int vdpasim_blk_handle_req(struct vdpasim *vdpasim,
> +=09=09=09=09  struct vdpasim_virtqueue *vq)

This function has a non-standard int return value. Please document it.

> +{
> +=09size_t wrote =3D 0, to_read =3D 0, to_write =3D 0;
> +=09struct virtio_blk_outhdr hdr;
> +=09uint8_t status;
> +=09uint32_t type;
> +=09ssize_t bytes;
> +=09loff_t offset;
> +=09int i, ret;
> +
> +=09vringh_kiov_cleanup(&vq->riov);
> +=09vringh_kiov_cleanup(&vq->wiov);
> +
> +=09ret =3D vringh_getdesc_iotlb(&vq->vring, &vq->riov, &vq->wiov,
> +=09=09=09=09   &vq->head, GFP_ATOMIC);
> +=09if (ret !=3D 1)
> +=09=09return ret;
> +
> +=09for (i =3D 0; i < vq->wiov.used; i++)
> +=09=09to_write +=3D vq->wiov.iov[i].iov_len;
> +=09to_write -=3D 1; /* last byte is the status */

What if vq->wiov.used =3D=3D 0?

> +
> +=09for (i =3D 0; i < vq->riov.used; i++)
> +=09=09to_read +=3D vq->riov.iov[i].iov_len;
> +
> +=09bytes =3D vringh_iov_pull_iotlb(&vq->vring, &vq->riov, &hdr, sizeof(h=
dr));
> +=09if (bytes !=3D sizeof(hdr))
> +=09=09return 0;
> +
> +=09to_read -=3D bytes;
> +
> +=09type =3D le32_to_cpu(hdr.type);
> +=09offset =3D le64_to_cpu(hdr.sector) << SECTOR_SHIFT;
> +=09status =3D VIRTIO_BLK_S_OK;
> +
> +=09switch (type) {
> +=09case VIRTIO_BLK_T_IN:
> +=09=09if (offset + to_write > VDPASIM_BLK_CAPACITY << SECTOR_SHIFT) {

Integer overflow is not handled.

--NPukt5Otb9an/u20
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQEzBAEBCAAdFiEEhpWov9P5fNqsNXdanKSrs4Grc8gFAl+ztcQACgkQnKSrs4Gr
c8iolAgApTBD1PaVo9UuMIiHwUOHDFHckP5/xzilctMNmZYAzMd60k7GRTsYu0z2
eTkPw8Ik1Z5PXqgtSa8w2alDtWE7kJV6x2K+D4Xs3Y4miArXfd8NlozeFYsJCScf
b37DkWKR7sXDLWv/ITGqH5M+HjT6hIbXA4hrOtrUsF9S07OfzwwS7T0HmBxreiom
IUaloJ95O+c8WzhlHzoyxJeC/MSV4wPyivmuJZ6FzrtPMvPhAJwjWa5Xark5s3bM
A9iDFP1eikLSn0zrn03cWy7FzA14QAyO/Ppnv0sllpLDLWx5flHXnyQRIZx74JNh
yuLDV+JjZJroWZYLpKPWdZQ+xoQ/Fw==
=TgK+
-----END PGP SIGNATURE-----

--NPukt5Otb9an/u20--