>Subject: Re: [PATCH rdma-next 1/5] RDMA/bnxt_re: Use correct sizing on buffers >holding page DMA addresses > >On Mon, Mar 18, 2019 at 08:23:36AM -0500, Shiraz Saleem wrote: >> From: Selvin Xavier <selvin.xavier@xxxxxxxxxxxx> >> >> umem->nmap is used while allocating internal buffer for storing >> page DMA addresses. This causes out of bounds array access while >> iterating the umem DMA-mapped SGL with umem page combining as >> umem->nmap can be less than number of system pages in umem. >> >> Use umem->npages instead of umem->nmap to size the page array. Add a >> new structure (bnxt_qplib_sg_info) to pass sglist, npages and nmap. >> >> Signed-off-by: Selvin Xavier <selvin.xavier@xxxxxxxxxxxx> >> Signed-off-by: Shiraz Saleem <shiraz.saleem@xxxxxxxxx> >> drivers/infiniband/hw/bnxt_re/ib_verbs.c | 25 ++++++++--------- >> drivers/infiniband/hw/bnxt_re/qplib_fp.c | 27 ++++++++++--------- >> drivers/infiniband/hw/bnxt_re/qplib_fp.h | 9 +++---- >> drivers/infiniband/hw/bnxt_re/qplib_rcfw.c | 4 +-- >> drivers/infiniband/hw/bnxt_re/qplib_res.c | 43 >> ++++++++++++++++++------------ drivers/infiniband/hw/bnxt_re/qplib_res.h | 8 >+++++- >> drivers/infiniband/hw/bnxt_re/qplib_sp.c | 4 +-- >> 7 files changed, 67 insertions(+), 53 deletions(-) >> >> diff --git a/drivers/infiniband/hw/bnxt_re/ib_verbs.c >> b/drivers/infiniband/hw/bnxt_re/ib_verbs.c >> index 071b2fc..b69da66 100644 >> +++ b/drivers/infiniband/hw/bnxt_re/ib_verbs.c >> @@ -895,8 +895,9 @@ static int bnxt_re_init_user_qp(struct bnxt_re_dev *rdev, >struct bnxt_re_pd *pd, >> return PTR_ERR(umem); >> >> qp->sumem = umem; >> - qplib_qp->sq.sglist = umem->sg_head.sgl; >> - qplib_qp->sq.nmap = umem->nmap; >> + qplib_qp->sq.sg_info.sglist = umem->sg_head.sgl; >> + qplib_qp->sq.sg_info.npages = umem->npages; > >'umem->npages' should be ib_umem_num_pages(umem) in all patches and then >you should entirely delete npages in 'RDMA/umem: Combine contiguous >PAGE_SIZE regions in SGEs' Ok. But the umem ODP code also updates umem->npages and there is a check in mlx5_ib_invalidate_range(). Do you want keep npages tracking in the ib_umem_odp struct? > >Otherwise this looks fine, please respin and I'll apply it > >Jason
