netdev - Re: [PATCH iproute2] ip-address: properly display zero IPv4 peer address

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: <1448367280.3677.16.camel@redhat.com>
Date:	Tue, 24 Nov 2015 13:14:40 +0100
From:	Thomas Haller <thaller@...hat.com>
To:	Phil Sutter <phil@....cc>,
	Stephen Hemminger <stephen@...workplumber.org>
Cc:	netdev@...r.kernel.org
Subject: Re: [PATCH iproute2] ip-address: properly display zero IPv4 peer
 address

On Tue, 2015-11-24 at 12:44 +0100, Phil Sutter wrote:
> On Mon, Nov 23, 2015 at 04:04:50PM -0800, Stephen Hemminger wrote:
> > On Thu, 22 Oct 2015 10:34:28 +0200
> > Thomas Haller <thaller@...hat.com> wrote:
> > 
> > > Kernel allows for zero IPv4 peer addresses (IFA_ADDRESS):
> > > 
> > >    ip address add 192.168.5.1 peer 0.0.0.0/24 dev dummy
> > > 
> > > which is distinct from a usual address like:
> > > 
> > >    ip address add 192.168.5.1/24 dev dummy
> > >    ip address add 192.168.5.1 peer 192.168.5.1/24 dev dummy
> > > 
> > > For IPv4, a missing IFA_ADDRESS attribute means that the peer
> > > is 0.0.0.0. See inet_fill_ifaddr(), which does:
> > > 
> > >   if ((ifa->ifa_address &&
> > >        nla_put_in_addr(skb, IFA_ADDRESS, ifa->ifa_address)) ||
> > > 
> > > Signed-off-by: Thomas Haller <thaller@...hat.com>
> > 
> > I would prefer that this apply to both IPv4 and IPv6.
> 
> The case that patch handles does not happen in IPv6.
> 
> > If the kernel sends back an address, then display it.
> 
> It's rather "if the kernel *does not* send back an address ...".
> 
> When reviewing this patch, I tried to find an easier (and less ugly)
> solution, but failed.  Here's the result from testing all variants:


Thank you Phil for evaluating this.

I also tried to change the patch to come up with a cleaner/unified way,
but the result wasn't subjectively better.


Thomas






For reference, the relevant lines of code from kernel:



ipv4: https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/net/ipv4/devinet.c?id=6a13feb9c82803e2b815eca72fa7a9f5561d7861#n1542

        if ((ifa->ifa_address &&
             nla_put_in_addr(skb, IFA_ADDRESS, ifa->ifa_address)) ||
            (ifa->ifa_local &&
             nla_put_in_addr(skb, IFA_LOCAL, ifa->ifa_local)) ||



ipv6: https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/net/ipv6/addrconf.c?id=6a13feb9c82803e2b815eca72fa7a9f5561d7861#n4310

        if (!ipv6_addr_any(&ifa->peer_addr)) {
                if (nla_put_in6_addr(skb, IFA_LOCAL, &ifa->addr) < 0 ||
                    nla_put_in6_addr(skb, IFA_ADDRESS, &ifa->peer_addr) < 0)
                        goto error;
        } else
                if (nla_put_in6_addr(skb, IFA_ADDRESS, &ifa->addr) < 0)
                        goto error;

Download attachment "signature.asc" of type "application/pgp-signature" (820 bytes)