Re: [PATCH v5] dma-buf: Fix silent overflow for phys vec to sgt

[David Hu]

On Thu, Jun 4, 2026 at 5:43 AM Leon Romanovsky <leon@xxxxxxxxxx> wrote:
>
> On Mon, Jun 01, 2026 at 08:00:12PM +0000, David Hu wrote:
> > @@ -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)
>
> I would suggest to use check_add_overflow() while calculating nents
> instead of this check.

Hi Leon,

Thank you for the review. Using `check_add_overflow()` is a great
suggestion and definitely
cleaner for the accumulation loop. I'll update this for v6.

> > @@ -133,6 +137,11 @@ struct sg_table *dma_buf_phys_vec_to_sgt(struct dma_buf_attachment *attach,
> >       }
> >
> >       nents = calc_sg_nents(dma->state, phys_vec, nr_ranges, size);
> > +     if (!nents) {
> > +             ret = -EINVAL;
> > +             goto err_free_state;
> > +     }
>
> Technically, this hunk is not necessary, since sg_alloc_table() will
> return -EINVAL when nents == 0. At least, that is the behavior I relied on.

I originally added this explicit check in v5 to address Jason's
feedback, and to make the
failure explicit rather than relying on `sg_alloc_table()` failing
silently on `nents=0`.

Jason, do you have a strong preference here? I am happy to drop the
hunk and rely on
`sg_alloc_table()` returning `-EINVAL` if you are both comfortable with that.

Thanks,
David