| 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
| ||
|
Message-ID: <CACGkMEuS3DsjgP0RB2C-DbsACq4FU6RKD4C+p3dOGQHWdZJJhg@mail.gmail.com>
Date: Tue, 27 Jun 2023 16:03:19 +0800
From: Jason Wang <jasowang@...hat.com>
To: Xuan Zhuo <xuanzhuo@...ux.alibaba.com>
Cc: virtualization@...ts.linux-foundation.org,
"Michael S. Tsirkin" <mst@...hat.com>, "David S. Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>,
Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, Alexei Starovoitov <ast@...nel.org>,
Daniel Borkmann <daniel@...earbox.net>, Jesper Dangaard Brouer <hawk@...nel.org>,
John Fastabend <john.fastabend@...il.com>, netdev@...r.kernel.org, bpf@...r.kernel.org
Subject: Re: [PATCH vhost v10 01/10] virtio_ring: put mapping error check in vring_map_one_sg
On Fri, Jun 2, 2023 at 5:22 PM Xuan Zhuo <xuanzhuo@...ux.alibaba.com> wrote:
>
> This patch put the dma addr error check in vring_map_one_sg().
>
> The benefits of doing this:
>
> 1. reduce one judgment of vq->use_dma_api.
> 2. make vring_map_one_sg more simple, without calling
> vring_mapping_error to check the return value. simplifies subsequent
> code
>
> Signed-off-by: Xuan Zhuo <xuanzhuo@...ux.alibaba.com>
Acked-by: Jason Wang <jasowang@...hat.com>
Thanks
> ---
> drivers/virtio/virtio_ring.c | 37 +++++++++++++++++++++---------------
> 1 file changed, 22 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
> index c5310eaf8b46..72ed07a604d4 100644
> --- a/drivers/virtio/virtio_ring.c
> +++ b/drivers/virtio/virtio_ring.c
> @@ -355,9 +355,8 @@ static struct device *vring_dma_dev(const struct vring_virtqueue *vq)
> }
>
> /* Map one sg entry. */
> -static dma_addr_t vring_map_one_sg(const struct vring_virtqueue *vq,
> - struct scatterlist *sg,
> - enum dma_data_direction direction)
> +static int vring_map_one_sg(const struct vring_virtqueue *vq, struct scatterlist *sg,
> + enum dma_data_direction direction, dma_addr_t *addr)
> {
> if (!vq->use_dma_api) {
> /*
> @@ -366,7 +365,8 @@ static dma_addr_t vring_map_one_sg(const struct vring_virtqueue *vq,
> * depending on the direction.
> */
> kmsan_handle_dma(sg_page(sg), sg->offset, sg->length, direction);
> - return (dma_addr_t)sg_phys(sg);
> + *addr = (dma_addr_t)sg_phys(sg);
> + return 0;
> }
>
> /*
> @@ -374,9 +374,14 @@ static dma_addr_t vring_map_one_sg(const struct vring_virtqueue *vq,
> * the way it expects (we don't guarantee that the scatterlist
> * will exist for the lifetime of the mapping).
> */
> - return dma_map_page(vring_dma_dev(vq),
> + *addr = dma_map_page(vring_dma_dev(vq),
> sg_page(sg), sg->offset, sg->length,
> direction);
> +
> + if (dma_mapping_error(vring_dma_dev(vq), *addr))
> + return -ENOMEM;
> +
> + return 0;
> }
>
> static dma_addr_t vring_map_single(const struct vring_virtqueue *vq,
> @@ -588,8 +593,9 @@ static inline int virtqueue_add_split(struct virtqueue *_vq,
>
> for (n = 0; n < out_sgs; n++) {
> for (sg = sgs[n]; sg; sg = sg_next(sg)) {
> - dma_addr_t addr = vring_map_one_sg(vq, sg, DMA_TO_DEVICE);
> - if (vring_mapping_error(vq, addr))
> + dma_addr_t addr;
> +
> + if (vring_map_one_sg(vq, sg, DMA_TO_DEVICE, &addr))
> goto unmap_release;
>
> prev = i;
> @@ -603,8 +609,9 @@ static inline int virtqueue_add_split(struct virtqueue *_vq,
> }
> for (; n < (out_sgs + in_sgs); n++) {
> for (sg = sgs[n]; sg; sg = sg_next(sg)) {
> - dma_addr_t addr = vring_map_one_sg(vq, sg, DMA_FROM_DEVICE);
> - if (vring_mapping_error(vq, addr))
> + dma_addr_t addr;
> +
> + if (vring_map_one_sg(vq, sg, DMA_FROM_DEVICE, &addr))
> goto unmap_release;
>
> prev = i;
> @@ -1279,9 +1286,8 @@ static int virtqueue_add_indirect_packed(struct vring_virtqueue *vq,
>
> for (n = 0; n < out_sgs + in_sgs; n++) {
> for (sg = sgs[n]; sg; sg = sg_next(sg)) {
> - addr = vring_map_one_sg(vq, sg, n < out_sgs ?
> - DMA_TO_DEVICE : DMA_FROM_DEVICE);
> - if (vring_mapping_error(vq, addr))
> + if (vring_map_one_sg(vq, sg, n < out_sgs ?
> + DMA_TO_DEVICE : DMA_FROM_DEVICE, &addr))
> goto unmap_release;
>
> desc[i].flags = cpu_to_le16(n < out_sgs ?
> @@ -1426,9 +1432,10 @@ static inline int virtqueue_add_packed(struct virtqueue *_vq,
> c = 0;
> for (n = 0; n < out_sgs + in_sgs; n++) {
> for (sg = sgs[n]; sg; sg = sg_next(sg)) {
> - dma_addr_t addr = vring_map_one_sg(vq, sg, n < out_sgs ?
> - DMA_TO_DEVICE : DMA_FROM_DEVICE);
> - if (vring_mapping_error(vq, addr))
> + dma_addr_t addr;
> +
> + if (vring_map_one_sg(vq, sg, n < out_sgs ?
> + DMA_TO_DEVICE : DMA_FROM_DEVICE, &addr))
> goto unmap_release;
>
> flags = cpu_to_le16(vq->packed.avail_used_flags |
> --
> 2.32.0.3.g01195cf9f
>
Powered by blists - more mailing lists