netdev - Re: [PATCH net-next] ipv6: recreate ipv6 link-local addresses when increasing MTU over IPV6_MIN

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, 26 Oct 2015 18:21:11 +0100
From:	Hannes Frederic Sowa <hannes@...essinduktion.org>
To:	Alexander Duyck <alexander.duyck@...il.com>, netdev@...r.kernel.org
Subject: Re: [PATCH net-next] ipv6: recreate ipv6 link-local addresses when
 increasing MTU over IPV6_MIN_MTU

Hi Alex,

On Mon, Oct 26, 2015, at 18:07, Alexander Duyck wrote:
> >> Seems like this code isn't quite correct.  You are calling ipv6_add_dev
> >> for slave devices, and if I understand things correctly I don't believe
> >> that was happening before and may be an unintended side effect.
> > Hmm, could you quickly help me where I get into this situation? I made
> > sure I enter the NETDEV_UP part before the IFF_SLAVE test and
> > disable_ipv6 te
> 
> I think I was getting a bit a head of myself.  I was looking over the 
> NETDEV_UP code and thinking that we could just fall into that path since 
> it is already calling ipv6_add_dev.  However now I am wondering if maybe 
> we need to look at adding an idev allocation somewhere before the 
> disable_ipv6 check.  I assume that is why you were allocating the idev 
> before you were getting into NETDEV_UP?

The original bug report was:

If user reduces the MTU below IPV6_MIN_MTU we addrconf_ifdown the
interface but don't reinitialize the interface if the MTU is increased
later on.

> >> You might want to instead just make it so that you only do the jump, and
> >> perhaps change the code in the NETDEV_UP/NETDEV_CHANGE section so that
> >> you test for NETDEV_CHANGE instead of NETDEV_UP.  That should be enough
> >> to get the effect you are looking for and I believe there would be no
> >> change to behaviour other than adding IPv6 link-local addresses when the
> >> MTU is increased.
> >>
> >> Give me a bit and I can submit an alternative that may actually work out
> >> a bit better I think.
> > If you go the NETDEV_CHANGE route instead of NETDEV_UP, you end up with
> > the IF_READY flag already set from ipv6_add_dev and thus won't do any
> > initialization of the device.
> 
> What I meant was that you don't need to change the event.  If you change 
> the check inside the NETDEV_UP/CHANGE code path so that it tests for 
> event != NETDEV_CHANGE instead of event == NETDEV_UP you don't need to 
> change the event type.

Yeah, that would be possible, too. I just find an equal easier to
follow. ;)

> > Sure, I wait.
> 
> Might be a bit longer.  I just realized that I think there is another 
> bug here where you are going through the NETDEV_UP path even though the 
> interface isn't up.  I'll run through some testing this morning to work 
> out the kinks.

Ok, cool. I have a look at it again, too.

Bye,
Hannes
--
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