On Tue, May 26, 2026 at 1:33 PM Pranjal Shrivastava praan@google.com wrote:
On Mon, May 11, 2026, David Hu wrote:
In case MMIO size is bigger than 4G, and peer2peer dma goes through host bridge, we trigger the code path to assign total linked IVOA, greater than 4G
Nit: s/IVOA/IOVA
to mapped_len, and leading to a silent overflow
Fixes: 3aa31a8bb11e ("dma-buf: provide phys_vec to scatter-gather mapping routine") Signed-off-by: David Hu xuehaohu@google.com
drivers/dma-buf/dma-buf-mapping.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/dma-buf/dma-buf-mapping.c b/drivers/dma-buf/dma-buf-mapping.c index 794acff2546a..658064140357 100644 --- a/drivers/dma-buf/dma-buf-mapping.c +++ b/drivers/dma-buf/dma-buf-mapping.c @@ -95,7 +95,8 @@ struct sg_table *dma_buf_phys_vec_to_sgt(struct dma_buf_attachment *attach, size_t nr_ranges, size_t size, enum dma_data_direction dir) {
unsigned int nents, mapped_len = 0;
unsigned int nents = 0;size_t mapped_len = 0; struct dma_buf_dma *dma; struct scatterlist *sgl; dma_addr_t addr;Minor nit: Let's follow the reverse xmas tree format? This looks correct to me, for this change:
Reviewed-by: Pranjal Shrivastava praan@google.com
Apart from this, I see similar issues at other places:
In calc_sg_nents(), nents is accumulated as an unsigned int. [1] If nr_ranges is very large, nents could also overflow, potentially leading to a small allocation in sg_alloc_table() and a subsequent out-of-bounds access in the mapping loop. It might be worth changing nents to size_t there and adding a check against UINT_MAX.
In fill_sg_entry(), the loop variable i is an int [2]. Changing
it to unsigned int would be more consistent with the nents type and safer for extremely large mappings.Maybe, we should also fix these? For example:
diff --git a/drivers/dma-buf/dma-buf-mapping.c b/drivers/dma-buf/dma-buf-mapping.c index 794acff2546a..ecf07ffca2b9 100644 --- a/drivers/dma-buf/dma-buf-mapping.c +++ b/drivers/dma-buf/dma-buf-mapping.c @@ -10,7 +10,7 @@ static struct scatterlist *fill_sg_entry(struct scatterlist *sgl, size_t length, dma_addr_t addr) { unsigned int len, nents;
int i;
unsigned int i; nents = DIV_ROUND_UP(length, UINT_MAX); for (i = 0; i < nents; i++) {@@ -36,7 +36,7 @@ static unsigned int calc_sg_nents(struct dma_iova_state *state, struct phys_vec *phys_vec, size_t nr_ranges, size_t size) {
unsigned int nents = 0;
size_t nents = 0; size_t i; if (!state || !dma_use_iova(state)) {@@ -51,6 +51,9 @@ static unsigned int calc_sg_nents(struct dma_iova_state *state, nents = DIV_ROUND_UP(size, UINT_MAX); }
if (nents > UINT_MAX)return 0;return nents;}
Thanks, Praan
[1] https://elixir.bootlin.com/linux/v7.1-rc3/source/drivers/dma-buf/dma-buf-map... [2] https://elixir.bootlin.com/linux/v7.1-rc3/source/drivers/dma-buf/dma-buf-map...
Thank you Pranjal for the review ! Good catch on other potential overflow sites. I have folded in your suggestions for calc_sg_nents(), and fill_sg_entry(), and applied reverse xmas tree formatting. Sending out a v2 shortly.