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: <20220408183045.wpyx7tqcgcimfudu@skbuf> Date: Fri, 8 Apr 2022 21:30:45 +0300 From: Vladimir Oltean <olteanv@...il.com> To: netdev@...r.kernel.org, "David S. Miller" <davem@...emloft.net>, Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, Nicolas Dichtel <nicolas.dichtel@...nd.com> Subject: What is the purpose of dev->gflags? Hello, I am trying to understand why dev->gflags, which holds a mask of IFF_PROMISC | IFF_ALLMULTI, exists independently of dev->flags. I do see that __dev_change_flags() (called from the ioctl/rtnetlink/sysfs code paths) updates the IFF_PROMISC and IFF_ALLMULTI bits of dev->gflags, while the direct calls to dev_set_promiscuity()/ dev_set_allmulti() don't. So at first I'd be tempted to say: IFF_PROMISC | IFF_ALLMULTI are exposed to user space when set in dev->gflags, hidden otherwise. This would be consistent with the implementation of dev_get_flags(). [ side note: why is that even desirable? why does it matter who made an interface promiscuous as long as it's promiscuous? ] But in the process of digging deeper I stumbled upon Nicolas' commit 991fb3f74c14 ("dev: always advertise rx_flags changes via netlink") which I am still struggling to understand. There, a call to __dev_notify_flags(gchanges=IFF_PROMISC) was added to __dev_set_promiscuity(), called with "notify=true" from dev_set_promiscuity(). In my understanding, "gchanges" means "changes to gflags", i.e. to what user space should know about. But as discussed above, direct calls to dev_set_promiscuity() don't update dev->gflags, yet user space is notified via rtmsg_ifinfo() of the promiscuity change. Another oddity with Nicolas' commit: the other added call to __dev_notify_flags(), this time from __dev_set_allmulti(). The logic is: static int __dev_set_allmulti(struct net_device *dev, int inc, bool notify) { unsigned int old_flags = dev->flags, old_gflags = dev->gflags; dev->flags |= IFF_ALLMULTI; (bla bla, stuff that doesn't modify dev->gflags) if (dev->flags ^ old_flags) { (bla bla, more stuff that doesn't modify dev->gflags) if (notify) __dev_notify_flags(dev, old_flags, dev->gflags ^ old_gflags); ~~~~~~~~~~~~~~~~~~~~~~~~ oops, dev->gflags was never modified, so this call to __dev_notify_flags() is effectively dead code, since user space is not notified, and a NETDEV_CHANGE netdev notifier isn't emitted either, since IFF_ALLMULTI is excluded from that } return 0; } Can someone please clarify what is at least the intention? As can be seen I'm highly confused. Thanks.
Powered by blists - more mailing lists