Re: [Linux-stable-mirror] [PATCH rdma-rc 1/2] IB/core: Only enforce security for InfiniBand
On 21/11/2017 15:22, Leon Romanovsky wrote:
On Tue, Nov 21, 2017 at 12:44:10PM +0200, Mark Bloch wrote:
Hi,
On 21/11/2017 12:26, Leon Romanovsky wrote:
From: Daniel Jurgens danielj@mellanox.com
For now the only LSM security enforcement mechanism available is specific to InfiniBand. Bypass enforcement for non-IB link types. This fixes a regression where modify_qp fails for iWARP because querying the PKEY returns -EINVAL.
Cc: Paul Moore paul@paul-moore.com Cc: Don Dutile ddutile@redhat.com Cc: stable@vger.kernel.org Reported-by: Potnuri Bharat Teja bharat@chelsio.com Fixes: d291f1a65232("IB/core: Enforce PKey security on QPs") Fixes: 47a2b338fe63("IB/core: Enforce security on management datagrams") Signed-off-by: Daniel Jurgens danielj@mellanox.com Reviewed-by: Parav Pandit parav@mellanox.com Tested-by: Potnuri Bharat Teja bharat@chelsio.com Signed-off-by: Leon Romanovsky leon@kernel.org
drivers/infiniband/core/security.c | 9 +++++++++ 1 file changed, 9 insertions(+)
diff --git a/drivers/infiniband/core/security.c b/drivers/infiniband/core/security.c index 23278ed5be45..314bf1137c7b 100644 --- a/drivers/infiniband/core/security.c +++ b/drivers/infiniband/core/security.c @@ -417,8 +417,17 @@ void ib_close_shared_qp_security(struct ib_qp_security *sec)
int ib_create_qp_security(struct ib_qp *qp, struct ib_device *dev) {
u8 i = rdma_start_port(dev);
bool is_ib = false; int ret;
while (i <= rdma_end_port(dev) && !is_ib)
is_ib = rdma_protocol_ib(dev, i++);What happens if we have mixed port types?
We will have is_ib set and qp_sec will be allocated on device layer and not on port level, but because pkeys are IB specific term (at least, I didn't find any mentioning in RoCE spec), the modify_qp won't query for PKEYS.
What is port level for qp_sec?
I just gave mlx4 as an example, but I was talking more about the ability of the RDMA subsystem to support mixed port types, so in this case, if in the future a vendor will come with an ib_device with 2 ports, one is IB and one is iWARP bad things will happen.
I believe mlx4 can expose two ports where each port uses a different ll protocol. Was that changed?
It is still true.
- /* If this isn't an IB device don't create the security context */
- if (!is_ib)
return 0;- qp->qp_sec = kzalloc(sizeof(*qp->qp_sec), GFP_KERNEL); if (!qp->qp_sec) return -ENOMEM;
Mark.
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Mark
On Tue, Nov 21, 2017 at 03:56:19PM +0200, Mark Bloch wrote:
I just gave mlx4 as an example, but I was talking more about the ability of the RDMA subsystem to support mixed port types, so in this case, if in the future a vendor will come with an ib_device with 2 ports, one is IB and one is iWARP bad things will happen.
That will never be allowed.
Even mixing roce and IB on the same device should be banned, IMHO.
If APM does not work between the ports then they do not belong on the same device.
Jason
On 21/11/2017 17:14, Jason Gunthorpe wrote:
On Tue, Nov 21, 2017 at 03:56:19PM +0200, Mark Bloch wrote:
I just gave mlx4 as an example, but I was talking more about the ability of the RDMA subsystem to support mixed port types, so in this case, if in the future a vendor will come with an ib_device with 2 ports, one is IB and one is iWARP bad things will happen.
That will never be allowed.
You say never be allowed, I say code talks, and in the code we don't have this restriction. maybe we should add something?
LSM security enforcement should only take place on IB devices, Daniel's comment even says that:
- /* If this isn't an IB device don't create the security context */
- if (!is_ib)
return 0;
but how do we define an ib device with different port types? also while today we only deal with pkeys (if I remember currently) in the future we might add other bits, and those bits might not play nicely in the that configuration.
Maybe we should make sure all the ports are IB, and if not, flag it to the user (dmesg?)
Even mixing roce and IB on the same device should be banned, IMHO.
If APM does not work between the ports then they do not belong on the same device.
Jason
Mark.
On Tue, Nov 21, 2017 at 05:33:04PM +0200, Mark Bloch wrote:
You say never be allowed, I say code talks, and in the code we don't have this restriction. maybe we should add something?
I say that because I will NAK any attempt to merge such a driver.
I'd welcome patches to encforce this restriction in the core.
Maybe we should make sure all the ports are IB, and if not, flag it to the user (dmesg?)
I would welcome this patch too.
Will this break any mellanox drivers?
Jason
On 11/21/2017 9:33 AM, Mark Bloch wrote:
On 21/11/2017 17:14, Jason Gunthorpe wrote:
On Tue, Nov 21, 2017 at 03:56:19PM +0200, Mark Bloch wrote:
I just gave mlx4 as an example, but I was talking more about the ability of the RDMA subsystem to support mixed port types, so in this case, if in the future a vendor will come with an ib_device with 2 ports, one is IB and one is iWARP bad things will happen.
That will never be allowed.
You say never be allowed, I say code talks, and in the code we don't have this restriction. maybe we should add something?
LSM security enforcement should only take place on IB devices, Daniel's comment even says that:
- /* If this isn't an IB device don't create the security context */
- if (!is_ib)
return 0;but how do we define an ib device with different port types? also while today we only deal with pkeys (if I remember currently) in the future we might add other bits, and those bits might not play nicely in the that configuration.
Maybe we should make sure all the ports are IB, and if not, flag it to the user (dmesg?)
The only warning that would make sense is if the mixed ports aren't all IB or RoCE. As you note, CX-3 can mix those two, we don't want to see warnings about that.
Even mixing roce and IB on the same device should be banned, IMHO. If APM does not work between the ports then they do not belong on the same device.
Jason
Mark.
On Tue, Nov 21, 2017 at 09:37:27AM -0600, Daniel Jurgens wrote:
The only warning that would make sense is if the mixed ports aren't all IB or RoCE. As you note, CX-3 can mix those two, we don't want to see warnings about that.
I would really like to see cx3 be changed to not do that, then we could finalize this issue upstream: All device ports must be the same protocol.
Jason
linux-stable-mirror@lists.linaro.org
-
Daniel Jurgens -
Jason Gunthorpe -
Jason Gunthorpe -
Mark Bloch