* [PATCH] Fix SPARC64 unaligned access in zd_mac_rx
@ 2009-07-31 7:59 Patrick Simmons
2009-07-31 8:39 ` Johannes Berg
0 siblings, 1 reply; 11+ messages in thread
From: Patrick Simmons @ 2009-07-31 7:59 UTC (permalink / raw)
To: linux-wireless; +Cc: Daniel Drake, Ulrich Kunitz
I'm running zd1211rw on SPARC64 and was getting a lot of "unaligned
access" messages in dmesg. I tracked the problem down to this line, and
changing the cast to a memcpy fixes the issue.
Please CC me with any comments as I am not subscribed to the list.
Signed-off-by: Patrick Simmons <linuxrocks123@netscape.net>
--- a/drivers/net/wireless/zd1211rw/zd_mac.c 2009-07-30
23:23:50.000000000 -0600
+++ b/drivers/net/wireless/zd1211rw/zd_mac.c 2009-07-30
23:58:19.000000000 -0600
@@ -694,7 +694,7 @@
&& !mac->pass_ctrl)
return 0;
- fc = *(__le16 *)buffer;
+ memcpy(&fc,buffer,sizeof(__le16));
need_padding = ieee80211_is_data_qos(fc) ^ ieee80211_has_a4(fc);
skb = dev_alloc_skb(length + (need_padding ? 2 : 0));
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Fix SPARC64 unaligned access in zd_mac_rx
2009-07-31 7:59 [PATCH] Fix SPARC64 unaligned access in zd_mac_rx Patrick Simmons
@ 2009-07-31 8:39 ` Johannes Berg
2009-08-01 5:23 ` David Miller
0 siblings, 1 reply; 11+ messages in thread
From: Johannes Berg @ 2009-07-31 8:39 UTC (permalink / raw)
To: Patrick Simmons; +Cc: linux-wireless, Daniel Drake, Ulrich Kunitz
[-- Attachment #1: Type: text/plain, Size: 469 bytes --]
On Fri, 2009-07-31 at 01:59 -0600, Patrick Simmons wrote:
> I'm running zd1211rw on SPARC64 and was getting a lot of "unaligned
> access" messages in dmesg. I tracked the problem down to this line, and
> changing the cast to a memcpy fixes the issue.
>
> - fc = *(__le16 *)buffer;
> + memcpy(&fc,buffer,sizeof(__le16));
> need_padding = ieee80211_is_data_qos(fc) ^ ieee80211_has_a4(fc);
Please use get_unaligned instead.
johannes
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Fix SPARC64 unaligned access in zd_mac_rx
2009-07-31 8:39 ` Johannes Berg
@ 2009-08-01 5:23 ` David Miller
2009-08-01 10:40 ` Michael Buesch
0 siblings, 1 reply; 11+ messages in thread
From: David Miller @ 2009-08-01 5:23 UTC (permalink / raw)
To: johannes; +Cc: linuxrocks123, linux-wireless, dsd, kune
From: Johannes Berg <johannes@sipsolutions.net>
Date: Fri, 31 Jul 2009 10:39:43 +0200
> On Fri, 2009-07-31 at 01:59 -0600, Patrick Simmons wrote:
>> I'm running zd1211rw on SPARC64 and was getting a lot of "unaligned
>> access" messages in dmesg. I tracked the problem down to this line, and
>> changing the cast to a memcpy fixes the issue.
>>
>> - fc = *(__le16 *)buffer;
>> + memcpy(&fc,buffer,sizeof(__le16));
>> need_padding = ieee80211_is_data_qos(fc) ^ ieee80211_has_a4(fc);
>
> Please use get_unaligned instead.
And, more specifically, one of the endian get_unaligned variants.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Fix SPARC64 unaligned access in zd_mac_rx
2009-08-01 5:23 ` David Miller
@ 2009-08-01 10:40 ` Michael Buesch
2009-08-01 16:12 ` David Miller
0 siblings, 1 reply; 11+ messages in thread
From: Michael Buesch @ 2009-08-01 10:40 UTC (permalink / raw)
To: David Miller; +Cc: johannes, linuxrocks123, linux-wireless, dsd, kune
On Saturday 01 August 2009 07:23:50 David Miller wrote:
> From: Johannes Berg <johannes@sipsolutions.net>
> Date: Fri, 31 Jul 2009 10:39:43 +0200
>
> > On Fri, 2009-07-31 at 01:59 -0600, Patrick Simmons wrote:
> >> I'm running zd1211rw on SPARC64 and was getting a lot of "unaligned
> >> access" messages in dmesg. I tracked the problem down to this line, and
> >> changing the cast to a memcpy fixes the issue.
> >>
> >> - fc = *(__le16 *)buffer;
> >> + memcpy(&fc,buffer,sizeof(__le16));
> >> need_padding = ieee80211_is_data_qos(fc) ^ ieee80211_has_a4(fc);
> >
> > Please use get_unaligned instead.
>
> And, more specifically, one of the endian get_unaligned variants.
No. fc is supposed to be LE.
--
Greetings, Michael.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Fix SPARC64 unaligned access in zd_mac_rx
2009-08-01 10:40 ` Michael Buesch
@ 2009-08-01 16:12 ` David Miller
2009-08-02 1:41 ` Patrick Simmons
0 siblings, 1 reply; 11+ messages in thread
From: David Miller @ 2009-08-01 16:12 UTC (permalink / raw)
To: mb; +Cc: johannes, linuxrocks123, linux-wireless, dsd, kune
From: Michael Buesch <mb@bu3sch.de>
Date: Sat, 1 Aug 2009 12:40:16 +0200
> On Saturday 01 August 2009 07:23:50 David Miller wrote:
>> From: Johannes Berg <johannes@sipsolutions.net>
>> Date: Fri, 31 Jul 2009 10:39:43 +0200
>>
>> > On Fri, 2009-07-31 at 01:59 -0600, Patrick Simmons wrote:
>> >> I'm running zd1211rw on SPARC64 and was getting a lot of "unaligned
>> >> access" messages in dmesg. I tracked the problem down to this line, and
>> >> changing the cast to a memcpy fixes the issue.
>> >>
>> >> - fc = *(__le16 *)buffer;
>> >> + memcpy(&fc,buffer,sizeof(__le16));
>> >> need_padding = ieee80211_is_data_qos(fc) ^ ieee80211_has_a4(fc);
>> >
>> > Please use get_unaligned instead.
>>
>> And, more specifically, one of the endian get_unaligned variants.
>
> No. fc is supposed to be LE.
Ok, I see.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Fix SPARC64 unaligned access in zd_mac_rx
2009-08-01 16:12 ` David Miller
@ 2009-08-02 1:41 ` Patrick Simmons
2009-08-02 7:50 ` Johannes Berg
0 siblings, 1 reply; 11+ messages in thread
From: Patrick Simmons @ 2009-08-02 1:41 UTC (permalink / raw)
To: David Miller; +Cc: mb, johannes, linux-wireless, dsd, kune
David Miller wrote:
> From: Michael Buesch <mb@bu3sch.de>
> Date: Sat, 1 Aug 2009 12:40:16 +0200
>
>> On Saturday 01 August 2009 07:23:50 David Miller wrote:
>>> From: Johannes Berg <johannes@sipsolutions.net>
>>> Date: Fri, 31 Jul 2009 10:39:43 +0200
>>>
>>>> On Fri, 2009-07-31 at 01:59 -0600, Patrick Simmons wrote:
>>>>> I'm running zd1211rw on SPARC64 and was getting a lot of "unaligned
>>>>> access" messages in dmesg. I tracked the problem down to this line, and
>>>>> changing the cast to a memcpy fixes the issue.
>>>>>
>>>>> - fc = *(__le16 *)buffer;
>>>>> + memcpy(&fc,buffer,sizeof(__le16));
>>>>> need_padding = ieee80211_is_data_qos(fc) ^ ieee80211_has_a4(fc);
>>>> Please use get_unaligned instead.
>>> And, more specifically, one of the endian get_unaligned variants.
>> No. fc is supposed to be LE.
>
> Ok, I see.
Revised patch below.
Signed-off-by: Patrick Simmons <linuxrocks123@netscape.net>
--- a/drivers/net/wireless/zd1211rw/zd_mac.c
+++ b/drivers/net/wireless/zd1211rw/zd_mac.c
@@ -694,7 +694,7 @@
&& !mac->pass_ctrl)
return 0;
- fc = *(__le16 *)buffer;
+ fc = get_unaligned(buffer);
need_padding = ieee80211_is_data_qos(fc) ^ ieee80211_has_a4(fc);
skb = dev_alloc_skb(length + (need_padding ? 2 : 0));
--
If I'm not here, I've gone out to find myself. If I get back before I
return, please keep me here.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Fix SPARC64 unaligned access in zd_mac_rx
2009-08-02 1:41 ` Patrick Simmons
@ 2009-08-02 7:50 ` Johannes Berg
2009-08-02 8:24 ` Patrick Simmons
2009-08-02 8:46 ` [PATCH] zd1211rw: fix " Patrick Simmons
0 siblings, 2 replies; 11+ messages in thread
From: Johannes Berg @ 2009-08-02 7:50 UTC (permalink / raw)
To: Patrick Simmons; +Cc: David Miller, mb, linux-wireless, dsd, kune
[-- Attachment #1: Type: text/plain, Size: 573 bytes --]
On Sat, 2009-08-01 at 19:41 -0600, Patrick Simmons wrote:
> --- a/drivers/net/wireless/zd1211rw/zd_mac.c
> +++ b/drivers/net/wireless/zd1211rw/zd_mac.c
> @@ -694,7 +694,7 @@
> && !mac->pass_ctrl)
> return 0;
>
> - fc = *(__le16 *)buffer;
> + fc = get_unaligned(buffer);
Now the code is completely incorrect -- you're now loading the lower 16
bits of the 'buffer' _pointer_ into the variable as an unaligned load.
It really needs to be
fc = get_unaligned((__le16 *)buffer);
I think.
johannes
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Fix SPARC64 unaligned access in zd_mac_rx
2009-08-02 7:50 ` Johannes Berg
@ 2009-08-02 8:24 ` Patrick Simmons
2009-08-02 8:34 ` Johannes Berg
2009-08-02 8:46 ` [PATCH] zd1211rw: fix " Patrick Simmons
1 sibling, 1 reply; 11+ messages in thread
From: Patrick Simmons @ 2009-08-02 8:24 UTC (permalink / raw)
To: Johannes Berg; +Cc: David Miller, mb, linux-wireless, dsd, kune
Johannes Berg wrote:
> Now the code is completely incorrect -- you're now loading the lower 16
> bits of the 'buffer' _pointer_ into the variable as an unaligned load.
>
> It really needs to be
>
> fc = get_unaligned((__le16 *)buffer);
>
> I think.
>
> johannes
>
It does look wrong, now that I look at it further, but I'm pretty sure
that what was happening was that only the lower 8 bits of the
dereferenced value were being accessed. What's odd is that it works on
my machine, though. Whatever. Your version is clearer, in any case.
What follows is hopefully the last version of this patch.
Signed-off-by: Patrick Simmons <linuxrocks123@netscape.net>
--- a/drivers/net/wireless/zd1211rw/zd_mac.c
+++ b/drivers/net/wireless/zd1211rw/zd_mac.c
@@ -694,7 +694,7 @@
&& !mac->pass_ctrl)
return 0;
- fc = *(__le16 *)buffer;
+ fc = get_unaligned((__le16*)buffer);
need_padding = ieee80211_is_data_qos(fc) ^ ieee80211_has_a4(fc);
skb = dev_alloc_skb(length + (need_padding ? 2 : 0));
--Patrick
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Fix SPARC64 unaligned access in zd_mac_rx
2009-08-02 8:24 ` Patrick Simmons
@ 2009-08-02 8:34 ` Johannes Berg
2009-08-02 8:45 ` Patrick Simmons
0 siblings, 1 reply; 11+ messages in thread
From: Johannes Berg @ 2009-08-02 8:34 UTC (permalink / raw)
To: Patrick Simmons; +Cc: David Miller, mb, linux-wireless, dsd, kune
[-- Attachment #1: Type: text/plain, Size: 1334 bytes --]
On Sun, 2009-08-02 at 02:24 -0600, Patrick Simmons wrote:
> It does look wrong, now that I look at it further, but I'm pretty sure
> that what was happening was that only the lower 8 bits of the
> dereferenced value were being accessed.
Yes, you're right, because buffer is a "u8 *", I initially thought that
it didn't take a pointer, but that wouldn't work, of course.
> What's odd is that it works on
> my machine, though. Whatever. Your version is clearer, in any case.
>
> What follows is hopefully the last version of this patch.
Looks good, but I think you should resubmit with
* a changed subject (e.g. "zd1211rw: fix unaligned access in zd_mac_rx")
* a standalone patch description so John doesn't have to pick it out of
your first patch etc.
Thanks!
johannes
> Signed-off-by: Patrick Simmons <linuxrocks123@netscape.net>
>
> --- a/drivers/net/wireless/zd1211rw/zd_mac.c
> +++ b/drivers/net/wireless/zd1211rw/zd_mac.c
> @@ -694,7 +694,7 @@
> && !mac->pass_ctrl)
> return 0;
>
> - fc = *(__le16 *)buffer;
> + fc = get_unaligned((__le16*)buffer);
> need_padding = ieee80211_is_data_qos(fc) ^ ieee80211_has_a4(fc);
>
> skb = dev_alloc_skb(length + (need_padding ? 2 : 0));
>
> --Patrick
>
>
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Fix SPARC64 unaligned access in zd_mac_rx
2009-08-02 8:34 ` Johannes Berg
@ 2009-08-02 8:45 ` Patrick Simmons
0 siblings, 0 replies; 11+ messages in thread
From: Patrick Simmons @ 2009-08-02 8:45 UTC (permalink / raw)
To: Johannes Berg; +Cc: David Miller, mb, linux-wireless, dsd, kune
Johannes Berg wrote:
> On Sun, 2009-08-02 at 02:24 -0600, Patrick Simmons wrote:
>
>
>> It does look wrong, now that I look at it further, but I'm pretty sure
>> that what was happening was that only the lower 8 bits of the
>> dereferenced value were being accessed.
>>
>
> Yes, you're right, because buffer is a "u8 *", I initially thought that
> it didn't take a pointer, but that wouldn't work, of course.
>
>
>> What's odd is that it works on
>> my machine, though. Whatever. Your version is clearer, in any case.
>>
>> What follows is hopefully the last version of this patch.
>>
>
> Looks good, but I think you should resubmit with
> * a changed subject (e.g. "zd1211rw: fix unaligned access in zd_mac_rx")
> * a standalone patch description so John doesn't have to pick it out of
> your first patch etc.
>
>
> Thanks!
> johannes
>
>
Sure thing; here it comes. Thanks for the review.
--Patrick
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH] zd1211rw: fix unaligned access in zd_mac_rx
2009-08-02 7:50 ` Johannes Berg
2009-08-02 8:24 ` Patrick Simmons
@ 2009-08-02 8:46 ` Patrick Simmons
1 sibling, 0 replies; 11+ messages in thread
From: Patrick Simmons @ 2009-08-02 8:46 UTC (permalink / raw)
To: linux-wireless
Cc: Johannes Berg, David Miller, mb, dsd, kune, Patrick Simmons
Fix an unaligned memory access in the zd_mac_rx function of zd1211rw
that causes problems on SPARC64.
Signed-off-by: Patrick Simmons <linuxrocks123@netscape.net>
--- a/drivers/net/wireless/zd1211rw/zd_mac.c
+++ b/drivers/net/wireless/zd1211rw/zd_mac.c
@@ -694,7 +694,7 @@
&& !mac->pass_ctrl)
return 0;
- fc = *(__le16 *)buffer;
+ fc = get_unaligned((__le16*)buffer);
need_padding = ieee80211_is_data_qos(fc) ^ ieee80211_has_a4(fc);
skb = dev_alloc_skb(length + (need_padding ? 2 : 0));
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2009-08-02 8:56 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-07-31 7:59 [PATCH] Fix SPARC64 unaligned access in zd_mac_rx Patrick Simmons
2009-07-31 8:39 ` Johannes Berg
2009-08-01 5:23 ` David Miller
2009-08-01 10:40 ` Michael Buesch
2009-08-01 16:12 ` David Miller
2009-08-02 1:41 ` Patrick Simmons
2009-08-02 7:50 ` Johannes Berg
2009-08-02 8:24 ` Patrick Simmons
2009-08-02 8:34 ` Johannes Berg
2009-08-02 8:45 ` Patrick Simmons
2009-08-02 8:46 ` [PATCH] zd1211rw: fix " Patrick Simmons
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).