[PATCH next] RDMA/mana_ib: Use safer allocation function()

[PATCH next] RDMA/mana_ib: Use safer allocation function()

[Dan Carpenter]

My static checker says this multiplication can overflow.  I'm not an
expert in this code but the call tree would be:

ib_uverbs_handler_UVERBS_METHOD_QP_CREATE() <- reads cap from the user
-> ib_create_qp_user()
   -> create_qp()
      -> mana_ib_create_qp()
         -> mana_ib_create_ud_qp()
            -> create_shadow_queue()

It can't hurt to use safer interfaces.

Cc: stable@vger.kernel.org
Fixes: c8017f5b4856 ("RDMA/mana_ib: UD/GSI work requests")
Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
---
There seems to be another integer overflow bug in mana_ib_queue_size() as
well?  It's basically the exact same issue.  Maybe we could put a cap on
attr->cap.max_send/recv_wr at a lower level.  Maybe there already is some
bounds checking that I have missed...

 drivers/infiniband/hw/mana/shadow_queue.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/infiniband/hw/mana/shadow_queue.h b/drivers/infiniband/hw/mana/shadow_queue.h
index d8bfb4c712d5..a4b3818f9c39 100644
--- a/drivers/infiniband/hw/mana/shadow_queue.h
+++ b/drivers/infiniband/hw/mana/shadow_queue.h
@@ -40,7 +40,7 @@ struct shadow_queue {
 
 static inline int create_shadow_queue(struct shadow_queue *queue, uint32_t length, uint32_t stride)
 {
-	queue->buffer = kvmalloc(length * stride, GFP_KERNEL);
+	queue->buffer = kvmalloc_array(length, stride, GFP_KERNEL);
 	if (!queue->buffer)
 		return -ENOMEM;
 
-- 
2.47.2

RE: [EXTERNAL] [PATCH next] RDMA/mana_ib: Use safer allocation function()

[Long Li]

> Subject: [EXTERNAL] [PATCH next] RDMA/mana_ib: Use safer allocation
> function()
> 
> My static checker says this multiplication can overflow.  I'm not an expert in this
> code but the call tree would be:
> 
> ib_uverbs_handler_UVERBS_METHOD_QP_CREATE() <- reads cap from the user
> -> ib_create_qp_user()
>    -> create_qp()
>       -> mana_ib_create_qp()
>          -> mana_ib_create_ud_qp()
>             -> create_shadow_queue()
> 
> It can't hurt to use safer interfaces.
> 
> Cc: stable@vger.kernel.org
> Fixes: c8017f5b4856 ("RDMA/mana_ib: UD/GSI work requests")
> Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>

Reviewed-by: Long Li <longli@microsoft.com>

> ---
> There seems to be another integer overflow bug in mana_ib_queue_size() as
> well?  It's basically the exact same issue.  Maybe we could put a cap on
> attr->cap.max_send/recv_wr at a lower level.  Maybe there already is
> attr->some
> bounds checking that I have missed...
> 
>  drivers/infiniband/hw/mana/shadow_queue.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/infiniband/hw/mana/shadow_queue.h
> b/drivers/infiniband/hw/mana/shadow_queue.h
> index d8bfb4c712d5..a4b3818f9c39 100644
> --- a/drivers/infiniband/hw/mana/shadow_queue.h
> +++ b/drivers/infiniband/hw/mana/shadow_queue.h
> @@ -40,7 +40,7 @@ struct shadow_queue {
> 
>  static inline int create_shadow_queue(struct shadow_queue *queue, uint32_t
> length, uint32_t stride)  {
> -	queue->buffer = kvmalloc(length * stride, GFP_KERNEL);
> +	queue->buffer = kvmalloc_array(length, stride, GFP_KERNEL);
>  	if (!queue->buffer)
>  		return -ENOMEM;
> 
> --
> 2.47.2

Re: [PATCH next] RDMA/mana_ib: Use safer allocation function()

[Leon Romanovsky]

On Thu, 06 Mar 2025 22:49:06 +0300, Dan Carpenter wrote:
> My static checker says this multiplication can overflow.  I'm not an
> expert in this code but the call tree would be:
> 
> ib_uverbs_handler_UVERBS_METHOD_QP_CREATE() <- reads cap from the user
> -> ib_create_qp_user()
>    -> create_qp()
>       -> mana_ib_create_qp()
>          -> mana_ib_create_ud_qp()
>             -> create_shadow_queue()
> 
> [...]

Applied, thanks!

[1/1] RDMA/mana_ib: Use safer allocation function()
      https://git.kernel.org/rdma/rdma/c/1d5c69514e7428

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