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

[Jason Gunthorpe <jgg@nvidia.com>]

On Thu, Sep 01, 2022 at 05:36:57PM +0530, Kashyap Desai wrote:
> > -----Original Message-----
> > From: Jason Gunthorpe [mailto:jgg@nvidia.com]
> > Sent: Friday, August 26, 2022 6:45 PM
> > To: Kashyap Desai <kashyap.desai@broadcom.com>
> > Cc: linux-rdma@vger.kernel.org; leonro@nvidia.com; Selvin Xavier
> > <selvin.xavier@broadcom.com>; Andrew Gospodarek
> > <andrew.gospodarek@broadcom.com>
> > Subject: Re: [PATCH rdma-rc v1] RDMA/core: fix sg_to_page mapping for
> > boundary condition
> >
> > 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.
> 
> Is this not DMAable for 64bit DMA mask device ? It is DMAable. So not sure
> why you call it as overflow. ?

Beacuse the normal math overflowed.

Should it work? Yes. Is it a special edge case that might have bugs?
Certainly.

So the IOMMU layer shouldn't be stressing this edge case at all. It is
crazy, there is no reason to do this.

> I agree that such mapping is obviously dangerous, but it is not illegal as
> well.
> Same sgl mapping works if it is direct attached Storage, so there will be
> a logical question why IB stack is not handling this.

Oh that is probably very driver dependent.

> > 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.
> 
> In proposed patch, A + B = 0 is possible, but it will be considered as end
> of the loop.

Like I said, don't do that. End of the loop is -1 which requires a
different loop logic design, so send a patch like that.

But I would still send a patch for iommu to not create this in the
first place.

Jason