linux-kernel - Re: [PATCH v3 1/2] net: core: Notify on changes to dev->promiscuity.

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20190829105650.btgvytgja63sx6wx@soft-dev3.microsemi.net>
Date:   Thu, 29 Aug 2019 12:56:54 +0200
From:   Horatiu Vultur <horatiu.vultur@...rochip.com>
To:     Jiri Pirko <jiri@...nulli.us>
CC:     <alexandre.belloni@...tlin.com>, <UNGLinuxDriver@...rochip.com>,
        <davem@...emloft.net>, <andrew@...n.ch>,
        <allan.nielsen@...rochip.com>, <ivecera@...hat.com>,
        <f.fainelli@...il.com>, <netdev@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v3 1/2] net: core: Notify on changes to dev->promiscuity.

The 08/29/2019 11:51, Jiri Pirko wrote:
> External E-Mail
> 
> 
> Thu, Aug 29, 2019 at 11:22:28AM CEST, horatiu.vultur@...rochip.com wrote:
> >Add the SWITCHDEV_ATTR_ID_PORT_PROMISCUITY switchdev notification type,
> >used to indicate whenever the dev promiscuity counter is changed.
> >
> >The notification doesn't use any switchdev_attr attribute because in the
> >notifier callbacks is it possible to get the dev and read directly
> >the promiscuity value.
> >
> >Signed-off-by: Horatiu Vultur <horatiu.vultur@...rochip.com>
> >---
> > include/net/switchdev.h | 1 +
> > net/core/dev.c          | 9 +++++++++
> > 2 files changed, 10 insertions(+)
> >
> >diff --git a/include/net/switchdev.h b/include/net/switchdev.h
> >index aee86a1..14b1617 100644
> >--- a/include/net/switchdev.h
> >+++ b/include/net/switchdev.h
> >@@ -40,6 +40,7 @@ enum switchdev_attr_id {
> > 	SWITCHDEV_ATTR_ID_BRIDGE_VLAN_FILTERING,
> > 	SWITCHDEV_ATTR_ID_BRIDGE_MC_DISABLED,
> > 	SWITCHDEV_ATTR_ID_BRIDGE_MROUTER,
> >+	SWITCHDEV_ATTR_ID_PORT_PROMISCUITY,
> > };
> > 
> > struct switchdev_attr {
> >diff --git a/net/core/dev.c b/net/core/dev.c
> >index 49589ed..40c74f2 100644
> >--- a/net/core/dev.c
> >+++ b/net/core/dev.c
> >@@ -142,6 +142,7 @@
> > #include <linux/net_namespace.h>
> > #include <linux/indirect_call_wrapper.h>
> > #include <net/devlink.h>
> >+#include <net/switchdev.h>
> > 
> > #include "net-sysfs.h"
> > 
> >@@ -7377,6 +7378,11 @@ static void dev_change_rx_flags(struct net_device *dev, int flags)
> > static int __dev_set_promiscuity(struct net_device *dev, int inc, bool notify)
> > {
> > 	unsigned int old_flags = dev->flags;
> >+	struct switchdev_attr attr = {
> >+		.orig_dev = dev,
> >+		.id = SWITCHDEV_ATTR_ID_PORT_PROMISCUITY,
> >+		.flags = SWITCHDEV_F_DEFER,
> 

Hi Jiri,

> NACK
> 
> This is invalid usecase for switchdev infra. Switchdev is there for
> bridge offload purposes only.
> 
> For promiscuity changes, the infrastructure is already present in the
> code. See __dev_notify_flags(). it calls:
> call_netdevice_notifiers_info(NETDEV_CHANGE, &change_info.info)
> and you can actually see the changed flag in ".flags_changed".
Yes, you are right. But in case the port is part of a bridge and then
enable promisc mode by a user space application(tpcdump) then the drivers
will not be notified. The reason is that the dev->flags will still be the
same(because IFF_PROMISC was already set) and only promiscuity will be
changed.

One fix could be to call __dev_notify_flags() no matter when the
promisc is enable or disabled.

> 
> You just have to register netdev notifier block in your driver. Grep
> for: register_netdevice_notifier
> 
> 
> >+	};
> > 	kuid_t uid;
> > 	kgid_t gid;
> > 
> >@@ -7419,6 +7425,9 @@ static int __dev_set_promiscuity(struct net_device *dev, int inc, bool notify)
> > 	}
> > 	if (notify)
> > 		__dev_notify_flags(dev, old_flags, IFF_PROMISC);
> >+
> >+	switchdev_port_attr_set(dev, &attr);
> >+
> > 	return 0;
> > }
> > 
> >-- 
> >2.7.4
> >
> 

-- 
/Horatiu