lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID:
 <SA6PR21MB4231DCD3836F24ED596C0B62CECA2@SA6PR21MB4231.namprd21.prod.outlook.com>
Date: Thu, 6 Mar 2025 20:04:22 +0000
From: Long Li <longli@...rosoft.com>
To: Dan Carpenter <dan.carpenter@...aro.org>, Konstantin Taranov
	<kotaranov@...rosoft.com>
CC: Jason Gunthorpe <jgg@...pe.ca>, Leon Romanovsky <leon@...nel.org>, Shiraz
 Saleem <shirazsaleem@...rosoft.com>, "linux-rdma@...r.kernel.org"
	<linux-rdma@...r.kernel.org>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>, "kernel-janitors@...r.kernel.org"
	<kernel-janitors@...r.kernel.org>
Subject: RE: [EXTERNAL] [PATCH next] RDMA/mana_ib: Use safer allocation
 function()

> 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@...r.kernel.org
> Fixes: c8017f5b4856 ("RDMA/mana_ib: UD/GSI work requests")
> Signed-off-by: Dan Carpenter <dan.carpenter@...aro.org>

Reviewed-by: Long Li <longli@...rosoft.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


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ