Search Linux Wireless

Re: [PATCH] ssb: fix unaligned access to mac address

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 03/09/2013 05:02 PM, Rafał Miłecki wrote:
2013/2/21 Larry Finger <Larry.Finger@xxxxxxxxxxxx>:
On 02/20/2013 02:11 PM, Joe Perches wrote:

On Wed, 2013-02-20 at 20:17 +0100, Hauke Mehrtens wrote:

On 02/20/2013 07:29 PM, Joe Perches wrote:

On Wed, 2013-02-20 at 11:56 -0600, Larry Finger wrote:

On 02/20/2013 11:31 AM, Joe Perches wrote:

On Mon, 2013-02-18 at 17:09 +0100, Rafał Miłecki wrote:

2013/2/16 Hauke Mehrtens <hauke@xxxxxxxxxx>:

The mac address should be aligned to u16 to prevent an unaligned
access
in drivers/ssb/pci.c where it is casted to __be16.

[]

diff --git a/include/linux/ssb/ssb.h b/include/linux/ssb/ssb.h

[]

@@ -26,6 +26,7 @@ struct ssb_sprom_core_pwr_info {

    struct ssb_sprom {
           u8 revision;
+       u8 country_code;        /* Country Code */
           u8 il0mac[6];           /* MAC address for 802.11b/g */


It looks a little hacky to me too, it's easy to forget about that
requirement and break that again in the future.

What about not casting il0mac to u16 at all? Maybe we should just
fill
it as u8 (which it is)?


Perhaps this?

From: Joe Perches <joe@xxxxxxxxxxx>
Subject: [PATCH] ssb: pci: Standardize a function to get mac address

Don't require alignment of mac addresses to u16.


...

I like the looks of sprom_get_mac() over that ugly *(((__be16
*)out->il0mac)
construct, but this patch breaks ssb. The resulting MAC address is all
ones. I
have not yet figured out the problem.


Dunno, I must have done something stupid.

I don't have one of these but the transform
looked correct when I did  it.

I'm not sure it's the best solution anyway
because some of the other ether address
functions like compare_ether_addr also
require 2 byte alignment and cast to u16.


Your patch looks good (haven't tested it), but as you said we should
still align the mac addresses to u16 so functions like
compare_ether_addr work with them.

There is also a v2 of my patch:
http://www.spinics.net/lists/linux-wireless/msg103628.html


Seems OK to me, though I suggest using ETH_ALEN
instead of 6.

Maybe using both patches would be better still.

And as long as I'm futzing with ssb, there's another
logging cleanup to go with it.

Here's a V2 with the missing SPOFF fixed.

Subject: [PATCH 1/2] ssb: pci: Standardize a function to get mac address
From: Joe Perches <joe@xxxxxxxxxxx>

Don't require alignment of mac addresses to u16.

Signed-off-by: Joe Perches <joe@xxxxxxxxxxx>
---

V2: Add missing SPOFF


Tested-by: Larry Finger <Larry.Finger@xxxxxxxxxxxx>

I didn't confirm my issue to be related to this patch, but with recent
kernels my MAC has changed from
00:11:22:33:44:55
to
11:00:33:22:55:44

Larry: did you verify MAC didn't change for you?

I just checked on x86_64, and the bytes are swapped here too. Do you want to push the patch, or should I do it?

Larry


--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Host AP]     [ATH6KL]     [Linux Wireless Personal Area Network]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Linux Kernel]     [IDE]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite Hiking]     [MIPS Linux]     [ARM Linux]     [Linux RAID]

  Powered by Linux