netdev - Re: Annoying gcc / rdma / networking warnings

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] [day] [month] [year] [list]

Message-ID: <20190513183053.GI7948@mellanox.com>
Date:   Mon, 13 May 2019 18:30:58 +0000
From:   Jason Gunthorpe <jgg@...lanox.com>
To:     David Miller <davem@...emloft.net>,
        "g@...lanox.com" <g@...lanox.com>
CC:     "torvalds@...ux-foundation.org" <torvalds@...ux-foundation.org>,
        "dledford@...hat.com" <dledford@...hat.com>,
        "linux-rdma@...r.kernel.org" <linux-rdma@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: Re: Annoying gcc / rdma / networking warnings

On Sun, May 12, 2019 at 08:17:01PM -0700, David Miller wrote:
> From: Jason Gunthorpe <jgg@...lanox.com>
> Date: Mon, 13 May 2019 01:11:42 +0000
> 
> > I think the specific sockaddr types should only ever be used if we
> > *know* the sa_family is that type. If the sa_family is not known then
> > it should be sockaddr or sockaddr_storage. Otherwise things get very
> > confusing.
> > 
> > When using sockaddr_storage code always has the cast to sockaddr
> > anyhow, as it is not a union, so this jaunty cast is not out of place
> > in sockets code.
> 
> From what I can see, each and every call side of these helpers like
> rdma_gid2ip() et al. redefine this union type over and over and over
> again in the local function.

Yes, the repeated union is very ugly and could be consolidated - or
should just use sockaddr_storage in the first place.

> It seems that if we just defined it explicitly in one place, like
> include/rdma/ib_addr.h, then we could have tdma_gid2ip(), addr_resolve(),
> and rdma_resolve_ip() take that type explcitily.

I pulled on this thread for a while and the number of places that
would need to convert to use a global 'union rdma_sockaddr_inet'
started to become pretty silly and weird. I eventually reached a point
where I had to cast a sockaddr * to the union - which is something
that makes no sense and I gave up.

So, I think this feels simpler to follow the usual sockaddr_storage
pattern and only use the union to declare the initial storage. Then
everything else just uses the sockaddr * plus casts..

Jason