Patchwork [5/6] alx: fix MAC address alignment problem

login
register
mail settings
Submitter Johannes Berg
Date June 20, 2013, 10:14 p.m.
Message ID <1371766494-18979-6-git-send-email-johannes@sipsolutions.net>
Download mbox | patch
Permalink /patch/253098/
State Changes Requested
Delegated to: David Miller
Headers show

Comments

Johannes Berg - June 20, 2013, 10:14 p.m.
In two places, parts of MAC addresses are used as u32/u16
values. This can cause alignment problems, use put_unaligned
and get_unaligned to fix this.

Reported-by: Ben Hutchings <ben@decadent.org.uk>
Signed-off-by: Johannes Berg <johannes@sipsolutions.net>
---
 drivers/net/ethernet/atheros/alx/hw.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)
David Laight - June 21, 2013, 8:38 a.m.
> In two places, parts of MAC addresses are used as u32/u16
> values. This can cause alignment problems, use put_unaligned
> and get_unaligned to fix this.

I suspect they are always 16bit aligned, so 16bit accesses
would be ok - rather than using byte accesses.
However I wouldn't know how to request this with the available
definitions....

	David

...
>  	/* addr should be big-endian */
> -	*(__be32 *)(addr + 2) = cpu_to_be32(mac0);
> -	*(__be16 *)addr = cpu_to_be16(mac1);
> +	put_unaligned(cpu_to_be32(mac0), (__be32 *)(addr + 2));
> +	put_unaligned(cpu_to_be16(mac1), (__be16 *)addr);
...
> -	val = be32_to_cpu(*(__be32 *)(addr + 2));
> +	val = be32_to_cpu(get_unaligned((__be32 *)(addr + 2)));
>  	alx_write_mem32(hw, ALX_STAD0, val);
> -	val = be16_to_cpu(*(__be16 *)addr);
> +	val = be16_to_cpu(get_unaligned((__be16 *)addr));
>  	alx_write_mem32(hw, ALX_STAD1, val);


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Johannes Berg - June 21, 2013, 8:43 a.m.
On Fri, 2013-06-21 at 09:38 +0100, David Laight wrote:
> > In two places, parts of MAC addresses are used as u32/u16
> > values. This can cause alignment problems, use put_unaligned
> > and get_unaligned to fix this.
> 
> I suspect they are always 16bit aligned, so 16bit accesses
> would be ok - rather than using byte accesses.
> However I wouldn't know how to request this with the available
> definitions....

For at least one of them, there's no such guarantee.

johannes

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

Patch

diff --git a/drivers/net/ethernet/atheros/alx/hw.c b/drivers/net/ethernet/atheros/alx/hw.c
index 2a2bbe9..6b34855 100644
--- a/drivers/net/ethernet/atheros/alx/hw.c
+++ b/drivers/net/ethernet/atheros/alx/hw.c
@@ -282,8 +282,8 @@  static bool alx_read_macaddr(struct alx_hw *hw, u8 *addr)
 	mac1 = alx_read_mem32(hw, ALX_STAD1);
 
 	/* addr should be big-endian */
-	*(__be32 *)(addr + 2) = cpu_to_be32(mac0);
-	*(__be16 *)addr = cpu_to_be16(mac1);
+	put_unaligned(cpu_to_be32(mac0), (__be32 *)(addr + 2));
+	put_unaligned(cpu_to_be16(mac1), (__be16 *)addr);
 
 	return is_valid_ether_addr(addr);
 }
@@ -326,9 +326,9 @@  void alx_set_macaddr(struct alx_hw *hw, const u8 *addr)
 	u32 val;
 
 	/* for example: 00-0B-6A-F6-00-DC * STAD0=6AF600DC, STAD1=000B */
-	val = be32_to_cpu(*(__be32 *)(addr + 2));
+	val = be32_to_cpu(get_unaligned((__be32 *)(addr + 2)));
 	alx_write_mem32(hw, ALX_STAD0, val);
-	val = be16_to_cpu(*(__be16 *)addr);
+	val = be16_to_cpu(get_unaligned((__be16 *)addr));
 	alx_write_mem32(hw, ALX_STAD1, val);
 }