* [PATCH rdma-next 0/3] irdma coverity fixes
@ 2021-06-22 17:52 Tatyana Nikolova
2021-06-22 17:52 ` [PATCH rdma-next 1/3] RDMA/irdma: Check contents of user-space irdma_mem_reg_req object Tatyana Nikolova
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Tatyana Nikolova @ 2021-06-22 17:52 UTC (permalink / raw)
To: jgg, dledford; +Cc: linux-rdma, shiraz.saleem, mustafa.ismail, Tatyana Nikolova
This is a short series of coverity fixes for irdma.
Shiraz Saleem (3):
RDMA/irdma: Check contents of user-space irdma_mem_reg_req object
RDMA/irdma: Check return value from ib_umem_find_best_pgsz
RDMA/irdma: Fix potential overflow expression in irdma_prm_get_pbles
drivers/infiniband/hw/irdma/pble.h | 2 +-
drivers/infiniband/hw/irdma/utils.c | 4 ++--
drivers/infiniband/hw/irdma/verbs.c | 26 +++++++++++++++++++++-----
3 files changed, 24 insertions(+), 8 deletions(-)
--
2.27.0
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH rdma-next 1/3] RDMA/irdma: Check contents of user-space irdma_mem_reg_req object
2021-06-22 17:52 [PATCH rdma-next 0/3] irdma coverity fixes Tatyana Nikolova
@ 2021-06-22 17:52 ` Tatyana Nikolova
2021-06-22 17:58 ` Jason Gunthorpe
2021-06-22 17:52 ` [PATCH rdma-next 2/3] RDMA/irdma: Check return value from ib_umem_find_best_pgsz Tatyana Nikolova
2021-06-22 17:52 ` [PATCH rdma-next 3/3] RDMA/irdma: Fix potential overflow expression in irdma_prm_get_pbles Tatyana Nikolova
2 siblings, 1 reply; 9+ messages in thread
From: Tatyana Nikolova @ 2021-06-22 17:52 UTC (permalink / raw)
To: jgg, dledford
Cc: linux-rdma, shiraz.saleem, mustafa.ismail, coverity-bot,
Tatyana Nikolova
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
--- a/drivers/infiniband/hw/irdma/verbs.c
+++ 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) {
+ err = -EINVAL;
+ goto error;
+ }
use_pbles = ((req.sq_pages + req.rq_pages) > 2);
err = irdma_handle_q_mem(iwdev, &req, iwpbl, use_pbles);
if (err)
@@ -2808,6 +2811,13 @@ static struct ib_mr *irdma_reg_user_mr(struct ib_pd *pd, u64 start, u64 len,
spin_unlock_irqrestore(&ucontext->qp_reg_mem_list_lock, flags);
break;
case IRDMA_MEMREG_TYPE_CQ:
+ if (iwdev->rf->sc_dev.hw_attrs.uk_attrs.feature_flags & IRDMA_FEATURE_CQ_RESIZE)
+ shadow_pgcnt = 0;
+ if (req.cq_pages + shadow_pgcnt > iwmr->page_cnt) {
+ err = -EINVAL;
+ goto error;
+ }
+
use_pbles = (req.cq_pages > 1);
err = irdma_handle_q_mem(iwdev, &req, iwpbl, use_pbles);
if (err)
--
2.27.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH rdma-next 2/3] RDMA/irdma: Check return value from ib_umem_find_best_pgsz
2021-06-22 17:52 [PATCH rdma-next 0/3] irdma coverity fixes Tatyana Nikolova
2021-06-22 17:52 ` [PATCH rdma-next 1/3] RDMA/irdma: Check contents of user-space irdma_mem_reg_req object Tatyana Nikolova
@ 2021-06-22 17:52 ` Tatyana Nikolova
2021-06-22 18:28 ` Jason Gunthorpe
2021-06-22 17:52 ` [PATCH rdma-next 3/3] RDMA/irdma: Fix potential overflow expression in irdma_prm_get_pbles Tatyana Nikolova
2 siblings, 1 reply; 9+ messages in thread
From: Tatyana Nikolova @ 2021-06-22 17:52 UTC (permalink / raw)
To: jgg, dledford
Cc: linux-rdma, shiraz.saleem, mustafa.ismail, coverity-bot,
Tatyana Nikolova
From: Shiraz Saleem <shiraz.saleem@intel.com>
iwmr->page_size stores the return from ib_umem_find_best_pgsz
and maybe zero when used in ib_umem_num_dma_blocks thus causing
a divide by zero error.
Fix this by erroring out of irdma_reg_user when 0 is returned
from ib_umem_find_best_pgsz.
Reported-by: coverity-bot <keescook+coverity-bot@chromium.org>
Addresses-Coverity-ID: 1505149 ("Integer handling issues")
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 | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/infiniband/hw/irdma/verbs.c b/drivers/infiniband/hw/irdma/verbs.c
index 8bd31656a83a..2c4f67fac360 100644
--- a/drivers/infiniband/hw/irdma/verbs.c
+++ b/drivers/infiniband/hw/irdma/verbs.c
@@ -2782,10 +2782,16 @@ static struct ib_mr *irdma_reg_user_mr(struct ib_pd *pd, u64 start, u64 len,
iwmr->ibmr.iova = virt;
iwmr->page_size = PAGE_SIZE;
- if (req.reg_type == IRDMA_MEMREG_TYPE_MEM)
+ if (req.reg_type == IRDMA_MEMREG_TYPE_MEM) {
iwmr->page_size = ib_umem_find_best_pgsz(region,
SZ_4K | SZ_2M | SZ_1G,
virt);
+ if (unlikely(!iwmr->page_size)) {
+ kfree(iwmr);
+ ib_umem_release(region);
+ return ERR_PTR(-EOPNOTSUPP);
+ }
+ }
iwmr->len = region->length;
iwpbl->user_base = virt;
palloc = &iwpbl->pble_alloc;
--
2.27.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH rdma-next 3/3] RDMA/irdma: Fix potential overflow expression in irdma_prm_get_pbles
2021-06-22 17:52 [PATCH rdma-next 0/3] irdma coverity fixes Tatyana Nikolova
2021-06-22 17:52 ` [PATCH rdma-next 1/3] RDMA/irdma: Check contents of user-space irdma_mem_reg_req object Tatyana Nikolova
2021-06-22 17:52 ` [PATCH rdma-next 2/3] RDMA/irdma: Check return value from ib_umem_find_best_pgsz Tatyana Nikolova
@ 2021-06-22 17:52 ` Tatyana Nikolova
2021-06-22 18:07 ` Jason Gunthorpe
2 siblings, 1 reply; 9+ messages in thread
From: Tatyana Nikolova @ 2021-06-22 17:52 UTC (permalink / raw)
To: jgg, dledford
Cc: linux-rdma, shiraz.saleem, mustafa.ismail, coverity-bot,
Tatyana Nikolova
From: Shiraz Saleem <shiraz.saleem@intel.com>
Coverity reports a signed 32-bit overflow on "1 << pprm->pble_shift" when
used expression to compute bits_needed that expects 64bit, unsigned.
Fix this by using the 1ULL in the left shift operator and convert
mem_size to u64.
Reported-by: coverity-bot <keescook+coverity-bot@chromium.org>
Addresses-Coverity-ID: 1505157 ("Integer handling issues")
Fixes: 915cc7ac0f8e ("RDMA/irdma: Add miscellaneous utility definitions")
Signed-off-by: Shiraz Saleem <shiraz.saleem@intel.com>
Signed-off-by: Tatyana Nikolova <tatyana.e.nikolova@intel.com>
---
drivers/infiniband/hw/irdma/pble.h | 2 +-
drivers/infiniband/hw/irdma/utils.c | 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/infiniband/hw/irdma/pble.h b/drivers/infiniband/hw/irdma/pble.h
index e4e635dc4fd9..e1b3b8118a2c 100644
--- a/drivers/infiniband/hw/irdma/pble.h
+++ b/drivers/infiniband/hw/irdma/pble.h
@@ -121,7 +121,7 @@ enum irdma_status_code irdma_prm_add_pble_mem(struct irdma_pble_prm *pprm,
struct irdma_chunk *pchunk);
enum irdma_status_code
irdma_prm_get_pbles(struct irdma_pble_prm *pprm,
- struct irdma_pble_chunkinfo *chunkinfo, u32 mem_size,
+ struct irdma_pble_chunkinfo *chunkinfo, u64 mem_size,
u64 **vaddr, u64 *fpm_addr);
void irdma_prm_return_pbles(struct irdma_pble_prm *pprm,
struct irdma_pble_chunkinfo *chunkinfo);
diff --git a/drivers/infiniband/hw/irdma/utils.c b/drivers/infiniband/hw/irdma/utils.c
index ea1df5918c11..e50b6f89b37e 100644
--- a/drivers/infiniband/hw/irdma/utils.c
+++ b/drivers/infiniband/hw/irdma/utils.c
@@ -2314,7 +2314,7 @@ enum irdma_status_code irdma_prm_add_pble_mem(struct irdma_pble_prm *pprm,
*/
enum irdma_status_code
irdma_prm_get_pbles(struct irdma_pble_prm *pprm,
- struct irdma_pble_chunkinfo *chunkinfo, u32 mem_size,
+ struct irdma_pble_chunkinfo *chunkinfo, u64 mem_size,
u64 **vaddr, u64 *fpm_addr)
{
u64 bits_needed;
@@ -2326,7 +2326,7 @@ irdma_prm_get_pbles(struct irdma_pble_prm *pprm,
*vaddr = NULL;
*fpm_addr = 0;
- bits_needed = (mem_size + (1 << pprm->pble_shift) - 1) >> pprm->pble_shift;
+ bits_needed = (mem_size + BIT_ULL(pprm->pble_shift) - 1) >> pprm->pble_shift;
spin_lock_irqsave(&pprm->prm_lock, flags);
while (chunk_entry != &pprm->clist) {
--
2.27.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH rdma-next 1/3] RDMA/irdma: Check contents of user-space irdma_mem_reg_req object
2021-06-22 17:52 ` [PATCH rdma-next 1/3] RDMA/irdma: Check contents of user-space irdma_mem_reg_req object Tatyana Nikolova
@ 2021-06-22 17:58 ` Jason Gunthorpe
2021-06-22 21:56 ` Nikolova, Tatyana E
0 siblings, 1 reply; 9+ messages in thread
From: Jason Gunthorpe @ 2021-06-22 17:58 UTC (permalink / raw)
To: Tatyana Nikolova
Cc: dledford, linux-rdma, shiraz.saleem, mustafa.ismail, coverity-bot
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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH rdma-next 3/3] RDMA/irdma: Fix potential overflow expression in irdma_prm_get_pbles
2021-06-22 17:52 ` [PATCH rdma-next 3/3] RDMA/irdma: Fix potential overflow expression in irdma_prm_get_pbles Tatyana Nikolova
@ 2021-06-22 18:07 ` Jason Gunthorpe
0 siblings, 0 replies; 9+ messages in thread
From: Jason Gunthorpe @ 2021-06-22 18:07 UTC (permalink / raw)
To: Tatyana Nikolova
Cc: dledford, linux-rdma, shiraz.saleem, mustafa.ismail, coverity-bot
On Tue, Jun 22, 2021 at 12:52:32PM -0500, Tatyana Nikolova wrote:
> From: Shiraz Saleem <shiraz.saleem@intel.com>
>
> Coverity reports a signed 32-bit overflow on "1 << pprm->pble_shift" when
> used expression to compute bits_needed that expects 64bit, unsigned.
>
> Fix this by using the 1ULL in the left shift operator and convert
> mem_size to u64.
>
> Reported-by: coverity-bot <keescook+coverity-bot@chromium.org>
> Addresses-Coverity-ID: 1505157 ("Integer handling issues")
> Fixes: 915cc7ac0f8e ("RDMA/irdma: Add miscellaneous utility definitions")
> Signed-off-by: Shiraz Saleem <shiraz.saleem@intel.com>
> Signed-off-by: Tatyana Nikolova <tatyana.e.nikolova@intel.com>
> drivers/infiniband/hw/irdma/pble.h | 2 +-
> drivers/infiniband/hw/irdma/utils.c | 4 ++--
> 2 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/infiniband/hw/irdma/pble.h b/drivers/infiniband/hw/irdma/pble.h
> index e4e635dc4fd9..e1b3b8118a2c 100644
> +++ b/drivers/infiniband/hw/irdma/pble.h
> @@ -121,7 +121,7 @@ enum irdma_status_code irdma_prm_add_pble_mem(struct irdma_pble_prm *pprm,
> struct irdma_chunk *pchunk);
> enum irdma_status_code
> irdma_prm_get_pbles(struct irdma_pble_prm *pprm,
> - struct irdma_pble_chunkinfo *chunkinfo, u32 mem_size,
> + struct irdma_pble_chunkinfo *chunkinfo, u64 mem_size,
> u64 **vaddr, u64 *fpm_addr);
> void irdma_prm_return_pbles(struct irdma_pble_prm *pprm,
> struct irdma_pble_chunkinfo *chunkinfo);
> diff --git a/drivers/infiniband/hw/irdma/utils.c b/drivers/infiniband/hw/irdma/utils.c
> index ea1df5918c11..e50b6f89b37e 100644
> +++ b/drivers/infiniband/hw/irdma/utils.c
> @@ -2314,7 +2314,7 @@ enum irdma_status_code irdma_prm_add_pble_mem(struct irdma_pble_prm *pprm,
> */
> enum irdma_status_code
> irdma_prm_get_pbles(struct irdma_pble_prm *pprm,
> - struct irdma_pble_chunkinfo *chunkinfo, u32 mem_size,
> + struct irdma_pble_chunkinfo *chunkinfo, u64 mem_size,
> u64 **vaddr, u64 *fpm_addr)
> {
> u64 bits_needed;
> @@ -2326,7 +2326,7 @@ irdma_prm_get_pbles(struct irdma_pble_prm *pprm,
> *vaddr = NULL;
> *fpm_addr = 0;
>
> - bits_needed = (mem_size + (1 << pprm->pble_shift) - 1) >> pprm->pble_shift;
> + bits_needed = (mem_size + BIT_ULL(pprm->pble_shift) - 1) >> pprm->pble_shift;
Isn't this just DIV_ROUND_UP_ULL() ?
Jason
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH rdma-next 2/3] RDMA/irdma: Check return value from ib_umem_find_best_pgsz
2021-06-22 17:52 ` [PATCH rdma-next 2/3] RDMA/irdma: Check return value from ib_umem_find_best_pgsz Tatyana Nikolova
@ 2021-06-22 18:28 ` Jason Gunthorpe
0 siblings, 0 replies; 9+ messages in thread
From: Jason Gunthorpe @ 2021-06-22 18:28 UTC (permalink / raw)
To: Tatyana Nikolova
Cc: dledford, linux-rdma, shiraz.saleem, mustafa.ismail, coverity-bot
On Tue, Jun 22, 2021 at 12:52:31PM -0500, Tatyana Nikolova wrote:
> From: Shiraz Saleem <shiraz.saleem@intel.com>
>
> iwmr->page_size stores the return from ib_umem_find_best_pgsz
> and maybe zero when used in ib_umem_num_dma_blocks thus causing
> a divide by zero error.
>
> Fix this by erroring out of irdma_reg_user when 0 is returned
> from ib_umem_find_best_pgsz.
>
> Reported-by: coverity-bot <keescook+coverity-bot@chromium.org>
> Addresses-Coverity-ID: 1505149 ("Integer handling issues")
> 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 | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
This patch applied to for-next, thanks
Jason
^ permalink raw reply [flat|nested] 9+ messages in thread
* RE: [PATCH rdma-next 1/3] RDMA/irdma: Check contents of user-space irdma_mem_reg_req object
2021-06-22 17:58 ` Jason Gunthorpe
@ 2021-06-22 21:56 ` Nikolova, Tatyana E
2021-06-22 23:33 ` Jason Gunthorpe
0 siblings, 1 reply; 9+ messages in thread
From: Nikolova, Tatyana E @ 2021-06-22 21:56 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: dledford, linux-rdma, Saleem, Shiraz, Ismail, Mustafa, coverity-bot
> -----Original Message-----
> From: Jason Gunthorpe <jgg@nvidia.com>
> Sent: Tuesday, June 22, 2021 12:59 PM
> To: Nikolova, Tatyana E <tatyana.e.nikolova@intel.com>
> Cc: dledford@redhat.com; linux-rdma@vger.kernel.org; Saleem, Shiraz
> <shiraz.saleem@intel.com>; Ismail, Mustafa <mustafa.ismail@intel.com>;
> coverity-bot <keescook+coverity-bot@chromium.org>
> Subject: Re: [PATCH rdma-next 1/3] RDMA/irdma: Check contents of user-
> space irdma_mem_reg_req object
>
> 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
>
Hi Jason,
The mem_reg_req fields sq_pages and rq_pages are u16 and the variable shadow_pgcnt is u8. They should be promoted to u32 when compared with iwmr->page_cnt which is u32. Isn't this overflow safe?
Is the issue you are mentioning about this line:
> > + qpmr->shadow = (dma_addr_t)arr[req->sq_pages + req-
> >rq_pages];
Thank you,
Tatyana
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH rdma-next 1/3] RDMA/irdma: Check contents of user-space irdma_mem_reg_req object
2021-06-22 21:56 ` Nikolova, Tatyana E
@ 2021-06-22 23:33 ` Jason Gunthorpe
0 siblings, 0 replies; 9+ messages in thread
From: Jason Gunthorpe @ 2021-06-22 23:33 UTC (permalink / raw)
To: Nikolova, Tatyana E
Cc: dledford, linux-rdma, Saleem, Shiraz, Ismail, Mustafa, coverity-bot
On Tue, Jun 22, 2021 at 09:56:42PM +0000, Nikolova, Tatyana E wrote:
> > > 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
>
> The mem_reg_req fields sq_pages and rq_pages are u16 and the
> variable shadow_pgcnt is u8. They should be promoted to u32 when
> compared with iwmr->page_cnt which is u32. Isn't this overflow safe?
I didn't check the sizes carefully, and I'm always nervous about
relying on implicit promotion for security properties as it is so
subtle and easy to get screwed up during maintenance
> Is the issue you are mentioning about this line:
> > > + qpmr->shadow = (dma_addr_t)arr[req->sq_pages + req->rq_pages];
I assume this is safe because of the if above?
Jason
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2021-06-22 23:33 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-22 17:52 [PATCH rdma-next 0/3] irdma coverity fixes Tatyana Nikolova
2021-06-22 17:52 ` [PATCH rdma-next 1/3] RDMA/irdma: Check contents of user-space irdma_mem_reg_req object Tatyana Nikolova
2021-06-22 17:58 ` Jason Gunthorpe
2021-06-22 21:56 ` Nikolova, Tatyana E
2021-06-22 23:33 ` Jason Gunthorpe
2021-06-22 17:52 ` [PATCH rdma-next 2/3] RDMA/irdma: Check return value from ib_umem_find_best_pgsz Tatyana Nikolova
2021-06-22 18:28 ` Jason Gunthorpe
2021-06-22 17:52 ` [PATCH rdma-next 3/3] RDMA/irdma: Fix potential overflow expression in irdma_prm_get_pbles Tatyana Nikolova
2021-06-22 18:07 ` Jason Gunthorpe
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.