On 11/12/2015 1:13 PM, ira.weiny wrote:
> On Thu, Nov 12, 2015 at 01:48:50PM +0200, Hal Rosenstock wrote:
>>
>> Receipt of CM MAD with response method for other than ClassPortInfo
>> attribute is invalid.
>>
>> CM attributes other than ClassPortInfo use send method only
>> and GetResp is valid for ClassPortInfo attribute.
>> Note also that the CM ClassPortInfo is not currently supported.
>>
>> The SRP initiator does not maintain a timeout policy for CM connect
>> requests relies on the CM layer to do that. The result was that
>> the SRP initiator hung as the connect request never completed.
>>
>> A new SRP target has been observed to respond to Send CM REQ
>> with GetResp of CM REQ with bad status. This is non conformant
>> with IBA spec but exposes a vulnerability in the current MAD/CM
>> code which will respond to the incoming GetResp of CM REQ as if
>> it was a valid incoming Send of CM REQ rather than tossing
>> this on the floor. It also causes the MAD layer not to
>> retransmit the original REQ even though it has not received a REP.
>>
>> Reviewed-by: Sagi Grimberg <sagig@xxxxxxxxxxxx>
>> Signed-off-by: Hal Rosenstock <hal@xxxxxxxxxxxx>
>> ---
>> drivers/infiniband/core/mad.c | 8 ++++++++
>> include/rdma/ib_mad.h | 2 ++
>> 2 files changed, 10 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/infiniband/core/mad.c b/drivers/infiniband/core/mad.c
>> index 8d8af7a..e2d425f 100644
>> --- a/drivers/infiniband/core/mad.c
>> +++ b/drivers/infiniband/core/mad.c
>> @@ -1811,6 +1811,14 @@ static int validate_mad(const struct ib_mad_hdr *mad_hdr,
>> if (qp_num == 0)
>> valid = 1;
>> } else {
>> + /* CM attributes other than ClassPortInfo only use Send method */
>> + if (mad_hdr->mgmt_class == IB_MGMT_CLASS_CM) {
>> + if (mad_hdr->attr_id != IB_MGMT_CLASSPORTINFO_ATTR_ID) {
>> + if (mad_hdr->method != IB_MGMT_METHOD_SEND)
>> + goto out;
>> + } else if (mad_hdr->method != IB_MGMT_METHOD_GET_RESP)
>> + goto out;
>> + }
>
> Doesn't this invalidate a CM Get(ClassPortInfo) mad? Is that the intention
> because it is not supported in the CM?
The intention was to future proof it for when it would be supported but
I misexecuted this as it does not currently handle get as you pointed
out. v2 of patch to follow...
> For the future it seems like these types of checks should be done at the class
> level. But I'm not advocating that right now.
Yes, architecturally these sort of checks belong at the class level.
I started with the checks at the class level (in CM) but since CM relies
on the MAD retransmission mechanism, I didn't want to reimplement that
there so decided to violate the layering and put these (CM) class
specific checks in the mad module.
-- Hal
> Ira
>
>> /* Filter GSI packets sent to QP0 */
>> if (qp_num != 0)
>> valid = 1;
>> diff --git a/include/rdma/ib_mad.h b/include/rdma/ib_mad.h
>> index 188df91..ec9b44d 100644
>> --- a/include/rdma/ib_mad.h
>> +++ b/include/rdma/ib_mad.h
>> @@ -237,6 +237,8 @@ struct ib_vendor_mad {
>> u8 data[IB_MGMT_VENDOR_DATA];
>> };
>>
>> +#define IB_MGMT_CLASSPORTINFO_ATTR_ID cpu_to_be16(0x0001)
>> +
>> struct ib_class_port_info {
>> u8 base_version;
>> u8 class_version;
>> --
>> 1.7.8.2
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
>> the body of a message to majordomo@xxxxxxxxxxxxxxx
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
