netdev - Re: RFC: [PATCH 2/3] netdevice: Fix promiscuity and allmulti overflow

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, 16 Jun 2008 11:38:36 +0200
From:	Patrick McHardy <kaber@...sh.net>
To:	Wang Chen <wangchen@...fujitsu.com>
CC:	"David S. Miller" <davem@...emloft.net>,
	NETDEV <netdev@...r.kernel.org>
Subject: Re: RFC: [PATCH 2/3] netdevice: Fix promiscuity and allmulti overflow

Wang Chen wrote:
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 5829630..a3c692d 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -2753,10 +2753,20 @@ static void __dev_set_promiscuity(struct net_device *dev, int inc)
>  
>  	ASSERT_RTNL();
>  
> +	dev->flags |= IFF_PROMISC;
>  	if ((dev->promiscuity += inc) == 0)
> -		dev->flags &= ~IFF_PROMISC;
> -	else
> -		dev->flags |= IFF_PROMISC;
> +		/*
> +		 * Avoid overflow.
> +		 * If inc causes overflow, ignore it and warn user.
> +		 */
> +		if (inc < 0)
> +			dev->flags &= ~IFF_PROMISC;
> +		else {
> +			dev->promiscuity -= inc;
> +			printk(KERN_ERR "%s: promiscuity touches roof, "
> +				"set promiscuity failed, promiscuity feature "
> +				"of device will be broken.\n");
> +		}

Additional parens around the inner block would make this more
readable.

I question the need for this though, userspace can only trigger
an increase/decrease by one no matter how often it enables
the ALLMULTI/PROMISC flags, and I doubt any codepath in the
kernel would lead to an overflow.

If this can really happen it would be better to leave the
counter untouched and return an error, we already have too
many device operations that might fail more or less silently.


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