RDMA/ucma: Protect kernel from QPN larger than declared in IBTA
PATCH 1/1 RDMA/ucma: Protect kernel from QPN larger than declared in IBTA
From: Leon Romanovsky <[email protected]>
IBTA declares QPN as 24bits, mask input to ensure that kernel doesn't get higher bits.
---
* Not fully tested yet, passed sanity tests for now.
--- drivers/infiniband/core/ucma.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/infiniband/core/ucma.c +1 -1
@@ -1045,7 +1045,7 @@ static void ucma_copy_conn_param(struct rdma_cm_id *id, dst->retry_count = src->retry_count; dst->rnr_retry_count = src->rnr_retry_count; dst->srq = src->srq; - dst->qp_num = src->qp_num; + dst->qp_num = src->qp_num &0xFFFFFF ; dst->qkey = (id->route.addr.src_addr.ss_family ==AF_IB ) ? src->qkey :0 ; }
--
2.20.1
Quoted 15 lines · Leon Romanovsky
From: Leon Romanovsky <[email protected]>
IBTA declares QPN as 24bits, mask input to ensure that kernel doesn't get higher bits.
Fixes 75216638572f ("RDMA/cma: Export rdma cm interface to userspace")Signed-off-by Leon Romanovsky <[email protected]>* Not fully tested yet, passed sanity tests for now.
drivers/infiniband/core/ucma.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)drivers/infiniband/core/ucma.c +1
-0+++ b/drivers/infiniband/core/ucma.c
drivers/infiniband/core/ucma.c static void ucma_copy_conn_param(struct rdma_cm_id *id, · line 1047
Quoted 3 lines
drivers/infiniband/core/ucma.c +0
-0@@ -1045,7 +1045,7 @@ static void ucma_copy_conn_param(struct rdma_cm_id *id, dst->retry_count = src->retry_count; dst->rnr_retry_count = src->rnr_retry_count; dst->srq = src->srq;
srq too?
Jason
Quoted 12 lines · Jason Gunthorpe
Quoted 11 lines · Leon Romanovsky
From: Leon Romanovsky <[email protected]>
IBTA declares QPN as 24bits, mask input to ensure that kernel doesn't get higher bits.
Fixes 75216638572f ("RDMA/cma: Export rdma cm interface to userspace")Signed-off-by Leon Romanovsky <[email protected]>* Not fully tested yet, passed sanity tests for now.
drivers/infiniband/core/ucma.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Quoted 9 lines
drivers/infiniband/core/ucma.c static void ucma_copy_conn_param(struct rdma_cm_id *id, · line 1047
Quoted 7 lines
drivers/infiniband/core/ucma.c +1
-0+++ b/drivers/infiniband/core/ucma.c@@ -1045,7 +1045,7 @@ static void ucma_copy_conn_param(struct rdma_cm_id *id, dst->retry_count = src->retry_count; dst->rnr_retry_count = src->rnr_retry_count; dst->srq = src->srq;srq too?
If I read IBTA correctly, the answer is yes. In all CM messages, QPN, SRQ number and EECN are 24 bits.
Thanks
Quoted 2 lines
Jason
Quoted 16 lines
IBTA declares QPN as 24bits, mask input to ensure that kernel doesn't get higher bits.
Fixes 75216638572f ("RDMA/cma: Export rdma cm interface to userspace")Signed-off-by Leon Romanovsky <[email protected]>---
* Not fully tested yet, passed sanity tests for now.--- drivers/infiniband/core/ucma.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)drivers/infiniband/core/ucma.c +0
-0
drivers/infiniband/core/ucma.c static void ucma_copy_conn_param(struct rdma_cm_id *id, · line 1048
Quoted 5 lines
drivers/infiniband/core/ucma.c +1
-1@@ -1045,7 +1045,7 @@ static void ucma_copy_conn_param(struct rdma_cm_id *id, dst->retry_count = src->retry_count; dst->rnr_retry_count = src->rnr_retry_count; dst->srq = src->srq; - dst->qp_num = src->qp_num; + dst->qp_num = src->qp_num &0xFFFFFF ;
Why not isolate IBTA restrictions in the ib_cm?
Quoted 11 lines · Sean
Quoted 11 lines
IBTA declares QPN as 24bits, mask input to ensure that kernel doesn't get higher bits.
Fixes 75216638572f ("RDMA/cma: Export rdma cm interface to userspace")Signed-off-by Leon Romanovsky <[email protected]>---
* Not fully tested yet, passed sanity tests for now.--- drivers/infiniband/core/ucma.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Quoted 12 lines
drivers/infiniband/core/ucma.c static void ucma_copy_conn_param(struct rdma_cm_id *id, · line 1048
Quoted 10 lines
drivers/infiniband/core/ucma.c +1
-1@@ -1045,7 +1045,7 @@ static void ucma_copy_conn_param(struct rdma_cm_id *id, dst->retry_count = src->retry_count; dst->rnr_retry_count = src->rnr_retry_count; dst->srq = src->srq; - dst->qp_num = src->qp_num; + dst->qp_num = src->qp_num &0xFFFFFF ;Why not isolate IBTA restrictions in the
ib_cm?
AFAIK, there are many places in IB/core code which assume that type of restriction, so it is safer and cleaner to sanitize input as early as possible.
Thanks
Quoted 18 lines · Leon Romanovsky
From: Leon Romanovsky <[email protected]>
IBTA declares QPN as 24bits, mask input to ensure that kernel doesn't get higher bits.
Fixes 75216638572f ("RDMA/cma: Export rdma cm interface to userspace")Signed-off-by Leon Romanovsky <[email protected]>---
* Not fully tested yet, passed sanity tests for now.--- drivers/infiniband/core/ucma.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)drivers/infiniband/core/ucma.c +0
-0
drivers/infiniband/core/ucma.c static void ucma_copy_conn_param(struct rdma_cm_id *id, · line 1050
Quoted 7 lines
drivers/infiniband/core/ucma.c +1
-1@@ -1045,7 +1045,7 @@ static void ucma_copy_conn_param(struct rdma_cm_id *id, dst->retry_count = src->retry_count; dst->rnr_retry_count = src->rnr_retry_count; dst->srq = src->srq; - dst->qp_num = src->qp_num; + dst->qp_num = src->qp_num &0xFFFFFF ; dst->qkey = (id->route.addr.src_addr.ss_family ==AF_IB ) ? src->qkey :0 ; }
This really needs to be squashed into the other qpn patch because what is really being proposed here is to move the masking from the core code to the ucma and the core code will assume that the caller is using correct QPNS. Maybe leave behind a WARN_ON to confirm this.
Jason