netdev - Re: [PATCH] ipvlan: Don't propagate IFF_ALLMULTI changes on down interfaces.

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, 3 Jun 2019 15:53:56 +0800
From:   Xin Long <lucien.xin@...il.com>
To:     Young Xiao <92siuyang@...il.com>
Cc:     davem <davem@...emloft.net>,
        Daniel Borkmann <daniel@...earbox.net>, petrm@...lanox.com,
        jiri@...lanox.com, idosch@...lanox.com, uehaibing@...wei.com,
        Hangbin Liu <liuhangbin@...il.com>,
        network dev <netdev@...r.kernel.org>,
        LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] ipvlan: Don't propagate IFF_ALLMULTI changes on down interfaces.

On Mon, Jun 3, 2019 at 11:22 AM Young Xiao <92siuyang@...il.com> wrote:
>
> Clearing the IFF_ALLMULTI flag on a down interface could cause an allmulti
> overflow on the underlying interface.
>
> Attempting the set IFF_ALLMULTI on the underlying interface would cause an
> error and the log message:
>
> "allmulti touches root, set allmulti failed."
s/root/roof

I guess this patch was inspired by:

commit bbeb0eadcf9fe74fb2b9b1a6fea82cd538b1e556
Author: Peter Christensen <pch@...bogen.com>
Date:   Thu May 8 11:15:37 2014 +0200

    macvlan: Don't propagate IFF_ALLMULTI changes on down interfaces.

I could trigger this error on macvlan prior to this patch with:

  # ip link add mymacvlan1 link eth2 type macvlan mode bridge
  # ip link set mymacvlan1 up
  # ip link set mymacvlan1 allmulticast on
  # ip link set mymacvlan1 down
  # ip link set mymacvlan1 allmulticast off
  # ip link set mymacvlan1 allmulticast on

but not on ipvlan, could you?

macvlan had this problem, as lowerdev's allmulticast is cleared when doing
dev_stop and set when doing dev_open, which doesn't happen on ipvlan.

So I'd think this patch fixes nothing unless you want to add
dev_set_allmulti(1/-1) in ipvlan_open/stop(), but that's another topic.

did I miss something?

>
> Signed-off-by: Young Xiao <92siuyang@...il.com>
> ---
>  drivers/net/ipvlan/ipvlan_main.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ipvlan/ipvlan_main.c b/drivers/net/ipvlan/ipvlan_main.c
> index bbeb162..523bb83 100644
> --- a/drivers/net/ipvlan/ipvlan_main.c
> +++ b/drivers/net/ipvlan/ipvlan_main.c
> @@ -242,8 +242,10 @@ static void ipvlan_change_rx_flags(struct net_device *dev, int change)
>         struct ipvl_dev *ipvlan = netdev_priv(dev);
>         struct net_device *phy_dev = ipvlan->phy_dev;
>
> -       if (change & IFF_ALLMULTI)
> -               dev_set_allmulti(phy_dev, dev->flags & IFF_ALLMULTI? 1 : -1);
> +       if (dev->flags & IFF_UP) {
> +               if (change & IFF_ALLMULTI)
> +                       dev_set_allmulti(phy_dev, dev->flags & IFF_ALLMULTI ? 1 : -1);
> +       }
>  }
>
>  static void ipvlan_set_multicast_mac_filter(struct net_device *dev)
> --
> 2.7.4
>