[patch] staging: rtl8723au: incorrect use of ether_addr_copy()

Jes Sorensen Jes.Sorensen at redhat.com
Wed Oct 8 15:09:04 UTC 2014


Joe Perches <joe at perches.com> writes:
> On Wed, 2014-10-08 at 16:33 +0200, Jes Sorensen wrote:
>> Joe Perches <joe at perches.com> writes:
>> > On Wed, 2014-10-08 at 15:46 +0300, Dan Carpenter wrote:
>> >> On Wed, Oct 08, 2014 at 05:26:11AM -0700, Joe Perches wrote:
>> >> > On Wed, 2014-10-08 at 13:40 +0300, Dan Carpenter wrote:
>> >> > > The return from myid() isn't aligned correctly for ether_addr_copy().
>> >> > 
>> >> > Hey Dan.
>> >> > 
>> >> > Actual evidence showing ether_addr_copy conversions
>> >> > may not always be wise.
>> >> > 
>> >> > How did you find them?
>> >> 
>> >> I was just trying to see how common these kinds of bugs are.  It didn't
>> >> take long to find, but my impression is that they are rare and I got
>> >> lucky.  These kinds of bugs are tricky to find and we don't have any
>> >> tools for it.
>> >
>> > As far as I know, that's true too.
>> >
>> > Jes, was the mac_addr field in this struct
>> > ever __aligned(2)?
>> >
>> > struct eeprom_priv {
>> >        u8              bautoload_fail_flag;
>> >        u8              bloadfile_fail_flag;
>> >        u8              bloadmac_fail_flag;
>> >        /* u8           bempty; */
>> >        /* u8           sys_config; */
>> >        u8              mac_addr[6];    /* PermanentAddress */
>> > ...
>> > }
>> >
>> > As far as I can tell from git history, it was
>> > that way at the first check-in.
>> 
>> I may have removed other entries that were unused, and that caused it to
>> become mis-aligned. I can't say for sure - the fix is straight forward
>> though.
>
> One option is to add __aligned(2) to the mac_addr field
> and make no other change.

As I said in another mail, just move it to the front of the struct and
be done with it. No point in wasting alignment bytes if we don't have
to.

Jes


More information about the devel mailing list