netdev - Re: [PATCH] net: meth: Add set_rx_mode hook to fix ICMPv6 neighbor discovery

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:	Sat, 17 Dec 2011 21:56:30 -0500 (EST)
From:	David Miller <davem@...emloft.net>
To:	kumba@...too.org
Cc:	netdev@...r.kernel.org, linux-mips@...ux-mips.org
Subject: Re: [PATCH] net: meth: Add set_rx_mode hook to fix ICMPv6 neighbor
 discovery

From: Joshua Kinard <kumba@...too.org>
Date: Sat, 17 Dec 2011 19:56:29 -0500

> +/* Maximum number of multicast addresses to filter (vs. Rx-all-multicast).
> + * MACE Ethernet uses a 64 element hash table based on the Ethernet CRC.
> + */
> +static int multicast_filter_limit = 32;
> +
> +

Unnecessary empty line, only one is sufficient.  I also don't see a reason
to even define this value.  If it's a constant then use a const type.

> +	/* Multicast filter. */
> +	unsigned long mcast_filter;
> +
 ...
> +		priv->mcast_filter = 0xffffffffffffffffUL;

You're assuming that unsigned long is 64-bits here.  You need to use a
type which matches your expections regardless of the architecture that
the code is built on.

> +		netdev_for_each_mc_addr(ha, dev)
> +			set_bit((ether_crc(ETH_ALEN, ha->addr) >> 26),
> +				    (volatile long unsigned int *)&priv->mcast_filter);

This makes an assumption not only about the size of the "unsigned long"
type, but also of the endianness of the architecture this runs on.

Please recode this to remove both assumptions.
--
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