>
> From: Maor Gottlieb <maorg@xxxxxxxxxxxx>
>
> When port is part of the modify mask, then we should take
> it from the qp_attr and not from the old pps. Same for PKEY.
>
> Cc: <stable@xxxxxxxxxxxxxxx>
> Fixes: 1dd017882e01 ("RDMA/core: Fix protection fault in
> get_pkey_idx_qp_list")
> Signed-off-by: Maor Gottlieb <maorg@xxxxxxxxxxxx>
> Signed-off-by: Leon Romanovsky <leonro@xxxxxxxxxxxx>
> ---
> drivers/infiniband/core/security.c | 12 ++++++++----
> 1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/infiniband/core/security.c
> b/drivers/infiniband/core/security.c
> index b9a36ea244d4..2d5608315dc8 100644
> --- a/drivers/infiniband/core/security.c
> +++ b/drivers/infiniband/core/security.c
> @@ -340,11 +340,15 @@ static struct ib_ports_pkeys *get_new_pps(const
> struct ib_qp *qp,
> return NULL;
>
> if (qp_attr_mask & IB_QP_PORT)
> - new_pps->main.port_num =
> - (qp_pps) ? qp_pps->main.port_num : qp_attr-
> >port_num;
> + new_pps->main.port_num = qp_attr->port_num;
> + else if (qp_pps)
> + new_pps->main.port_num = qp_pps->main.port_num;
> +
> if (qp_attr_mask & IB_QP_PKEY_INDEX)
> - new_pps->main.pkey_index = (qp_pps) ? qp_pps-
> >main.pkey_index :
> - qp_attr->pkey_index;
> + new_pps->main.pkey_index = qp_attr->pkey_index;
> + else if (qp_pps)
> + new_pps->main.pkey_index = qp_pps->main.pkey_index;
> +
> if ((qp_attr_mask & IB_QP_PKEY_INDEX) && (qp_attr_mask &
> IB_QP_PORT))
> new_pps->main.state = IB_PORT_PKEY_VALID;
>
I agree with this aspect of the patch and it does fix the panic, because the correct unit
is gotten from qp_pps.
My issue is that the new_pps->main.state will come back as 0, and the insert routine will drop any new pkey index update.
The sequence I'm concerned about is:
0x71 attr mask with both pkey index and port.
A ulp decides to change the pkey index and does an 0x51 modify without setting the unit.
I see new_pps->main.state being returned as 0 and port_pkey_list_insert() will early out.
I asked this exact question in https://marc.info/?l=linux-rdma&m=158278763015030&w=2.
I also asked about the logical or, and you answered that pointing to an additional patch.
You but didn't address the main.state being 0 and losing the pkey_index update in the 0x51 modify.
Mike
