Re: [PATCH rdma-rc v1] RDMA/core: fix sg_to_page mapping for boundary condition

[Jason Gunthorpe <jgg@nvidia.com>]

On Mon, Aug 22, 2022 at 07:51:22PM +0530, Kashyap Desai wrote:

> Now, we will enter into below loop with dma_addr = page_addr =
> 0xffffffffffffe000 and "end_dma_addr = dma_addr + dma_len" is ZERO.
> eval 0xffffffffffffe000 + 8192
> hexadecimal: 0

This is called overflow.

Anything doing maths on the sgl's is likely to become broken by this -
which is why I think it is unnecessarily dangerous for the iommu code
to general dma addresses like this. It just shouldn't.

> > It should not create mappings that are so dangerous. There is really no
> reason to
> > use the last page of IOVA space that includes -1.
> 
> That is correct, but if API which deals with mapping they handle this kind
> of request gracefully is needed. Right ?

Ideally, but that is a game of wack a mole across the kernel, and
redoing algorithms to avoid overflowing addition is tricky stuff.

> I thought about better approach without creating regression and I found
> having loop using sg_dma_len can avoid such issues gracefully.
> How about original patch. ?

It overflows too.

You need to write the code so you never create the situation where
A+B=0 - don't try to fix things up after that happens.

Usually that means transforming the algorithm so that it works on a
"last byte" so we never need to compute an address that is +1 to the
last byte, which would be the overflown 0.

> Above revert is part of my test. In my setup "iommu_dma_forcedac = $2 =
> false".

So you opt into this behavior, OK

Jason