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

Date:	Tue, 24 Nov 2015 12:44:17 +0100
From:	Phil Sutter <phil@....cc>
To:	Stephen Hemminger <stephen@...workplumber.org>
Cc:	Thomas Haller <thaller@...hat.com>, netdev@...r.kernel.org
Subject: Re: [PATCH iproute2] ip-address: properly display zero IPv4 peer
 address

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:

1) ip a a 192.168.1.1/24                             dev test0
2) ip a a 192.168.2.1        peer 192.168.2.1/24     dev test0
3) ip a a 192.168.3.1        peer 0.0.0.0/24         dev test0
4) ip a a 192.168.4.1        peer 192.168.4.2        dev test0
5) ip a a feed:babe::1:1/112                         dev test0
6) ip a a feed:babe::2:1     peer feed:babe::2:1/112 dev test0
7) ip a a feed:babe::3:1     peer ::/112             dev test0
8) ip a a feed:babe::4:1     peer feed:babe::4:2     dev test0

cmd ifa_local      ifa_address
---------------------------------
1)  192.168.1.1    192.168.1.1
2)  192.168.2.1    192.168.2.1
3)  192.168.3.1    unset
4)  192.168.4.1    192.168.4.2
5)  unset          feed:babe::1:1
6)  unset          feed:babe::2:1
7)  unset          feed:babe::3:1
8)  feed:babe::4:1 feed:babe::4:2

No idea how this looks for decnet and ipx. Looking only at IPv6 though,
the patch's check for !AF_INET before setting rta_tb[IFA_ADDRESS] =
rta_tb[IFA_LOCAL] could indeed be skipped.

On a side note, I'm pretty sure the later memcmp() could be skipped in
many cases, at least by comparing the pointer values of
rta_tb[IFA_ADDRESS] and rta_tb[IFA_LOCAL].

Cheers, Phil
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html