Eric Auger
2019-Jul-22 14:55 UTC
[PATCH 0/2] Fix NULL pointer dereference with virtio-blk-pci and virtual IOMMU
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/virtio_ring.c | 2 +- include/linux/dma-mapping.h | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) -- 2.20.1
Eric Auger
2019-Jul-22 14:55 UTC
[PATCH 1/2] dma-mapping: Protect dma_addressing_limited against NULL dma_mask
dma_addressing_limited() should not be called on a device with a NULL dma_mask. If this occurs let's WARN_ON_ONCE and immediately return. Existing call sites are updated separately. Fixes: b866455423e0 ("dma-mapping: add a dma_addressing_limited helper") Signed-off-by: Eric Auger <eric.auger at redhat.com> --- v1 -> v2: - add a WARN_ON_ONCE() - reword the commit message --- include/linux/dma-mapping.h | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h index e11b115dd0e4..ef0cf9537abc 100644 --- a/include/linux/dma-mapping.h +++ b/include/linux/dma-mapping.h @@ -689,8 +689,9 @@ static inline int dma_coerce_mask_and_coherent(struct device *dev, u64 mask) */ static inline bool dma_addressing_limited(struct device *dev) { - return min_not_zero(*dev->dma_mask, dev->bus_dma_mask) < - dma_get_required_mask(dev); + return WARN_ON_ONCE(!dev->dma_mask) ? false : + min_not_zero(*dev->dma_mask, dev->bus_dma_mask) < + dma_get_required_mask(dev); } #ifdef CONFIG_ARCH_HAS_SETUP_DMA_OPS -- 2.20.1
Eric Auger
2019-Jul-22 14:55 UTC
[PATCH 2/2] virtio/virtio_ring: Fix the dma_max_mapping_size call
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/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) max_segment_size = dma_max_mapping_size(&vdev->dev); return max_segment_size; -- 2.20.1
Christoph Hellwig
2019-Jul-22 15:26 UTC
[PATCH 1/2] dma-mapping: Protect dma_addressing_limited against NULL dma_mask
> static inline bool dma_addressing_limited(struct device *dev) > { > - return min_not_zero(*dev->dma_mask, dev->bus_dma_mask) < > - dma_get_required_mask(dev); > + return WARN_ON_ONCE(!dev->dma_mask) ? false : > + min_not_zero(*dev->dma_mask, dev->bus_dma_mask) < > + dma_get_required_mask(dev);This should really use a separate if statement, but I can fix that up when applying it.
Christoph Hellwig
2019-Jul-22 15:27 UTC
[PATCH 2/2] virtio/virtio_ring: Fix the dma_max_mapping_size call
On Mon, Jul 22, 2019 at 04:55:09PM +0200, 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>Looks good. virtio maintainers, let me know if you want to queue it up or if I should pick the patch up through the dma-mapping tree.
Michael S. Tsirkin
2019-Jul-22 15:33 UTC
[PATCH 2/2] virtio/virtio_ring: Fix the dma_max_mapping_size call
On Mon, Jul 22, 2019 at 04:55:09PM +0200, 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 we really have to teach all users about this hack? Why not just make dma_max_mapping_size DTRT?> --- > drivers/virtio/virtio_ring.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > 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) > max_segment_size = dma_max_mapping_size(&vdev->dev); > > return max_segment_size; > -- > 2.20.1
Robin Murphy
2019-Jul-22 15:36 UTC
[PATCH 2/2] virtio/virtio_ring: Fix the dma_max_mapping_size call
On 22/07/2019 15:55, 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/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_dma_api() itself? After all, if the device has no mask then it's likely that other DMA API ops wouldn't really work as expected either. Robin.> max_segment_size = dma_max_mapping_size(&vdev->dev); > > return max_segment_size; >