On Wed, May 04, 2022 at 04:06:10PM +0200, Johannes Nixdorf wrote:
> commit 9995b408f17ff8c7f11bc725c8aa225ba3a63b1c upstream.
>
> There are two reasons for addrconf_notify() to be called with NETDEV_DOWN:
> either the network device is actually going down, or IPv6 was disabled
> on the interface.
>
> If either of them stays down while the other is toggled, we repeatedly
> call the code for NETDEV_DOWN, including ipv6_mc_down(), while never
> calling the corresponding ipv6_mc_up() in between. This will cause a
> new entry in idev->mc_tomb to be allocated for each multicast group
> the interface is subscribed to, which in turn leaks one struct ifmcaddr6
> per nontrivial multicast group the interface is subscribed to.
>
> The following reproducer will leak at least $n objects:
>
> ip addr add ff2e::4242/32 dev eth0 autojoin
> sysctl -w net.ipv6.conf.eth0.disable_ipv6=1
> for i in $(seq 1 $n); do
> ip link set up eth0; ip link set down eth0
> done
>
> Joining groups with IPV6_ADD_MEMBERSHIP (unprivileged) or setting the
> sysctl net.ipv6.conf.eth0.forwarding to 1 (=> subscribing to ff02::2)
> can also be used to create a nontrivial idev->mc_list, which will the
> leak objects with the right up-down-sequence.
>
> Based on both sources for NETDEV_DOWN events the interface IPv6 state
> should be considered:
>
> - not ready if the network interface is not ready OR IPv6 is disabled
> for it
> - ready if the network interface is ready AND IPv6 is enabled for it
>
> The functions ipv6_mc_up() and ipv6_down() should only be run when this
> state changes.
>
> Implement this by remembering when the IPv6 state is ready, and only
> run ipv6_mc_down() if it actually changed from ready to not ready.
>
> The other direction (not ready -> ready) already works correctly, as:
>
> - the interface notification triggered codepath for NETDEV_UP /
> NETDEV_CHANGE returns early if ipv6 is disabled, and
> - the disable_ipv6=0 triggered codepath skips fully initializing the
> interface as long as addrconf_link_ready(dev) returns false
> - calling ipv6_mc_up() repeatedly does not leak anything
>
> Fixes: 3ce62a84d53c ("ipv6: exit early in addrconf_notify() if IPv6 is disabled")
> Signed-off-by: Johannes Nixdorf <j.nixdorf@xxxxxx>
> Signed-off-by: David S. Miller <davem@xxxxxxxxxxxxx>
> [jnixdorf: context updated for bpo to v4.19/v5.4]
> Signed-off-by: Johannes Nixdorf <j.nixdorf@xxxxxx>
> ---
> net/ipv6/addrconf.c | 8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
> index 9d8b791f63ef..295adfabf870 100644
> --- a/net/ipv6/addrconf.c
> +++ b/net/ipv6/addrconf.c
> @@ -3660,6 +3660,7 @@ static int addrconf_ifdown(struct net_device *dev, int how)
> struct inet6_dev *idev;
> struct inet6_ifaddr *ifa, *tmp;
> bool keep_addr = false;
> + bool was_ready;
> int state, i;
>
> ASSERT_RTNL();
> @@ -3725,7 +3726,10 @@ static int addrconf_ifdown(struct net_device *dev, int how)
>
> addrconf_del_rs_timer(idev);
>
> - /* Step 2: clear flags for stateless addrconf */
> + /* Step 2: clear flags for stateless addrconf, repeated down
> + * detection
> + */
> + was_ready = idev->if_flags & IF_READY;
> if (!how)
> idev->if_flags &= ~(IF_RS_SENT|IF_RA_RCVD|IF_READY);
>
> @@ -3799,7 +3803,7 @@ static int addrconf_ifdown(struct net_device *dev, int how)
> if (how) {
> ipv6_ac_destroy_dev(idev);
> ipv6_mc_destroy_dev(idev);
> - } else {
> + } else if (was_ready) {
> ipv6_mc_down(idev);
> }
>
> --
> 2.36.0
>
All now queued up, thanks.
greg k-h
