[PATCH] fix unaligned exception in /drivers/net/wireless/orinoco.c

[PATCH] fix unaligned exception in /drivers/net/wireless/orinoco.c

[Hennerich, Michael]

This short patch prevents an unaligned exception to occur. (GCC 4.1)
tmp is defined as char pointer while it is later accessed as short.

Best regards,
Michael 

Index: linux-2.6.19.2/drivers/net/wireless/orinoco.c
===================================================================
--- linux-2.6.x/drivers/net/wireless/orinoco.c  (Revision 2649)
+++ linux-2.6.x/drivers/net/wireless/orinoco.c  (Arbeitskopie)
@@ -2053,7 +2053,7 @@
        int err;
        struct comp_id nic_id, sta_id;
        unsigned int firmver;
-       char tmp[SYMBOL_MAX_VER_LEN+1];
+       char tmp[SYMBOL_MAX_VER_LEN+1]__attribute__((aligned(2)));

        /* Get the hardware version */
        err = HERMES_READ_RECORD(hw, USER_BAP, HERMES_RID_NICID,
&nic_id);



Data access misaligned address violation
 - Attempted misaligned data memory or data cache access.
                                                         
CURRENT PROCESS:
                
COMM=insmod PID=38
TEXT = 0x03694000-0x03697d08  DATA = 0x031904f8-0x0319bc18
BSS = 0x0319bc18-0x03160000   USER-STACK = 0x0317fe60     
                                                     
return address: 0x000a326a; contents of [PC-16...PC+8]:
                                                       

RETE:  00000000  RETN: 03183d48  RETX: 000a326a  RETS: 0313a614
IPEND: 8030  SYSCFG: 0036
SEQSTAT: 00000024    SP: 03183c6c
R0: 00000000    R1: 03183d9f    R2: 0000ffff    R3: 0000c000
R4: 00000036    R5: 0000fc0e    R6: 00000012    R7: 00000000
P0: 20310036    P1: 03183d9f    P2: 00000011    P3: 03678800
P4: 03183de2    P5: 03678b84    FP: 03678b84                
A0.w: 00000000    A0.x: 00000000    A1.w: 00000000    A1.x: 00000000
LB0: 000a326a  LT0: 000a3268  LC0: 00000011
LB1: 000a291c  LT1: 000a2912  LC1: 00000000
B0: 0000004c  L0: 00000000  M0: 0000001f  I0: 0308eb04
B1: 00000067  L1: 00000000  M1: 0365ff88  I1: 03183d74
B2: 0014458c  L2: 00000000  M2: 00000000  I2: fffef6f2
B3: 00000000  L3: 00000000  M3: 00000000  I3: 00000000

USP: 0317e0e8   ASTAT: 02002060
DCPLB_FAULT_ADDR=03183da0
ICPLB_FAULT_ADDR=000a326a

Hardware Trace:
 0 Target : <0x0000c4a8> { _trap_c + 0x0 }
   Source : <0x00010018> { _exception_to_level5 + 0xb4 }
 1 Target : <0x0000ff64> { _exception_to_level5 + 0x0 } 
   Source : <0x0000ff62> { _ex_trap_c + 0x4e }         
 2 Target : <0x0000ff14> { _ex_trap_c + 0x0 } 
   Source : <0x000100b8> { _trap + 0x28 }    
 3 Target : <0x00010090> { _trap + 0x0 } 
   Source : <0x000a3268> { _insw + 0x10 }
 4 Target : <0x000a3258> { _insw + 0x0 } 
   Source : <0x0313a612> { :hermes:_hermes_read_ltv + 0xfe }
 5 Target : <0x0313a5da> { :hermes:_hermes_read_ltv + 0xc6 }
   Source : <0x0313a5b8> { :hermes:_hermes_read_ltv + 0xa4 }
 6 Target : <0x0313a5a0> { :hermes:_hermes_read_ltv + 0x8c }
   Source : <0x0313a59a> { :hermes:_hermes_read_ltv + 0x86 }
 7 Target : <0x0313a570> { :hermes:_hermes_read_ltv + 0x5c }
   Source : <0x0313a0e4> { :hermes:_hermes_bap_seek + 0x2c }
 8 Target : <0x0313a0dc> { :hermes:_hermes_bap_seek + 0x24 }
   Source : <0x0313a252> { :hermes:_hermes_bap_seek + 0x19a }
 9 Target : <0x0313a250> { :hermes:_hermes_bap_seek + 0x198 }
   Source : <0x0313a1f4> { :hermes:_hermes_bap_seek + 0x13c }
10 Target : <0x0313a1f2> { :hermes:_hermes_bap_seek + 0x13a }
   Source : <0x0313a1ea> { :hermes:_hermes_bap_seek + 0x132 }
11 Target : <0x0313a1c4> { :hermes:_hermes_bap_seek + 0x10c }
   Source : <0x0313a1f0> { :hermes:_hermes_bap_seek + 0x138 }
12 Target : <0x0313a1c4> { :hermes:_hermes_bap_seek + 0x10c }
   Source : <0x0313a1f0> { :hermes:_hermes_bap_seek + 0x138 }
13 Target : <0x0313a1c4> { :hermes:_hermes_bap_seek + 0x10c }
   Source : <0x0313a1f0> { :hermes:_hermes_bap_seek + 0x138 }
14 Target : <0x0313a1c4> { :hermes:_hermes_bap_seek + 0x10c }
   Source : <0x0313a1f0> { :hermes:_hermes_bap_seek + 0x138 }
15 Target : <0x0313a1c4> { :hermes:_hermes_bap_seek + 0x10c }
   Source : <0x0313a1f0> { :hermes:_hermes_bap_seek + 0x138 }
Stack from 03183c4c:                                         
        0000cb12 0001001c 0017e710 0017e710 0017e70c 03183d9c 000a2cce
001b6974
        000a326a 00008030 00000024 00000000 03183d48 000a326a 000a326a
0313a614
        00000000 02002060 000a291c 000a326a 000a2912 000a3268 00000000
00000011
        00000000 00000000 00000000 00000000 00000000 0014458c 00000067
0000004c
        00000000 00000000 00000000 00000000 00000000 00000000 0365ff88
0000001f
        00000000 fffef6f2 03183d74 0308eb04 0317e0e8 03678b84 03678b84
03183de2

Re: [PATCH] fix unaligned exception in /drivers/net/wireless/orinoco.c

[John W. Linville]

On Thu, Jan 18, 2007 at 09:57:18AM -0000, Hennerich, Michael wrote:
> 
> This short patch prevents an unaligned exception to occur. (GCC 4.1)
> tmp is defined as char pointer while it is later accessed as short.
> 
> Best regards,
> Michael 
> 
> Index: linux-2.6.19.2/drivers/net/wireless/orinoco.c
> ===================================================================
> --- linux-2.6.x/drivers/net/wireless/orinoco.c  (Revision 2649)
> +++ linux-2.6.x/drivers/net/wireless/orinoco.c  (Arbeitskopie)
> @@ -2053,7 +2053,7 @@
>         int err;
>         struct comp_id nic_id, sta_id;
>         unsigned int firmver;
> -       char tmp[SYMBOL_MAX_VER_LEN+1];
> +       char tmp[SYMBOL_MAX_VER_LEN+1] __attribute__ ((aligned(2)));
> 
>         /* Get the hardware version */
>         err = HERMES_READ_RECORD(hw, USER_BAP, HERMES_RID_NICID, &nic_id);

This patch seems fine, such as it is.  But, it seems like it might
also be appropriate to change hermes_read_ltv and/or hermes_read_words
to not take void * parameters?  This patch would still be needed,
but it might be more obvious to future coders?

John
-- 
John W. Linville
linville@tuxdriver.com

Re: [PATCH] fix unaligned exception in /drivers/net/wireless/orinoco.c

[Pavel Roskin]

On Fri, 2007-02-02 at 15:20 -0500, John W. Linville wrote:
> On Thu, Jan 18, 2007 at 09:57:18AM -0000, Hennerich, Michael wrote:
> > 
> > This short patch prevents an unaligned exception to occur. (GCC 4.1)
> > tmp is defined as char pointer while it is later accessed as short.

Signed-off-by: Pavel Roskin <proski@gnu.org>

> This patch seems fine, such as it is.  But, it seems like it might
> also be appropriate to change hermes_read_ltv and/or hermes_read_words
> to not take void * parameters?  This patch would still be needed,
> but it might be more obvious to future coders?

I agree.  If we use any optimization that requires alignment of the
buffer for aligned access, it needs to be clearly specified and
(ideally) enforced.

I've tried to make a patch, but it seems to be a bigger effort than I
expected.  It turns out that hermes_clear_words() is not doing what it
should (although it would only affect some buggy firmwares), and it's
clear that I just cannot replace a couple of arguments and hope for the
best.  I need to dust off my 802.11b cards and re-test everything.

I've started orinoco branch locally, and I hope I'll be able to clean
the driver from all that bitrot soon.

-- 
Regards,
Pavel Roskin