| 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: <20251027-fix-unmap-v5-1-4f0fcf8ffb7d@fb.com>
Date: Mon, 27 Oct 2025 10:33:41 -0700
From: Alex Mastro <amastro@...com>
To: Alex Williamson <alex@...zbot.org>
CC: Jason Gunthorpe <jgg@...pe.ca>,
Alejandro Jimenez
<alejandro.j.jimenez@...cle.com>,
David Matlack <dmatlack@...gle.com>, <kvm@...r.kernel.org>,
<linux-kernel@...r.kernel.org>, Alex Mastro
<amastro@...com>,
Jason Gunthorpe <jgg@...pe.ca>
Subject: [PATCH v5 1/5] vfio/type1: sanitize for overflow using
check_*_overflow
Adopt check_*_overflow functions to clearly express overflow check
intent.
Tested-by: Alejandro Jimenez <alejandro.j.jimenez@...cle.com>
Fixes: 73fa0d10d077 ("vfio: Type1 IOMMU implementation")
Reviewed-by: Jason Gunthorpe <jgg@...dia.com>
Reviewed-by: Alejandro Jimenez <alejandro.j.jimenez@...cle.com>
Signed-off-by: Alex Mastro <amastro@...com>
---
drivers/vfio/vfio_iommu_type1.c | 86 ++++++++++++++++++++++++++++++-----------
1 file changed, 63 insertions(+), 23 deletions(-)
diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
index 916cad80941c..91b1480b7a37 100644
--- a/drivers/vfio/vfio_iommu_type1.c
+++ b/drivers/vfio/vfio_iommu_type1.c
@@ -38,6 +38,7 @@
#include <linux/workqueue.h>
#include <linux/notifier.h>
#include <linux/mm_inline.h>
+#include <linux/overflow.h>
#include "vfio.h"
#define DRIVER_VERSION "0.2"
@@ -182,7 +183,7 @@ static struct vfio_dma *vfio_find_dma(struct vfio_iommu *iommu,
}
static struct rb_node *vfio_find_dma_first_node(struct vfio_iommu *iommu,
- dma_addr_t start, u64 size)
+ dma_addr_t start, size_t size)
{
struct rb_node *res = NULL;
struct rb_node *node = iommu->dma_list.rb_node;
@@ -895,14 +896,20 @@ static int vfio_iommu_type1_pin_pages(void *iommu_data,
unsigned long remote_vaddr;
struct vfio_dma *dma;
bool do_accounting;
+ dma_addr_t iova_end;
+ size_t iova_size;
- if (!iommu || !pages)
+ if (!iommu || !pages || npage <= 0)
return -EINVAL;
/* Supported for v2 version only */
if (!iommu->v2)
return -EACCES;
+ if (check_mul_overflow(npage, PAGE_SIZE, &iova_size) ||
+ check_add_overflow(user_iova, iova_size - 1, &iova_end))
+ return -EOVERFLOW;
+
mutex_lock(&iommu->lock);
if (WARN_ONCE(iommu->vaddr_invalid_count,
@@ -1008,12 +1015,21 @@ static void vfio_iommu_type1_unpin_pages(void *iommu_data,
{
struct vfio_iommu *iommu = iommu_data;
bool do_accounting;
+ dma_addr_t iova_end;
+ size_t iova_size;
int i;
/* Supported for v2 version only */
if (WARN_ON(!iommu->v2))
return;
+ if (WARN_ON(npage <= 0))
+ return;
+
+ if (WARN_ON(check_mul_overflow(npage, PAGE_SIZE, &iova_size) ||
+ check_add_overflow(user_iova, iova_size - 1, &iova_end)))
+ return;
+
mutex_lock(&iommu->lock);
do_accounting = list_empty(&iommu->domain_list);
@@ -1374,7 +1390,8 @@ static int vfio_dma_do_unmap(struct vfio_iommu *iommu,
int ret = -EINVAL, retries = 0;
unsigned long pgshift;
dma_addr_t iova = unmap->iova;
- u64 size = unmap->size;
+ dma_addr_t iova_end;
+ size_t size = unmap->size;
bool unmap_all = unmap->flags & VFIO_DMA_UNMAP_FLAG_ALL;
bool invalidate_vaddr = unmap->flags & VFIO_DMA_UNMAP_FLAG_VADDR;
struct rb_node *n, *first_n;
@@ -1387,6 +1404,11 @@ static int vfio_dma_do_unmap(struct vfio_iommu *iommu,
goto unlock;
}
+ if (iova != unmap->iova || size != unmap->size) {
+ ret = -EOVERFLOW;
+ goto unlock;
+ }
+
pgshift = __ffs(iommu->pgsize_bitmap);
pgsize = (size_t)1 << pgshift;
@@ -1396,10 +1418,15 @@ static int vfio_dma_do_unmap(struct vfio_iommu *iommu,
if (unmap_all) {
if (iova || size)
goto unlock;
- size = U64_MAX;
- } else if (!size || size & (pgsize - 1) ||
- iova + size - 1 < iova || size > SIZE_MAX) {
- goto unlock;
+ size = SIZE_MAX;
+ } else {
+ if (!size || size & (pgsize - 1))
+ goto unlock;
+
+ if (check_add_overflow(iova, size - 1, &iova_end)) {
+ ret = -EOVERFLOW;
+ goto unlock;
+ }
}
/* When dirty tracking is enabled, allow only min supported pgsize */
@@ -1446,7 +1473,7 @@ static int vfio_dma_do_unmap(struct vfio_iommu *iommu,
if (dma && dma->iova != iova)
goto unlock;
- dma = vfio_find_dma(iommu, iova + size - 1, 0);
+ dma = vfio_find_dma(iommu, iova_end, 0);
if (dma && dma->iova + dma->size != iova + size)
goto unlock;
}
@@ -1648,7 +1675,9 @@ static int vfio_dma_do_map(struct vfio_iommu *iommu,
{
bool set_vaddr = map->flags & VFIO_DMA_MAP_FLAG_VADDR;
dma_addr_t iova = map->iova;
+ dma_addr_t iova_end;
unsigned long vaddr = map->vaddr;
+ unsigned long vaddr_end;
size_t size = map->size;
int ret = 0, prot = 0;
size_t pgsize;
@@ -1656,8 +1685,15 @@ static int vfio_dma_do_map(struct vfio_iommu *iommu,
/* Verify that none of our __u64 fields overflow */
if (map->size != size || map->vaddr != vaddr || map->iova != iova)
+ return -EOVERFLOW;
+
+ if (!size)
return -EINVAL;
+ if (check_add_overflow(iova, size - 1, &iova_end) ||
+ check_add_overflow(vaddr, size - 1, &vaddr_end))
+ return -EOVERFLOW;
+
/* READ/WRITE from device perspective */
if (map->flags & VFIO_DMA_MAP_FLAG_WRITE)
prot |= IOMMU_WRITE;
@@ -1673,13 +1709,7 @@ static int vfio_dma_do_map(struct vfio_iommu *iommu,
WARN_ON((pgsize - 1) & PAGE_MASK);
- if (!size || (size | iova | vaddr) & (pgsize - 1)) {
- ret = -EINVAL;
- goto out_unlock;
- }
-
- /* Don't allow IOVA or virtual address wrap */
- if (iova + size - 1 < iova || vaddr + size - 1 < vaddr) {
+ if ((size | iova | vaddr) & (pgsize - 1)) {
ret = -EINVAL;
goto out_unlock;
}
@@ -1710,7 +1740,7 @@ static int vfio_dma_do_map(struct vfio_iommu *iommu,
goto out_unlock;
}
- if (!vfio_iommu_iova_dma_valid(iommu, iova, iova + size - 1)) {
+ if (!vfio_iommu_iova_dma_valid(iommu, iova, iova_end)) {
ret = -EINVAL;
goto out_unlock;
}
@@ -2977,7 +3007,8 @@ static int vfio_iommu_type1_dirty_pages(struct vfio_iommu *iommu,
struct vfio_iommu_type1_dirty_bitmap_get range;
unsigned long pgshift;
size_t data_size = dirty.argsz - minsz;
- size_t iommu_pgsize;
+ size_t size, iommu_pgsize;
+ dma_addr_t iova, iova_end;
if (!data_size || data_size < sizeof(range))
return -EINVAL;
@@ -2986,14 +3017,24 @@ static int vfio_iommu_type1_dirty_pages(struct vfio_iommu *iommu,
sizeof(range)))
return -EFAULT;
- if (range.iova + range.size < range.iova)
+ iova = range.iova;
+ size = range.size;
+
+ if (iova != range.iova || size != range.size)
+ return -EOVERFLOW;
+
+ if (!size)
return -EINVAL;
+
+ if (check_add_overflow(iova, size - 1, &iova_end))
+ return -EOVERFLOW;
+
if (!access_ok((void __user *)range.bitmap.data,
range.bitmap.size))
return -EINVAL;
pgshift = __ffs(range.bitmap.pgsize);
- ret = verify_bitmap_size(range.size >> pgshift,
+ ret = verify_bitmap_size(size >> pgshift,
range.bitmap.size);
if (ret)
return ret;
@@ -3007,19 +3048,18 @@ static int vfio_iommu_type1_dirty_pages(struct vfio_iommu *iommu,
ret = -EINVAL;
goto out_unlock;
}
- if (range.iova & (iommu_pgsize - 1)) {
+ if (iova & (iommu_pgsize - 1)) {
ret = -EINVAL;
goto out_unlock;
}
- if (!range.size || range.size & (iommu_pgsize - 1)) {
+ if (size & (iommu_pgsize - 1)) {
ret = -EINVAL;
goto out_unlock;
}
if (iommu->dirty_page_tracking)
ret = vfio_iova_dirty_bitmap(range.bitmap.data,
- iommu, range.iova,
- range.size,
+ iommu, iova, size,
range.bitmap.pgsize);
else
ret = -EINVAL;
--
2.47.3
Powered by blists - more mailing lists