netdev - Re: [PATCH 4/11] use ether_addr_equal

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, 6 Jan 2014 13:48:02 +0300
From:	Dan Carpenter <dan.carpenter@...cle.com>
To:	Johannes Berg <johannes@...solutions.net>
Cc:	Henrique de Moraes Holschuh <hmh@....eng.br>,
	Julia Lawall <julia.lawall@...6.fr>,
	kernel-janitors@...r.kernel.org,
	Emmanuel Grumbach <emmanuel.grumbach@...el.com>,
	Intel Linux Wireless <ilw@...ux.intel.com>,
	"John W. Linville" <linville@...driver.com>,
	linux-wireless@...r.kernel.org, netdev@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 4/11]  use ether_addr_equal_64bits

On Tue, Dec 31, 2013 at 12:13:08AM +0100, Johannes Berg wrote:
> On Mon, 2013-12-30 at 19:57 -0200, Henrique de Moraes Holschuh wrote:
> > On Mon, 30 Dec 2013, Johannes Berg wrote:
> > > On Mon, 2013-12-30 at 20:58 +0100, Julia Lawall wrote:
> > > > > Is there any way we could catch (sparse, or some other script?) that
> > > > > struct reorganising won't break the condition needed ("within a
> > > > > structure that contains at least two more bytes")?
> > > > 
> > > > What kind of reorganizing could happen?  Do you mean that the programmer 
> > > > might do at some time in the future, or something the compiler might do?
> > > 
> > > I'm just thinking of a programmer, e.g. changing a struct like this:
> > > 
> > >  struct foo {
> > >    u8 addr[ETH_ALEN];
> > > -  u16 dummy;
> > >  };
> > > 
> > > for example.
> > 
> > That is easily resolved by:
> > 
> > struct foo {
> > 	u8 addr[ETH_ALEN];
> > 	u16 required_padding;	/* do not remove upon pain of death */
> > };
> 
> That'd be a stupid waste of struct space. If anything, there should be
> *only* a comment saying that at least two bytes are needed - I'd still
> prefer an automated check.
> 

This is the sort of thing where Smatch is probably the right tool.  I'll
let you know how it turns out.

My guess is that it would be rare to run into this bug in real life.
Most structs have 4 or 8 byte alignment and most times the addr is not
at the end of the struct.  But I also agree that this function should
only be used in a fast path.

regards,
dan carpenter

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