Hi Andi,
On 2026-09-11 at 00:21:15 +0200, Andi Shyti wrote:
> Hi Krzysztof,
>
> ...
>
> > @@ -1477,6 +1477,10 @@ int iommu_dma_map_sg(struct device *dev, struct
> > scatterlist *sg, int nents,
> > sg_dma_len(s) = s_length;
> > s->offset -= s_iova_off;
> > s_length = iova_align(iovad, s_length + s_iova_off);
> > + if (overflows_type(s_length, s->length)) {
> > + ret = -EOVERFLOW;
>
> the value of ret here doesn't really matter because it returns
> anyway an -EINVAL. We can completely remove this assignment.
Assigning an error to ret here lets us return that EINVAL.
Without the assignment, we'd be checking against an
uninitialized variable. I'd rather have this temporary error set
than use garbage value.
>
> > + goto out_restore_sg;
> > + }
> > s->length = s_length;
> >
> > /*
> > @@ -1493,7 +1497,18 @@ int iommu_dma_map_sg(struct device *dev, struct
> > scatterlist *sg, int nents,
> > * time through here (i.e. before it has a meaningful value).
> > */
> > if (pad_len && pad_len < s_length - 1) {
> > - prev->length += pad_len;
> > + unsigned int new_prev_len;
>
> Please add a blank line here.
>
> > + /*
> > + * For large mappings spanning multiple GBs we
> > + * may not be able to fit all needed padding into
> > + * sg->length.
> > + */
> > + if (check_add_overflow(prev->length, pad_len,
> > &new_prev_len)) {
> > + ret = -EOVERFLOW;
>
> same goes here.
>
> Thanks,
> Andi
>
> > + goto out_restore_sg;
> > + }
--
Best Regards,
Krzysztof
