On 09/03/2020 11:02 am, Kevin Grandemange wrote:
pageno is an int and the PAGE_SHIFT shift is done on an int,
overflowing if the memory is bigger than 2G
This can be reproduced using for example a reserved-memory of 4G
Nit: the example shows 16GB, not 4.
reserved-memory {
#address-cells = <2>;
#size-cells = <2>;
ranges;
reserved_dma: buffer@0 {
compatible = "shared-dma-pool";
no-map;
reg = <0x5 0x00000000 0x4 0x0>;
};
};
Signed-off-by: Kevin Grandemange <[email protected]>
---
kernel/dma/coherent.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/kernel/dma/coherent.c b/kernel/dma/coherent.c
index 551b0eb7028a..c20c6c0635b7 100644
--- a/kernel/dma/coherent.c
+++ b/kernel/dma/coherent.c
@@ -130,6 +130,7 @@ static void *__dma_alloc_from_coherent(struct device *dev,
int order = get_order(size);
unsigned long flags;
int pageno;
+ ssize_t mem_offset;
void *ret;
spin_lock_irqsave(&mem->spinlock, flags);
@@ -144,8 +145,9 @@ static void *__dma_alloc_from_coherent(struct device *dev,
/*
* Memory was found in the coherent area.
*/
- *dma_handle = dma_get_device_base(dev, mem) + (pageno << PAGE_SHIFT);
- ret = mem->virt_base + (pageno << PAGE_SHIFT);
+ mem_offset = (ssize_t)pageno << PAGE_SHIFT;
Isn't this still capable of overflowing for 32-bit architectures where
ssize_t is most likely to be long and LONG_MAX == INT_MAX (before we
even get to PAE and friends)? Logically, dma_addr_t would be the most
appropriate type here.
FWIW since you have to have an explicit cast either way, I don't see
much point in introducing the local variable vs. just adding the cast
into the existing expression.
Thanks,
Robin.
+ *dma_handle = dma_get_device_base(dev, mem) + mem_offset;
+ ret = mem->virt_base + mem_offset;
spin_unlock_irqrestore(&mem->spinlock, flags);
memset(ret, 0, size);
return ret;
_______________________________________________
iommu mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/iommu
