On Fri, Mar 13, 2020 at 08:47:05AM -0400, Mike Marciniszyn wrote:
> The following modify sequence (loosely based on ipoib) will
> lose a pkey modifcation:
>
> - Modify (pkey index, port)
> - Modify (new pkey index, NO port)
>
> After the first modify, the qp_pps list will have saved the pkey and the
> unit on the main list.
>
> During the second modify, get_new_pps() will fetch the port from qp_pps
> and read the new pkey index from qp_attr->pkey_index. The state will
> still be zero, or IB_PORT_PKEY_NOT_VALID. Because of the invalid state,
> the new values will never replace the one in the qp pps list, losing
> the new pkey.
>
> This happens because the following if statements will never correct the
> state because the first term will be false. If the code had been executed,
> it would incorrectly overwrite valid values.
>
> if ((qp_attr_mask & IB_QP_PKEY_INDEX) && (qp_attr_mask & IB_QP_PORT))
> new_pps->main.state = IB_PORT_PKEY_VALID;
>
>
> if (!(qp_attr_mask & (IB_QP_PKEY_INDEX | IB_QP_PORT)) && qp_pps) {
> new_pps->main.port_num = qp_pps->main.port_num;
> new_pps->main.pkey_index = qp_pps->main.pkey_index;
> if (qp_pps->main.state != IB_PORT_PKEY_NOT_VALID)
> new_pps->main.state = IB_PORT_PKEY_VALID;
> }
>
> Fix by joining the two if statements with an or test to see if qp_pps
> is non-NULL and in the correct state.
>
> Fixes: 1dd017882e01 ("RDMA/core: Fix protection fault in get_pkey_idx_qp_list")
> Reviewed-by: Kaike Wan <kaike.wan@xxxxxxxxx>
> Signed-off-by: Mike Marciniszyn <mike.marciniszyn@xxxxxxxxx>
> ---
> drivers/infiniband/core/security.c | 11 +++--------
> 1 file changed, 3 insertions(+), 8 deletions(-)
>
Thanks,
Reviewed-by: Leon Romanovsky <leonro@xxxxxxxxxxxx>
