On 11/21/2017 4:44 AM, Mark Bloch wrote:
> Hi,
>
> On 21/11/2017 12:26, Leon Romanovsky wrote:
>> From: Daniel Jurgens <danielj@xxxxxxxxxxxx>
>>
>> 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@xxxxxxxxxxxxxx>
>> Cc: Don Dutile <ddutile@xxxxxxxxxx>
>> Cc: stable@xxxxxxxxxxxxxxx
>> Reported-by: Potnuri Bharat Teja <bharat@xxxxxxxxxxx>
>> Fixes: d291f1a65232("IB/core: Enforce PKey security on QPs")
>> Fixes: 47a2b338fe63("IB/core: Enforce security on management datagrams")
>> Signed-off-by: Daniel Jurgens <danielj@xxxxxxxxxxxx>
>> Reviewed-by: Parav Pandit <parav@xxxxxxxxxxxx>
>> Tested-by: Potnuri Bharat Teja <bharat@xxxxxxxxxxx>
>> Signed-off-by: Leon Romanovsky <leon@xxxxxxxxxx>
>> ---
>> 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?
> I believe mlx4 can expose two ports where each port uses a different ll protocol.
> Was that changed?
If any port on the device is IB security is enforced on all QPs because a QP could eventually be modified to an IB port. The primary motivation was to fix a regression in iWARP, which has no concept of PKeys, queries of the PKey cache always returned -EINVAL. There is a valid PKey cache for RoCE, so it's doesn't suffer the same issue, but this will benefit an all RoCE port device by eliminating the overhead of security enforcement. If there is ever a multiport device that mixes iWARP and IB this would be a problem, but I don't see that happening.
>
>> + /* 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.