Re: [PATCH rdma-next 17/23] RDMA/mlx5: Validate EQE port and GSI port before indexing devr.ports[]

[Yishai Hadas <yishaih@nvidia.com>]

On 07/11/2026 22:25, netdev-bot+sashiko@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found more severe
> issue(s) in other patches of this series. This mail is only an FYI, in case
> you want to address the nit picks below when you respin.
> 
> Low priority / nit picks for your consideration:
> 
> - [Low] This patch makes the existing invalid-port check and warning in
>    mlx5_ib_handle_event() (drivers/infiniband/hw/mlx5/main.c)…
> 
> --
> 
> RDMA/mlx5: Validate EQE port and GSI port before indexing devr.ports[]
> 
> This commit adds port validation to handle_port_change() and
> mlx5_ib_create_gsi(). The port number from the NIC is checked with
> rdma_is_port_valid() and limited to ARRAY_SIZE(devr.ports) before it is
> used to index devr.ports[].
> 
>> diff --git a/drivers/infiniband/hw/mlx5/main.c b/drivers/infiniband/hw/mlx5/main.c
>> index f4f2ad80e76c4..c29102260989f 100644
>> --- a/drivers/infiniband/hw/mlx5/main.c
>> +++ b/drivers/infiniband/hw/mlx5/main.c
>> @@ -3063,6 +3063,10 @@ static int handle_port_change(struct mlx5_ib_dev *ibdev, struct mlx5_eqe *eqe,
>>   {
>>   	u32 port = (eqe->data.port.port >> 4) & 0xf;
>>   
>> +	if (!rdma_is_port_valid(&ibdev->ib_dev, port) ||
>> +	    port > ARRAY_SIZE(ibdev->devr.ports))
>> +		return -EINVAL;
>> +
> 
> [Severity: Low]
> Does this early return make the port check and warning in
> mlx5_ib_handle_event() unreachable?
> 
> mlx5_ib_handle_event() treats any non-zero return from
> handle_port_change() as a reason to skip the event:
> 
> drivers/infiniband/hw/mlx5/main.c:mlx5_ib_handle_event() {
>      ...
> 	case MLX5_EVENT_TYPE_PORT_CHANGE:
> 		if (handle_port_change(ibdev, work->param, &ibev))
> 			goto out;
> 		break;
> 	case MLX5_EVENT_TYPE_GENERAL_EVENT:
> 		handle_general_event(ibdev, work->param, &ibev);
> 		fallthrough;
> 	default:
> 		goto out;
> 	}
> 
> 	ibev.device = &ibdev->ib_dev;
> 
> 	if (!rdma_is_port_valid(&ibdev->ib_dev, ibev.element.port_num)) {
> 		mlx5_ib_warn(ibdev, "warning: event on port %d\n",  ibev.element.port_num);
> 		goto out;
> 	}
>      ...
> }
> 
> The only way to reach the rdma_is_port_valid() check is a successful
> handle_port_change(). After this patch, a successful return always means
> the port is valid.
> 
> Before the patch, a port change EQE with port 0 or a port above
> phys_port_cnt went through handle_port_change() and returned 0 if its
> sub_type was not PKEY (LID, GUID, CLIENT_REREG, or ACTIVE/DOWN on IB).
> The caller's check then printed the "warning: event on port %d" message.
> 
> After the patch, the same event is dropped with no log message.
> 
> Could the warning be moved into the new early-return path in
> handle_port_change()? Or, if the silent drop is intended, could the
> caller's check be removed, since it is now dead code?
> 
>> +
>>   	ibev->element.port_num = port;
>>   
>>   	switch (eqe->sub_type) {
> 

Right, V1 will move the warning into handle_port_change() and drop the 
caller's check.

Yishai