Re: [PATCH] RDMA/core: Insure security pkey modify is not lost

[Jason Gunthorpe <jgg@ziepe.ca>]

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@intel.com>
> Signed-off-by: Mike Marciniszyn <mike.marciniszyn@intel.com>
> ---
>  drivers/infiniband/core/security.c |   11 +++--------
>  1 file changed, 3 insertions(+), 8 deletions(-)

Applied to for-rc, thanks

Jason