linux-kernel - Re: [PATCH] IB/core: Make ib_mad_client

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 26 Apr 2018 18:06:10 +0200
From:   Håkon Bugge <haakon.bugge@...cle.com>
To:     Jason Gunthorpe <jgg@...pe.ca>, Doug Ledford <dledford@...hat.com>
Cc:     Don Hiatt <don.hiatt@...el.com>, Ira Weiny <ira.weiny@...el.com>,
        Sean Hefty <sean.hefty@...el.com>,
        OFED mailing list <linux-rdma@...r.kernel.org>,
        linux-kernel@...r.kernel.org, jackm <jackm@....mellanox.co.il>
Subject: Re: [PATCH] IB/core: Make ib_mad_client_id atomic



> On 23 Apr 2018, at 21:16, jackm <jackm@....mellanox.co.il> wrote:
> 
> On Mon, 23 Apr 2018 16:19:57 +0200
> Håkon Bugge <haakon.bugge@...cle.com> wrote:
> 
> 
>>> 
>>> This actually looks like a genuine bug, why is it described only as
>>> 'confusing'? ib_register_mad_agent is callable from userspace, so at
>>> least two userspace agents can race and get the same TID’s.  
>> 
>> My understanding is that every lookup is using the {port, TID} tuple.
>> As such, it is not a bug, but, very confusing.
> Haakon, you are correct (see snippet from the IB spec, below).
> 
> We will NOT have a situation where there are 2 threads/apps
> with the same agent ID on the *same port* (accessing the agent ID
> allocator is protected by a per-port spinlock). Having the same agent ID
> on DIFFERENT ports is OK.
> Thus, there is NO bug here. (But as Haakon says, IMHO it is more robust
> to avoid having the same agent ID for 2 agents even if those agents are
> on different ports).
> 
>> 
>>> TIDs need to be globally unique on the entire machine.  
>> 
> Jason, that is not exactly correct.
> 
> From the IB Spec 1.3, C13-18.1.1 (in section 13.4.6.4 - TransactionID
> usage):
> "When initiating a new operation, MADHeader:TransactionID
> shall be set to such a value that within that MAD the combination of
> TID, SGID, and MgmtClass is different from that of any other currently
> executing operation. If the MAD does not have a GRH, its SLID is used
> in the combination in place of an SGID."
> 
> Since the SGID/SLID is different for each port, the per-port guarantee
> of no 2 agents receiving the same agent-ID value is sufficient.
> 
> -Jack

Shall I interpret this silence as my commit is good to go or that I should add Jack’s tangible information to the commit message?


Thxs, Håkon