netdev - Re: [PATCH] net-driver: Drivers don't set IFF_* flag [Was: [PATCH 3/3] netdevice: order of synchronization of IFF_PROMISC and IFF

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, 23 Jun 2008 13:04:18 +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:
> Jeff Garzik said the following on 2008-6-18 10:52:
>> Drivers should not be setting IFF_* flags in set_multicast_list().
>>
>> The normal logic is that a driver interprets the request implied in
>> set_multicast_list ("promisc, all-multi, or select multi?"), and then
>> programs the hardware based on that.
>>
>> On some hardware, IFF_ALLMULTI requires that the hardware receive all
>> packets (promisc).  Even for that case, the driver should -not- be
>> setting the IFF_PROMISC flag.  It should be aware of its own hardware
>> programming state through some other method.
>>
> 
> Subject: [PATCH] net-driver: Drivers don't set IFF_* flag
> 
> Some hardware set promisc when they are requested to set IFF_ALLMULTI flag.
> It's ok, but if drivers set IFF_PROMISC flag when they set promisc,
> it will broken upper layer handle for promisc and allmulti.
> In addition, drivers can use their own hardware programming to make it.
> So do not allow drivers to set IFF_* flags.
> 
> This is a general driver fix, so I didn't split it to pieces and send
> to specific driver maintainers.

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.

> diff --git a/drivers/net/tulip/de4x5.c b/drivers/net/tulip/de4x5.c
> index bc30c6e..df22589 100644
> --- a/drivers/net/tulip/de4x5.c
> +++ b/drivers/net/tulip/de4x5.c
> @@ -5520,6 +5520,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++;
>  	break;
>  
>      case DE4X5_CLR_PROM:             /* Clear Promiscuous Mode */
> @@ -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().


--
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