search for: virtio_max_dma_size

Both split ring and packed ring use 32bits to describe the length of a descriptor: see struct vring_desc and struct vring_packed_desc. This means the max segment size supported by virtio is U32_MAX. An example of virtio_max_dma_size in virtio_blk.c: u32 v, max_size; max_size = virtio_max_dma_size(vdev); -> implicit convert err = virtio_cread_feature(vdev, VIRTIO_BLK_F_SIZE_MAX, struct virtio_blk_config, size_max, &v); max_size = min(max_size, v); There is a risk during implicit co...

...7 +0800, zhenwei pi <pizhenwei at bytedance.com> wrote: > Both split ring and packed ring use 32bits to describe the length of > a descriptor: see struct vring_desc and struct vring_packed_desc. > This means the max segment size supported by virtio is U32_MAX. > > An example of virtio_max_dma_size in virtio_blk.c: > u32 v, max_size; > > max_size = virtio_max_dma_size(vdev); -> implicit convert > err = virtio_cread_feature(vdev, VIRTIO_BLK_F_SIZE_MAX, > struct virtio_blk_config, size_max, &v); > max_size = min(max_size, v); > &...

On Wed, May 10, 2023 at 10:54:37AM +0800, zhenwei pi wrote: > Both split ring and packed ring use 32bits to describe the length of > a descriptor: see struct vring_desc and struct vring_packed_desc. > This means the max segment size supported by virtio is U32_MAX. > > An example of virtio_max_dma_size in virtio_blk.c: > u32 v, max_size; > > max_size = virtio_max_dma_size(vdev); -> implicit convert > err = virtio_cread_feature(vdev, VIRTIO_BLK_F_SIZE_MAX, > struct virtio_blk_config, size_max, &v); > max_size = min(max_size, v); >...

On Wed, May 10, 2023 at 10:54:37AM +0800, zhenwei pi wrote: > Both split ring and packed ring use 32bits to describe the length of > a descriptor: see struct vring_desc and struct vring_packed_desc. > This means the max segment size supported by virtio is U32_MAX. > > An example of virtio_max_dma_size in virtio_blk.c: > u32 v, max_size; > > max_size = virtio_max_dma_size(vdev); -> implicit convert > err = virtio_cread_feature(vdev, VIRTIO_BLK_F_SIZE_MAX, > struct virtio_blk_config, size_max, &v); > max_size = min(max_size, v); >...

...izhenwei at bytedance.com> wrote: > > Both split ring and packed ring use 32bits to describe the length of > > a descriptor: see struct vring_desc and struct vring_packed_desc. > > This means the max segment size supported by virtio is U32_MAX. > > > > An example of virtio_max_dma_size in virtio_blk.c: > > u32 v, max_size; > > > > max_size = virtio_max_dma_size(vdev); -> implicit convert > > err = virtio_cread_feature(vdev, VIRTIO_BLK_F_SIZE_MAX, > > struct virtio_blk_config, size_max, &v); > > max_s...

...> wrote: > > > Both split ring and packed ring use 32bits to describe the length of > > > a descriptor: see struct vring_desc and struct vring_packed_desc. > > > This means the max segment size supported by virtio is U32_MAX. > > > > > > An example of virtio_max_dma_size in virtio_blk.c: > > > u32 v, max_size; > > > > > > max_size = virtio_max_dma_size(vdev); -> implicit convert > > > err = virtio_cread_feature(vdev, VIRTIO_BLK_F_SIZE_MAX, > > > struct virtio_blk_config, size_max, &...

The following codes have an implicit conversion from size_t to u32: (u32)max_size = (size_t)virtio_max_dma_size(vdev); This may lead overflow, Ex (size_t)4G -> (u32)0. Once virtio_max_dma_size() has a larger size than U32_MAX, use U32_MAX instead. Signed-off-by: zhenwei pi <pizhenwei at bytedance.com> --- drivers/block/virtio_blk.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --...

...ize, sg_elems, opt_io_size; + u32 v, blk_size, max_size, sg_elems, opt_io_size; u16 min_io_size; u8 physical_block_exp, alignment_offset; @@ -826,14 +826,16 @@ static int virtblk_probe(struct virtio_device *vdev) /* No real sector limit. */ blk_queue_max_hw_sectors(q, -1U); + max_size = virtio_max_dma_size(vdev); + /* Host can optionally specify maximum segment size and number of * segments. */ err = virtio_cread_feature(vdev, VIRTIO_BLK_F_SIZE_MAX, struct virtio_blk_config, size_max, &v); if (!err) - blk_queue_max_segment_size(q, v); - else - blk_queue_max_segment_size(q, -1U)...

.../* wtf swapping */ if (bo->pages) @@ -215,8 +216,13 @@ int virtio_gpu_object_get_sg_table(struct virtio_gpu_device *qdev, if (!bo->pages) goto out; - ret = sg_alloc_table_from_pages(bo->pages, pages, nr_pages, 0, - nr_pages << PAGE_SHIFT, GFP_KERNEL); + max_segment = virtio_max_dma_size(qdev->vdev); + max_segment &= ~(size_t)(PAGE_SIZE - 1); + if (max_segment > SCATTERLIST_MAX_SEGMENT) + max_segment = SCATTERLIST_MAX_SEGMENT; + ret = __sg_alloc_table_from_pages(bo->pages, pages, nr_pages, 0, + nr_pages << PAGE_SHIFT, + max_segment, GFP_KERNEL); if...

.../* wtf swapping */ if (bo->pages) @@ -215,8 +216,13 @@ int virtio_gpu_object_get_sg_table(struct virtio_gpu_device *qdev, if (!bo->pages) goto out; - ret = sg_alloc_table_from_pages(bo->pages, pages, nr_pages, 0, - nr_pages << PAGE_SHIFT, GFP_KERNEL); + max_segment = virtio_max_dma_size(qdev->vdev); + max_segment &= ~(size_t)(PAGE_SIZE - 1); + if (max_segment > SCATTERLIST_MAX_SEGMENT) + max_segment = SCATTERLIST_MAX_SEGMENT; + ret = __sg_alloc_table_from_pages(bo->pages, pages, nr_pages, 0, + nr_pages << PAGE_SHIFT, + max_segment, GFP_KERNEL); if...

.../* wtf swapping */ if (bo->pages) @@ -215,8 +216,13 @@ int virtio_gpu_object_get_sg_table(struct virtio_gpu_device *qdev, if (!bo->pages) goto out; - ret = sg_alloc_table_from_pages(bo->pages, pages, nr_pages, 0, - nr_pages << PAGE_SHIFT, GFP_KERNEL); + max_segment = virtio_max_dma_size(qdev->vdev); + max_segment &= PAGE_MASK; + if (max_segment > SCATTERLIST_MAX_SEGMENT) + max_segment = SCATTERLIST_MAX_SEGMENT; + ret = __sg_alloc_table_from_pages(bo->pages, pages, nr_pages, 0, + nr_pages << PAGE_SHIFT, + max_segment, GFP_KERNEL); if (ret) goto...

.../* wtf swapping */ if (bo->pages) @@ -215,8 +216,13 @@ int virtio_gpu_object_get_sg_table(struct virtio_gpu_device *qdev, if (!bo->pages) goto out; - ret = sg_alloc_table_from_pages(bo->pages, pages, nr_pages, 0, - nr_pages << PAGE_SHIFT, GFP_KERNEL); + max_segment = virtio_max_dma_size(qdev->vdev); + max_segment &= PAGE_MASK; + if (max_segment > SCATTERLIST_MAX_SEGMENT) + max_segment = SCATTERLIST_MAX_SEGMENT; + ret = __sg_alloc_table_from_pages(bo->pages, pages, nr_pages, 0, + nr_pages << PAGE_SHIFT, + max_segment, GFP_KERNEL); if (ret) goto...

.../* wtf swapping */ if (bo->pages) @@ -215,8 +216,13 @@ int virtio_gpu_object_get_sg_table(struct virtio_gpu_device *qdev, if (!bo->pages) goto out; - ret = sg_alloc_table_from_pages(bo->pages, pages, nr_pages, 0, - nr_pages << PAGE_SHIFT, GFP_KERNEL); + max_segment = virtio_max_dma_size(qdev->vdev); + max_segment &= PAGE_MASK; + if (max_segment > SCATTERLIST_MAX_SEGMENT) + max_segment = SCATTERLIST_MAX_SEGMENT; + ret = __sg_alloc_table_from_pages(bo->pages, pages, nr_pages, 0, + nr_pages << PAGE_SHIFT, + max_segment, GFP_KERNEL); if (ret) goto...

When running a guest featuring a virtio-blk-pci protected with a virtual IOMMU we hit a NULL pointer dereference. This series removes the dma_max_mapping_size() call in virtio_max_dma_size when the device does not have any dma_mask set. A check is also added to early return in dma_addressing_limited() if the dma_mask is NULL. Eric Auger (2): dma-mapping: Protect dma_addressing_limited against NULL dma_mask virtio/virtio_ring: Fix the dma_max_mapping_size call drivers/virtio/vi...

...5, Eric Auger wrote: > Do not call dma_max_mapping_size for devices that have no DMA > mask set, otherwise we can hit a NULL pointer dereference. > > This occurs when a virtio-blk-pci device is protected with > a virtual IOMMU. > > Fixes: e6d6dd6c875e ("virtio: Introduce virtio_max_dma_size()") > Signed-off-by: Eric Auger <eric.auger at redhat.com> > Suggested-by: Christoph Hellwig <hch at lst.de> > --- > drivers/virtio/virtio_ring.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/virtio/virtio_ring.c b/drivers...

...5, Eric Auger wrote: > Do not call dma_max_mapping_size for devices that have no DMA > mask set, otherwise we can hit a NULL pointer dereference. > > This occurs when a virtio-blk-pci device is protected with > a virtual IOMMU. > > Fixes: e6d6dd6c875e ("virtio: Introduce virtio_max_dma_size()") > Signed-off-by: Eric Auger <eric.auger at redhat.com> > Suggested-by: Christoph Hellwig <hch at lst.de> > --- > drivers/virtio/virtio_ring.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/virtio/virtio_ring.c b/drivers...

...a path. */ -static bool vring_use_dma_api(struct virtio_device *vdev) +static bool vring_use_dma_api(const struct virtio_device *vdev) { if (!virtio_has_dma_quirk(vdev)) return true; @@ -289,7 +289,7 @@ static bool vring_use_dma_api(struct virtio_device *vdev) return false; } -size_t virtio_max_dma_size(struct virtio_device *vdev) +size_t virtio_max_dma_size(const struct virtio_device *vdev) { size_t max_segment_size = SIZE_MAX; @@ -423,7 +423,7 @@ static void virtqueue_init(struct vring_virtqueue *vq, u32 num) */ static void vring_unmap_one_split_indirect(const struct vring_virtqueue *v...

...100, Robin Murphy wrote: >>> diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c >>> index c8be1c4f5b55..37c143971211 100644 >>> --- a/drivers/virtio/virtio_ring.c >>> +++ b/drivers/virtio/virtio_ring.c >>> @@ -262,7 +262,7 @@ size_t virtio_max_dma_size(struct virtio_device *vdev) >>> { >>> size_t max_segment_size = SIZE_MAX; >>> - if (vring_use_dma_api(vdev)) >>> + if (vring_use_dma_api(vdev) && vdev->dev.dma_mask) >> >> Hmm, might it make sense to roll that check up into vring_use...

...100, Robin Murphy wrote: >>> diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c >>> index c8be1c4f5b55..37c143971211 100644 >>> --- a/drivers/virtio/virtio_ring.c >>> +++ b/drivers/virtio/virtio_ring.c >>> @@ -262,7 +262,7 @@ size_t virtio_max_dma_size(struct virtio_device *vdev) >>> { >>> size_t max_segment_size = SIZE_MAX; >>> - if (vring_use_dma_api(vdev)) >>> + if (vring_use_dma_api(vdev) && vdev->dev.dma_mask) >> >> Hmm, might it make sense to roll that check up into vring_use...

...0, Eric Auger wrote: > Do not call dma_max_mapping_size for devices that have no DMA > mask set, otherwise we can hit a NULL pointer dereference. > > This occurs when a virtio-blk-pci device is protected with > a virtual IOMMU. > > Fixes: e6d6dd6c875e ("virtio: Introduce virtio_max_dma_size()") > Signed-off-by: Eric Auger <eric.auger at redhat.com> > Suggested-by: Christoph Hellwig <hch at lst.de> Christoph, I wonder why did you suggest this? The connection between dma_mask and dma_max_mapping_size is far from obvious. The documentation doesn't exist. Do w...