On Tue, May 26, 2026 at 1:33 PM Pranjal Shrivastava <[email protected]> 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 <[email protected]>
> > ---
> > 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 <[email protected]>
>
> Apart from this, I see similar issues at other places:
>
> 1. 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.
>
> 2. 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-mapping.c#L39
> [2]
> https://elixir.bootlin.com/linux/v7.1-rc3/source/drivers/dma-buf/dma-buf-mapping.c#L13
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.
