| From d9650083e768b3e4b28985753f646ff52722f13d Mon Sep 17 00:00:00 2001 |
| From: Mike Marciniszyn <mike.marciniszyn@intel.com> |
| Date: Fri, 13 Mar 2020 08:47:05 -0400 |
| Subject: [PATCH] RDMA/core: Ensure security pkey modify is not lost |
| |
| commit 2d47fbacf2725a67869f4d3634c2415e7dfab2f4 upstream. |
| |
| 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") |
| Link: https://lore.kernel.org/r/20200313124704.14982.55907.stgit@awfm-01.aw.intel.com |
| Reviewed-by: Kaike Wan <kaike.wan@intel.com> |
| Signed-off-by: Mike Marciniszyn <mike.marciniszyn@intel.com> |
| Reviewed-by: Leon Romanovsky <leonro@mellanox.com> |
| Signed-off-by: Jason Gunthorpe <jgg@mellanox.com> |
| Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com> |
| |
| diff --git a/drivers/infiniband/core/security.c b/drivers/infiniband/core/security.c |
| index 2d5608315dc8..75e7ec017836 100644 |
| --- a/drivers/infiniband/core/security.c |
| +++ b/drivers/infiniband/core/security.c |
| @@ -349,16 +349,11 @@ static struct ib_ports_pkeys *get_new_pps(const struct ib_qp *qp, |
| 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)) |
| + if (((qp_attr_mask & IB_QP_PKEY_INDEX) && |
| + (qp_attr_mask & IB_QP_PORT)) || |
| + (qp_pps && qp_pps->main.state != IB_PORT_PKEY_NOT_VALID)) |
| 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; |
| - } |
| - |
| if (qp_attr_mask & IB_QP_ALT_PATH) { |
| new_pps->alt.port_num = qp_attr->alt_port_num; |
| new_pps->alt.pkey_index = qp_attr->alt_pkey_index; |
| -- |
| 2.7.4 |
| |