Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20151121192035.GC3818@brightrain.aerifal.cx>
Date: Sat, 21 Nov 2015 14:20:35 -0500
From: Rich Felker <dalias@...c.org>
To: musl@...ts.openwall.com
Subject: Re: [PATCHv2] properly handle point-to-point interfaces in
 getifaddrs()

On Sat, Nov 21, 2015 at 11:14:46AM +0200, Timo Teras wrote:
> On Thu, 19 Nov 2015 21:43:10 +0100
> Jo-Philipp Wich <jow@...nwrt.org> wrote:
> 
> > With point-to-point interfaces, the IFA_ADDRESS netlink attribute
> > contains the peer address while an extra attribute IFA_LOCAL carries
> > the actual local interface address.
> > 
> > Both the glibc and uclibc implementations of getifaddrs() handle this
> > case by moving the ifa_addr contents to the broadcast/remote address
> > union and overwriting ifa_addr upon receipt of an IFA_LOCAL attribute.
> > 
> > This patch adds the same special treatment logic of IFA_LOCAL to
> > musl's implementation of getifaddrs() in order to align its behaviour
> > with that of uclibc and musl.
> > 
> > Signed-off-by: Jo-Philipp Wich <jow@...nwrt.org>
> > ---
> > Changelog v2:
> >  * Handle IFA_LOCAL, IFA_ADDRESS in arbritary order
> >  * Remove misleading comment for IFA_BROADCAST, no such attribute on
> > ptp links ---
> >  src/network/getifaddrs.c | 19 ++++++++++++++++---
> >  1 file changed, 16 insertions(+), 3 deletions(-)
> 
> I wrote the code before looking into how ptp links are reported, and
> just assumed it'd be somehow consistent. But IFA_ADDRESS indeed is
> the peer for ptp links. How nicely inconsistent from kernel side ;)
> 
> Seems iproute2 basically does:
>  1. If IFA_LOCAL not set, copy IFA_ADDRESS to it
>  2. If IFA_ADDRESS is not set, copy IFA_LOCAL to it
>  3. Print IFA_LOCAL as local address
>  4. Print IFA_ADDRESS as peer address if it's not equal to IFA_LOCAL
> 
> So this looks right to me.

Are you sure? The new patch seems to have exactly the same issue with
depending on the order of the records as the old patch had. To solve
it I think both need to be stored in temp storage during the loop,
then code after the loop has to resolve which to use. Am I missing
something?

Rich

Powered by blists - more mailing lists

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.