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:   Mon, 23 Apr 2018 22:16:18 +0300
From:   jackm <jackm@....mellanox.co.il>
To:     Håkon Bugge <haakon.bugge@...cle.com>
Cc:     Jason Gunthorpe <jgg@...pe.ca>, Doug Ledford <dledford@...hat.com>,
        Don Hiatt <don.hiatt@...el.com>,
        Dasaratharaman Chandramouli 
        <dasaratharaman.chandramouli@...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
Subject: Re: [PATCH] IB/core: Make ib_mad_client_id atomic

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