| 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
| ||
|
Message-ID: <63892fca-4da6-0beb-73d3-b163977ed2fb@gmail.com>
Date: Fri, 6 Mar 2020 12:14:08 +0100
From: Rafał Miłecki <zajec5@...il.com>
To: Hangbin Liu <liuhangbin@...il.com>
Cc: Network Development <netdev@...r.kernel.org>,
"David S. Miller" <davem@...emloft.net>,
Alexey Kuznetsov <kuznet@....inr.ac.ru>,
Hideaki YOSHIFUJI <yoshfuji@...ux-ipv6.org>,
Felix Fietkau <nbd@....name>, John Crispin <john@...ozen.org>,
Jo-Philipp Wich <jo@...n.io>
Subject: Re: Regression: net/ipv6/mld running system out of memory (not a
leak)
Hi Hangbin,
I took a moment to note down what exactly happens in the IPv6 code so
we can be sure things work as expected. I compared kernel behavior
without your patch and with it for two different network interfaces.
On 04.03.2020 10:07, Hangbin Liu wrote:
> From 9cf504dc30198133e470ea783c3fa53a8f56a20a Mon Sep 17 00:00:00 2001
> From: Hangbin Liu <liuhangbin@...il.com>
> Date: Tue, 3 Mar 2020 20:58:39 +0800
> Subject: [PATCH] ipv6/mcast: join and leave allnodes, allrouters addresses
> when link up/down
>
> This patch fixes two issues:
>
> 1) When link up with IPv6 forwarding enabled, we forgot to join
> sitelocal_allrouters(ff01::2) and interfacelocal_allrouters(ff05::2),
> like what we do in function dev_forward_change()
>
> 2) When link down, there is no listener for the allnodes, allrouters
> addresses. So we should remove them instead of storing the info in
> idev->mc_tomb.
>
> To fix these two issue, I add two new functions ipv6_mc_join_localaddr()
> and ipv6_mc_leave_localaddr() to handle these addresses. And just join
> leave the group in ipv6_mc_{up, down}.
********************
WITHOUT YOUR PATCH
********************
The problem we're dealing with seems to be specific to non-Ethernen
devices. For ARPHRD_IEEE80211_RADIOTAP:
1. Multicast addresses get added normally - as for any Ethernet device
2. addrconf_dev_config() returns without calling addrconf_add_dev()
That means wireless monitor interface gets mcast addresses added but
not removed (like it happens for Ethernet devices).
For ARPHRD_ETHER things seem to work correctly.
#
# wlan0 (ARPHRD_ETHER)
#
addrconf_notify(NETDEV_REGISTER)
ipv6_add_dev
ipv6_dev_mc_inc(ff01::1)
ipv6_dev_mc_inc(ff02::1)
ipv6_dev_mc_inc(ff02::2)
addrconf_notify(NETDEV_UP)
addrconf_dev_config
addrconf_add_dev
ipv6_find_idev
ipv6_mc_up
mld_del_delrec(ff02::2)
igmp6_group_added(ff02::2)
mld_del_delrec(ff02::1)
igmp6_group_added(ff02::1)
mld_del_delrec(ff01::1)
igmp6_group_added(ff01::1)
addrconf_notify(NETDEV_DOWN)
addrconf_ifdown
ipv6_mc_down
igmp6_group_dropped(ff02::2)
igmp6_leave_group(ff02::2)
mld_add_delrec(ff02::2)
igmp6_group_dropped(ff02::1)
igmp6_group_dropped(ff01::1)
ipv6_mc_destroy_dev
ipv6_mc_down
igmp6_group_dropped(ff02::2)
igmp6_group_dropped(ff02::1)
igmp6_group_dropped(ff01::1)
mld_clear_delrec
__ipv6_dev_mc_dec(ff02::1)
__ipv6_dev_mc_dec(ff02::2)
#
# mon-phy0 (ARPHRD_IEEE80211_RADIOTAP) ***
#
addrconf_notify(NETDEV_REGISTER)
ipv6_add_dev
ipv6_dev_mc_inc(ff01::1)
ipv6_dev_mc_inc(ff02::1)
ipv6_dev_mc_inc(ff02::2)
addrconf_notify(NETDEV_UP)
addrconf_dev_config
/* Alas, we support only Ethernet autoconfiguration. */
return;
addrconf_notify(NETDEV_DOWN)
addrconf_ifdown
ipv6_mc_down
igmp6_group_dropped(ff02::2)
igmp6_leave_group(ff02::2)
mld_add_delrec(ff02::2)
igmp6_group_dropped(ff02::1)
igmp6_group_dropped(ff01::1)
ipv6_mc_destroy_dev
ipv6_mc_down
igmp6_group_dropped(ff02::2)
igmp6_group_dropped(ff02::1)
igmp6_group_dropped(ff01::1)
mld_clear_delrec
__ipv6_dev_mc_dec(ff02::1)
__ipv6_dev_mc_dec(ff02::2)
*****************
WITH YOUR PATCH
*****************
Things work OK - with your changes all calls like:
ipv6_dev_mc_inc(ff01::1)
ipv6_dev_mc_inc(ff02::1)
ipv6_dev_mc_inc(ff02::2)
are now part of ipv6_mc_up() which gets never called for the
ARPHRD_IEEE80211_RADIOTAP.
I got one more question though:
Should we really call ipv6_mc_down() for ARPHRD_IEEE80211_RADIOTAP?
We don't call ipv6_mc_up() so maybe ipv6_mc_down() should be avoided
too? It seems like asking for more problems in the future. Even now
we call ipv6_mc_leave_localaddr() without ipv6_mc_join_localaddr()
called first which seems unintuitive.
#
# wlan0 (ARPHRD_ETHER)
#
addrconf_notify(NETDEV_REGISTER)
ipv6_add_dev
addrconf_notify(NETDEV_UP)
addrconf_dev_config
addrconf_add_dev
ipv6_find_idev
ipv6_mc_up
ipv6_mc_join_localaddr
ipv6_dev_mc_inc(ff01::1)
ipv6_dev_mc_inc(ff02::1)
ipv6_dev_mc_inc(ff01::2)
ipv6_dev_mc_inc(ff02::2)
ipv6_dev_mc_inc(ff05::2)
mld_del_delrec(ff05::2)
igmp6_group_added(ff05::2)
mld_del_delrec(ff02::2)
igmp6_group_added(ff02::2)
mld_del_delrec(ff01::2)
igmp6_group_added(ff01::2)
mld_del_delrec(ff02::1)
igmp6_group_added(ff02::1)
mld_del_delrec(ff01::1)
igmp6_group_added(ff01::1)
addrconf_notify(NETDEV_DOWN)
addrconf_ifdown
ipv6_mc_down
ipv6_mc_leave_localaddr
__ipv6_dev_mc_dec(ff01::1)
__ipv6_dev_mc_dec(ff02::1)
__ipv6_dev_mc_dec(ff01::2)
__ipv6_dev_mc_dec(ff02::2)
igmp6_group_dropped(ff02::2)
__ipv6_dev_mc_dec(ff05::2)
igmp6_group_dropped(ff05::2)
ipv6_mc_destroy_dev
ipv6_mc_down
ipv6_mc_leave_localaddr
__ipv6_dev_mc_dec(ff01::1)
__ipv6_dev_mc_dec(ff02::1)
__ipv6_dev_mc_dec(ff01::2)
__ipv6_dev_mc_dec(ff02::2)
__ipv6_dev_mc_dec(ff05::2)
mld_clear_delrec
#
# mon-phy0 (ARPHRD_IEEE80211_RADIOTAP)
#
addrconf_notify(NETDEV_REGISTER)
ipv6_add_dev
addrconf_notify(NETDEV_UP)
addrconf_dev_config
/* Alas, we support only Ethernet autoconfiguration. */
return;
addrconf_notify(NETDEV_DOWN)
addrconf_ifdown
ipv6_mc_down
ipv6_mc_leave_localaddr
__ipv6_dev_mc_dec(ff01::1)
__ipv6_dev_mc_dec(ff02::1)
__ipv6_dev_mc_dec(ff01::2)
__ipv6_dev_mc_dec(ff02::2)
__ipv6_dev_mc_dec(ff05::2)
ipv6_mc_destroy_dev
ipv6_mc_down
ipv6_mc_leave_localaddr
__ipv6_dev_mc_dec(ff01::1)
__ipv6_dev_mc_dec(ff02::1)
__ipv6_dev_mc_dec(ff01::2)
__ipv6_dev_mc_dec(ff02::2)
__ipv6_dev_mc_dec(ff05::2)
mld_clear_delrec
Powered by blists - more mailing lists