[PATCH for-rc v3] RDMA: Fix ib block iterator counter overflow

[PATCH for-rc v3] RDMA: Fix ib block iterator counter overflow

[ynachum]

From: Yonatan Nachum <ynachum@amazon.com>

When registering a new DMA MR after selecting the best aligned page size
for it, we iterate over the given sglist to split each entry to smaller,
aligned to the selected page size, DMA blocks.

In given circumstances where the sg entry and page size fit certain
sizes and the sg entry is not aligned to the selected page size, the
total size of the aligned pages we need to cover the sg entry is >= 4GB.
Under this circumstances, while iterating page aligned blocks, the
counter responsible for counting how much we advanced from the start of
the sg entry is overflowed because its type is u32 and we pass 4GB in
size. This can lead to an infinite loop inside the iterator function
because the overflow prevents the counter to be larger
than the size of the sg entry.

Fix the presented problem by changing the advancement condition to
eliminate overflow.

Backtrace:
[  192.374329] efa_reg_user_mr_dmabuf
[  192.376783] efa_register_mr
[  192.382579] pgsz_bitmap 0xfffff000 rounddown 0x80000000
[  192.386423] pg_sz [0x80000000] umem_length[0xc0000000]
[  192.392657] start 0x0 length 0xc0000000 params.page_shift 31 params.page_num 3
[  192.399559] hp_cnt[3], pages_in_hp[524288]
[  192.403690] umem->sgt_append.sgt.nents[1]
[  192.407905] number entries: [1], pg_bit: [31]
[  192.411397] biter->__sg_nents [1] biter->__sg [0000000008b0c5d8]
[  192.415601] biter->__sg_advance [665837568] sg_dma_len[3221225472]
[  192.419823] biter->__sg_nents [1] biter->__sg [0000000008b0c5d8]
[  192.423976] biter->__sg_advance [2813321216] sg_dma_len[3221225472]
[  192.428243] biter->__sg_nents [1] biter->__sg [0000000008b0c5d8]
[  192.432397] biter->__sg_advance [665837568] sg_dma_len[3221225472]

Fixes: a808273a495c ("RDMA/verbs: Add a DMA iterator to return aligned contiguous memory blocks")
Signed-off-by: Yonatan Nachum <ynachum@amazon.com>
---
 drivers/infiniband/core/verbs.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/infiniband/core/verbs.c b/drivers/infiniband/core/verbs.c
index 26b021f43ba4..11b1c1603aeb 100644
--- a/drivers/infiniband/core/verbs.c
+++ b/drivers/infiniband/core/verbs.c
@@ -2957,15 +2957,18 @@ EXPORT_SYMBOL(__rdma_block_iter_start);
 bool __rdma_block_iter_next(struct ib_block_iter *biter)
 {
 	unsigned int block_offset;
+	unsigned int sg_delta;
 
 	if (!biter->__sg_nents || !biter->__sg)
 		return false;
 
 	biter->__dma_addr = sg_dma_address(biter->__sg) + biter->__sg_advance;
 	block_offset = biter->__dma_addr & (BIT_ULL(biter->__pg_bit) - 1);
-	biter->__sg_advance += BIT_ULL(biter->__pg_bit) - block_offset;
+	sg_delta = BIT_ULL(biter->__pg_bit) - block_offset;
 
-	if (biter->__sg_advance >= sg_dma_len(biter->__sg)) {
+	if (sg_dma_len(biter->__sg) - biter->__sg_advance > sg_delta) {
+		biter->__sg_advance += sg_delta;
+	} else {
 		biter->__sg_advance = 0;
 		biter->__sg = sg_next(biter->__sg);
 		biter->__sg_nents--;
-- 
2.38.1

Re: [PATCH for-rc v3] RDMA: Fix ib block iterator counter overflow

[Jason Gunthorpe]

On Mon, Jan 09, 2023 at 01:37:11PM +0000, ynachum@amazon.com wrote:
> From: Yonatan Nachum <ynachum@amazon.com>
> 
> When registering a new DMA MR after selecting the best aligned page size
> for it, we iterate over the given sglist to split each entry to smaller,
> aligned to the selected page size, DMA blocks.
> 
> In given circumstances where the sg entry and page size fit certain
> sizes and the sg entry is not aligned to the selected page size, the
> total size of the aligned pages we need to cover the sg entry is >= 4GB.
> Under this circumstances, while iterating page aligned blocks, the
> counter responsible for counting how much we advanced from the start of
> the sg entry is overflowed because its type is u32 and we pass 4GB in
> size. This can lead to an infinite loop inside the iterator function
> because the overflow prevents the counter to be larger
> than the size of the sg entry.
> 
> Fix the presented problem by changing the advancement condition to
> eliminate overflow.
> 
> Backtrace:
> [  192.374329] efa_reg_user_mr_dmabuf
> [  192.376783] efa_register_mr
> [  192.382579] pgsz_bitmap 0xfffff000 rounddown 0x80000000
> [  192.386423] pg_sz [0x80000000] umem_length[0xc0000000]
> [  192.392657] start 0x0 length 0xc0000000 params.page_shift 31 params.page_num 3
> [  192.399559] hp_cnt[3], pages_in_hp[524288]
> [  192.403690] umem->sgt_append.sgt.nents[1]
> [  192.407905] number entries: [1], pg_bit: [31]
> [  192.411397] biter->__sg_nents [1] biter->__sg [0000000008b0c5d8]
> [  192.415601] biter->__sg_advance [665837568] sg_dma_len[3221225472]
> [  192.419823] biter->__sg_nents [1] biter->__sg [0000000008b0c5d8]
> [  192.423976] biter->__sg_advance [2813321216] sg_dma_len[3221225472]
> [  192.428243] biter->__sg_nents [1] biter->__sg [0000000008b0c5d8]
> [  192.432397] biter->__sg_advance [665837568] sg_dma_len[3221225472]
> 
> Fixes: a808273a495c ("RDMA/verbs: Add a DMA iterator to return aligned contiguous memory blocks")
> Signed-off-by: Yonatan Nachum <ynachum@amazon.com>
> ---
>  drivers/infiniband/core/verbs.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/infiniband/core/verbs.c b/drivers/infiniband/core/verbs.c
> index 26b021f43ba4..11b1c1603aeb 100644
> --- a/drivers/infiniband/core/verbs.c
> +++ b/drivers/infiniband/core/verbs.c
> @@ -2957,15 +2957,18 @@ EXPORT_SYMBOL(__rdma_block_iter_start);
>  bool __rdma_block_iter_next(struct ib_block_iter *biter)
>  {
>  	unsigned int block_offset;
> +	unsigned int sg_delta;
>  
>  	if (!biter->__sg_nents || !biter->__sg)
>  		return false;
>  
>  	biter->__dma_addr = sg_dma_address(biter->__sg) + biter->__sg_advance;
>  	block_offset = biter->__dma_addr & (BIT_ULL(biter->__pg_bit) - 1);
> -	biter->__sg_advance += BIT_ULL(biter->__pg_bit) - block_offset;
> +	sg_delta = BIT_ULL(biter->__pg_bit) - block_offset;

This shouldn't be BIT_ULL if sg_delta is unsigned int. It looks like
this hasn't been especially careful about supporting > 4G page sizes

But this looks OK otherwise

Jason

Re: [PATCH for-rc v3] RDMA: Fix ib block iterator counter overflow

[Leon Romanovsky]

On Mon, 09 Jan 2023 13:37:11 +0000, ynachum@amazon.com wrote:
> When registering a new DMA MR after selecting the best aligned page size
> for it, we iterate over the given sglist to split each entry to smaller,
> aligned to the selected page size, DMA blocks.
> 
> In given circumstances where the sg entry and page size fit certain
> sizes and the sg entry is not aligned to the selected page size, the
> total size of the aligned pages we need to cover the sg entry is >= 4GB.
> Under this circumstances, while iterating page aligned blocks, the
> counter responsible for counting how much we advanced from the start of
> the sg entry is overflowed because its type is u32 and we pass 4GB in
> size. This can lead to an infinite loop inside the iterator function
> because the overflow prevents the counter to be larger
> than the size of the sg entry.
> 
> [...]

Applied, thanks!

[1/1] RDMA: Fix ib block iterator counter overflow
      https://git.kernel.org/rdma/rdma/c/e71b6f877b4070

Best regards,
-- 
Leon Romanovsky <leon@kernel.org>