[PATCH 2.6.13-rc3] fix unaligned memory access in snsc_event.c

[PATCH 2.6.13-rc3] fix unaligned memory access in snsc_event.c

[Greg Howard]

Hi Tony,

It's been pointed out that environmental events from the system
controllers on Altix machines cause the kernel to complain about
unaligned memory accesses.  This turns out to be because
"be32_to_cpup()" didn't do everything I thought/hoped it did.
I've added a new function to the file that copies a (big-endian)
integer out of a byte buffer and into a little-endian integer
variable.  Please apply the following patch if it looks right to
you.

Thanks!
- Greg Howard, SGI

Signed-off-by: Greg Howard <ghoward@sgi.com>

diff -uprN -X 0/Documentation/dontdiff 0/drivers/char/snsc_event.c 1/drivers/char/snsc_event.c
--- 0/drivers/char/snsc_event.c	2005-07-12 23:46:46 -05:00
+++ 1/drivers/char/snsc_event.c	2005-07-26 11:34:06 -05:00
@@ -26,6 +26,18 @@ static struct subch_data_s *event_sd;
 void scdrv_event(unsigned long);
 DECLARE_TASKLET(sn_sysctl_event, scdrv_event, 0);

+static int
+copy_buffer_to_int(char *buffer)
+{
+	int i, result = 0;
+	for(i = 0; i < sizeof(result); i++ ) {
+		result |= ((unsigned)(*(unsigned char *)buffer++)
+				<< (8 * ((sizeof(result) - i) - 1)));
+	}
+	return result;
+}
+
+
 /*
  * scdrv_event_interrupt
  *
@@ -64,11 +76,11 @@ scdrv_parse_event(char *event, int *src,
 	char *desc_end;

 	/* record event source address */
-	*src = be32_to_cpup((__be32 *)event);
+	*src = copy_buffer_to_int(event);
 	event += 4; 			/* move on to event code */

 	/* record the system controller's event code */
-	*code = be32_to_cpup((__be32 *)event);
+	*code = copy_buffer_to_int(event);
 	event += 4;			/* move on to event arguments */

 	/* how many arguments are in the packet? */
@@ -82,7 +94,7 @@ scdrv_parse_event(char *event, int *src,
 		/* not an integer argument, so give up */
 		return -1;
 	}
-	*esp_code = be32_to_cpup((__be32 *)event);
+	*esp_code = copy_buffer_to_int(event);
 	event += 4;

 	/* parse out the event description */

Re: [PATCH 2.6.13-rc3] fix unaligned memory access in snsc_event.c

[Andreas Schwab]

Greg Howard <ghoward@sgi.com> writes:

> I've added a new function to the file that copies a (big-endian)
> integer out of a byte buffer and into a little-endian integer
> variable.

How about using get_unaligned?

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [PATCH 2.6.13-rc3] fix unaligned memory access in snsc_event.c

[Greg Howard]

On Tue, 26 Jul 2005, Andreas Schwab wrote:

> How about using get_unaligned?

Good idea.  I didn't know this function existed when I submitted
the previous patch.

Here's a revised version:


It's been pointed out that environmental events from the system
controllers on Altix machines cause the kernel to complain about
unaligned memory accesses.  This turns out to be because
"be32_to_cpup()" didn't do everything I thought/hoped it did.
I've added calls to pull the offending integers out of the
buffers using get_unaligned() before feeding them to
be32_to_cpup().

Thanks!
- Greg Howard, SGI

Signed-off-by: Greg Howard <ghoward@sgi.com>

--- 0/drivers/char/snsc_event.c	2005-07-12 23:46:46 -05:00
+++ 1/drivers/char/snsc_event.c	2005-07-28 15:21:03 -05:00
@@ -19,6 +19,7 @@
 #include <linux/sched.h>
 #include <linux/byteorder/generic.h>
 #include <asm/sn/sn_sal.h>
+#include <asm/unaligned.h>
 #include "snsc.h"

 static struct subch_data_s *event_sd;
@@ -62,13 +63,16 @@ static int
 scdrv_parse_event(char *event, int *src, int *code, int *esp_code, char *desc)
 {
 	char *desc_end;
+	__be32 from_buf;

 	/* record event source address */
-	*src = be32_to_cpup((__be32 *)event);
+	from_buf = get_unaligned((__be32 *)event);
+	*src = be32_to_cpup(&from_buf);
 	event += 4; 			/* move on to event code */

 	/* record the system controller's event code */
-	*code = be32_to_cpup((__be32 *)event);
+	from_buf = get_unaligned((__be32 *)event);
+	*code = be32_to_cpup(&from_buf);
 	event += 4;			/* move on to event arguments */

 	/* how many arguments are in the packet? */
@@ -82,7 +86,8 @@ scdrv_parse_event(char *event, int *src,
 		/* not an integer argument, so give up */
 		return -1;
 	}
-	*esp_code = be32_to_cpup((__be32 *)event);
+	from_buf = get_unaligned((__be32 *)event);
+	*esp_code = be32_to_cpup(&from_buf);
 	event += 4;

 	/* parse out the event description */