On Thu, Feb 13, 2025 at 04:30:11PM +0200, Margolin, Michael wrote:
>
> On 2/13/2025 4:04 PM, Jason Gunthorpe wrote:
> > CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you can confirm the sender and know the content is safe.
> >
> >
> >
> > On Thu, Feb 13, 2025 at 02:51:26PM +0200, Leon Romanovsky wrote:
> > > diff --git a/drivers/infiniband/core/umem.c b/drivers/infiniband/core/umem.c
> > > index e7e428369159..63a92d6cfbc2 100644
> > > --- a/drivers/infiniband/core/umem.c
> > > +++ b/drivers/infiniband/core/umem.c
> > > @@ -112,8 +112,7 @@ unsigned long ib_umem_find_best_pgsz(struct ib_umem *umem,
> > > /* If the current entry is physically contiguous with the previous
> > > * one, no need to take its start addresses into consideration.
> > > */
> > > - if (curr_base + curr_len != sg_dma_address(sg)) {
> > > -
> > > + if (curr_base != sg_dma_address(sg) - curr_len) {
> > > curr_base = sg_dma_address(sg);
> > > curr_len = 0;
> > I'm not sure about this, what ensures sg_dma_address() > curr_len?
> >
> > curr_base + curr_len could also overflow, we've seen that AMD IOMMU
> > sometimes uses the very high addresess already
>
> I think the only case we care about where curr_base + curr_len can overflow
> is when next sg_dma_address() == 0.
>
> But maybe we should just add an explicit check:
>
> - if (curr_base + curr_len != sg_dma_address(sg)) {
> + if (curr_base + curr_len < curr_base ||
> + curr_base + curr_len != sg_dma_address(sg)) {
> curr_base = sg_dma_address(sg);
> curr_len = 0;
Ugh
I wonder if we should try to make a overflow.h helper for these kinds
of problems.
/* Check if a + n == b, failing if a+n overflows */
check_consecutive(a, n, b)
?
It is a fairly common problem
I suggest to take the patch as it originally was and try to propose
the above helper?
Jason
