Re: [PATCH rdma-next 1/3] RDMA/irdma: Check contents of user-space irdma_mem_reg_req object

[Jason Gunthorpe <jgg@nvidia.com>]

On Tue, Jun 22, 2021 at 12:52:30PM -0500, Tatyana Nikolova wrote:
> From: Shiraz Saleem <shiraz.saleem@intel.com>
> 
> The contents of user-space req object is used in array indexing
> in irdma_handle_q_mem without checking for valid values.
> 
> Guard against bad input on each of these req object pages by
> limiting them to number of pages that make up the region.
> 
> Reported-by: coverity-bot <keescook+coverity-bot@chromium.org>
> Addresses-Coverity-ID: 1505160 ("TAINTED_SCALAR")
> Fixes: b48c24c2d710 ("RDMA/irdma: Implement device supported verb APIs")
> Signed-off-by: Shiraz Saleem <shiraz.saleem@intel.com>
> Signed-off-by: Tatyana Nikolova <tatyana.e.nikolova@intel.com>
>  drivers/infiniband/hw/irdma/verbs.c | 18 ++++++++++++++----
>  1 file changed, 14 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/infiniband/hw/irdma/verbs.c b/drivers/infiniband/hw/irdma/verbs.c
> index e8b170f0d997..8bd31656a83a 100644
> +++ b/drivers/infiniband/hw/irdma/verbs.c
> @@ -2360,10 +2360,8 @@ static int irdma_handle_q_mem(struct irdma_device *iwdev,
>  	u64 *arr = iwmr->pgaddrmem;
>  	u32 pg_size;
>  	int err = 0;
> -	int total;
>  	bool ret = true;
>  
> -	total = req->sq_pages + req->rq_pages + req->cq_pages;
>  	pg_size = iwmr->page_size;
>  	err = irdma_setup_pbles(iwdev->rf, iwmr, use_pbles);
>  	if (err)
> @@ -2381,7 +2379,7 @@ static int irdma_handle_q_mem(struct irdma_device *iwdev,
>  	switch (iwmr->type) {
>  	case IRDMA_MEMREG_TYPE_QP:
>  		hmc_p = &qpmr->sq_pbl;
> -		qpmr->shadow = (dma_addr_t)arr[total];
> +		qpmr->shadow = (dma_addr_t)arr[req->sq_pages + req->rq_pages];
>  
>  		if (use_pbles) {
>  			ret = irdma_check_mem_contiguous(arr, req->sq_pages,
> @@ -2406,7 +2404,7 @@ static int irdma_handle_q_mem(struct irdma_device *iwdev,
>  		hmc_p = &cqmr->cq_pbl;
>  
>  		if (!cqmr->split)
> -			cqmr->shadow = (dma_addr_t)arr[total];
> +			cqmr->shadow = (dma_addr_t)arr[req->cq_pages];
>  
>  		if (use_pbles)
>  			ret = irdma_check_mem_contiguous(arr, req->cq_pages,
> @@ -2748,6 +2746,7 @@ static struct ib_mr *irdma_reg_user_mr(struct ib_pd *pd, u64 start, u64 len,
>  	struct ib_umem *region;
>  	struct irdma_mem_reg_req req;
>  	u32 stag = 0;
> +	u8 shadow_pgcnt = 1;
>  	bool use_pbles = false;
>  	unsigned long flags;
>  	int err = -EINVAL;
> @@ -2795,6 +2794,10 @@ static struct ib_mr *irdma_reg_user_mr(struct ib_pd *pd, u64 start, u64 len,
>  
>  	switch (req.reg_type) {
>  	case IRDMA_MEMREG_TYPE_QP:
> +		if (req.sq_pages + req.rq_pages + shadow_pgcnt > iwmr->page_cnt) {

Math on values from userspace should use the check overflow helpers or
otherwise be designed to be overflow safe

Jason