* [PATCH] RDMA/core: Insure security pkey modify is not lost
@ 2020-03-13 12:47 Mike Marciniszyn
2020-03-18 13:31 ` Jason Gunthorpe
` (2 more replies)
0 siblings, 3 replies; 4+ messages in thread
From: Mike Marciniszyn @ 2020-03-13 12:47 UTC (permalink / raw)
To: jgg, dledford; +Cc: linux-rdma
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(-)
diff --git a/drivers/infiniband/core/security.c b/drivers/infiniband/core/security.c
index 2d56083..75e7ec0 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;
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] RDMA/core: Insure security pkey modify is not lost
2020-03-13 12:47 [PATCH] RDMA/core: Insure security pkey modify is not lost Mike Marciniszyn
@ 2020-03-18 13:31 ` Jason Gunthorpe
2020-03-18 13:59 ` Leon Romanovsky
2020-03-24 22:54 ` Jason Gunthorpe
2 siblings, 0 replies; 4+ messages in thread
From: Jason Gunthorpe @ 2020-03-18 13:31 UTC (permalink / raw)
To: Mike Marciniszyn, Maor Gottlieb, Leon Romanovsky; +Cc: dledford, linux-rdma
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>
Leon? Maor?
> diff --git a/drivers/infiniband/core/security.c b/drivers/infiniband/core/security.c
> index 2d56083..75e7ec0 100644
> +++ 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;
>
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] RDMA/core: Insure security pkey modify is not lost
2020-03-13 12:47 [PATCH] RDMA/core: Insure security pkey modify is not lost Mike Marciniszyn
2020-03-18 13:31 ` Jason Gunthorpe
@ 2020-03-18 13:59 ` Leon Romanovsky
2020-03-24 22:54 ` Jason Gunthorpe
2 siblings, 0 replies; 4+ messages in thread
From: Leon Romanovsky @ 2020-03-18 13:59 UTC (permalink / raw)
To: Mike Marciniszyn; +Cc: jgg, dledford, linux-rdma
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(-)
>
Thanks,
Reviewed-by: Leon Romanovsky <leonro@mellanox.com>
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] RDMA/core: Insure security pkey modify is not lost
2020-03-13 12:47 [PATCH] RDMA/core: Insure security pkey modify is not lost Mike Marciniszyn
2020-03-18 13:31 ` Jason Gunthorpe
2020-03-18 13:59 ` Leon Romanovsky
@ 2020-03-24 22:54 ` Jason Gunthorpe
2 siblings, 0 replies; 4+ messages in thread
From: Jason Gunthorpe @ 2020-03-24 22:54 UTC (permalink / raw)
To: Mike Marciniszyn; +Cc: dledford, linux-rdma
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
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-03-24 22:54 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-03-13 12:47 [PATCH] RDMA/core: Insure security pkey modify is not lost Mike Marciniszyn
2020-03-18 13:31 ` Jason Gunthorpe
2020-03-18 13:59 ` Leon Romanovsky
2020-03-24 22:54 ` Jason Gunthorpe
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).