RE: [PATCH] IB/CM: fix memory corruption by avoiding unnecessary memset

[Parav Pandit]

Hi Qing,

> -----Original Message-----
> From: linux-rdma-owner@xxxxxxxxxxxxxxx [mailto:linux-rdma-
> owner@xxxxxxxxxxxxxxx] On Behalf Of Qing Huang
> Sent: Thursday, November 02, 2017 6:22 PM
> To: linux-rdma@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx
> Cc: dledford@xxxxxxxxxx; sean.hefty@xxxxxxxxx; hal.rosenstock@xxxxxxxxx;
> ira.weiny@xxxxxxxxx; Mark Bloch <markb@xxxxxxxxxxxx>; Qing Huang
> <qing.huang@xxxxxxxxxx>
> Subject: [PATCH] IB/CM: fix memory corruption by avoiding unnecessary
> memset
> 
> The size of path array could be dynamic. However the fixed number(2) of
> memset could cause memory corruption by writing into wrong memory space.
> 
> Fixes: 9fdca4da4d8c (IB/SA: Split struct sa_path_rec based on IB ands
> 	ROCE specific fields)
> 
> Signed-off-by: Qing Huang <qing.huang@xxxxxxxxxx>
> ---
>  drivers/infiniband/core/cm.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/infiniband/core/cm.c b/drivers/infiniband/core/cm.c index
> 4c4b465..af4f6a0 100644
> --- a/drivers/infiniband/core/cm.c
> +++ b/drivers/infiniband/core/cm.c
> @@ -1856,7 +1856,9 @@ static int cm_req_handler(struct cm_work *work)
>  	cm_process_routed_req(req_msg, work->mad_recv_wc->wc);
> 
>  	memset(&work->path[0], 0, sizeof(work->path[0]));
> -	memset(&work->path[1], 0, sizeof(work->path[1]));
> +	if (cm_req_has_alt_path(req_msg))
> +		memset(&work->path[1], 0, sizeof(work->path[1]));
> +
>  	grh = rdma_ah_read_grh(&cm_id_priv->av.ah_attr);
>  	ret = ib_get_cached_gid(work->port->cm_dev->ib_device,
>  				work->port->port_num,
> @@ -3823,8 +3825,8 @@ static void cm_recv_handler(struct ib_mad_agent
> *mad_agent,
> 
>  	switch (mad_recv_wc->recv_buf.mad->mad_hdr.attr_id) {
>  	case CM_REQ_ATTR_ID:
> -		paths = 1 + (((struct cm_req_msg *) mad_recv_wc-
> >recv_buf.mad)->
> -						    alt_local_lid != 0);
> +		paths = 1 + cm_req_has_alt_path(
> +				(struct cm_req_msg *)mad_recv_wc-
> >recv_buf.mad);
>  		event = IB_CM_REQ_RECEIVED;
>  		break;
>  	case CM_MRA_ATTR_ID:
> --
> 2.9.3
> 
Thanks for the patch. Few weeks back I came across this bug and fix [1] is merged now by Doug.
[1] has one additional fix in cm_format_req_event() function as well.

[1] https://patchwork.kernel.org/patch/10015997/