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

[netdev-bot+sashiko@kernel.org]

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) {

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261103163949.1554-1-yishaih%40nvidia.com