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
| ||
|
Date: Mon, 23 Jun 2008 15:47:14 +0200 From: Patrick McHardy <kaber@...sh.net> To: Wang Chen <wangchen@...fujitsu.com> CC: Jeff Garzik <jgarzik@...ox.com>, Alan Cox <alan@...rguk.ukuu.org.uk>, "David S. Miller" <davem@...emloft.net>, NETDEV <netdev@...r.kernel.org> Subject: Re: [PATCH] net-driver: Drivers don't set IFF_* flag [Was: [PATCH 3/3] netdevice: order of synchronization of IFF_PROMISC and IFF_ALLMULTI] Wang Chen wrote: > Patrick McHardy said the following on 2008-6-23 19:04: >> Did you check that these drivers don't use the PROMISC flag they >> set themselves somewhere? As Jeff said, they might use it to be >> aware of their hardware programming state. >> > > Yes. I checked. > The flag is set but not be used anywhere else. > All of the drivers set their own state and at the same time > set IFF_PROMIDC flag. > I think that by setting IFF_PROMISC the drivers want to inform > upper layer that they set hardware to promisc although they are > requested to set ALLMULTI. > But the driver's redundant action is unneeded. > Because, if the hardwares have to set promisc mode when they > required to receive all multicast packets, it's ok, upper layer > don't need to be informed. > Only if allmulti and promiscuity all be zero, the promisc mode will be off. OK, thanks for the explanation. >>> @@ -5528,6 +5529,7 @@ de4x5_ioctl(struct net_device *dev, struct ifreq >>> *rq, int cmd) >>> omr &= ~OMR_PR; >>> outl(omr, DE4X5_OMR); >>> dev->flags &= ~IFF_PROMISC; >>> + dev->promiscuity = 0; >>> break; >> Shouldn't this be using dev_set_promiscuity(). >> I actually meant dev_change_flags(), sorry. > No. > 1. dev_set_promiscuity do > a. set/unset IFF_PROMISC > b. promiscuity++/-- > c. audit > d. dev_set_rx_mode (upload unicast and multicast list to device) > Here, in ioctl, a & b is enough. Auditing should certainly be done if promiscous mode is set. Calling dev_set_rx_mode doesn't hurt, even if it does the ioctl handler could be changed not to care. Besides this is neither taking the rtnl_mutex as required nor sending notifcations to userspace. > 2. dev->flags unset IFF_PROMISC and dev->promiscuity = 0 can not be > replaced by dev_set_promiscuity(). Because, we don't decrease > promiscuity here, but set promiscuity zero for unset IFF_PROMISC. And that looks like a bug, the driver shouldn't disable promiscuity if something still requires it. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@...r.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists