Re: PATCH : ppp + big-endian = kernel crash

[Philippe De Muyter]

David S. Miller wrote :
> From: Andrew Morton <akpm@xxxxxxxx>
> Date: Sun, 29 May 2005 19:52:45 -0700
> 
> > > All these patches to PPP and friends are merely papering over the
> > > larger problem.
> > 
> > It's not a thing we want to do in the general case, sure.  But it's
> > reasonable to identify those bits of net code which the nommu people care
> > about and look to see if there's some sane workaround to get them going.
> > 
> > Otherwise, things like PPP will simply unavailable to some architectures...
> 
> Some time ago there was a proposal that would allow appropriate
> handling of these sorts of things.
> 
> Accessors to packet headers would go through a macro, and this
> along with some other defines would allow an architecture to
> decide between two schemes:
> 
> 1) Use normal loads and stores, let trap handler take care of
>    unaligned cases.
> 2) Use something akin to get_unaligned(), no trap handler stuff.
> 
> Sure, to make things faster we can do something like this PPP
> patch, but it needs lots of work, first of all you need to
> replace this:
> 
> 	for ( ... )
> 		p[i-1] = p[i];
> 
> stuff with a proper memmove() call.
> 

Ok, here is a revised patch :

This patch avoids ppp-generated kernel crashes on machines where
unaligned accesses are forbidden.

Signed-off-by: Philippe De Muyter <phdm@xxxxxxxxx>

--- linux/drivers/net/ppp_async.c.orig	2005-06-01 10:46:58.000000000 +0200
+++ linux/drivers/net/ppp_async.c	2005-06-01 17:04:13.000000000 +0200
@@ -31,6 +31,7 @@
 #include <linux/spinlock.h>
 #include <linux/init.h>
 #include <asm/uaccess.h>
+#include <asm/string.h>
 
 #define PPP_VERSION	"2.4.2"
 
@@ -816,7 +817,15 @@ process_input_packet(struct asyncppp *ap
 	proto = p[0];
 	if (proto & 1) {
 		/* protocol is compressed */
-		skb_push(skb, 1)[0] = 0;
+		if ((unsigned long)skb->data & 1)
+			skb_push(skb, 1)[0] = 0;
+		else { /* Ditto, but realign the payload to 4-byte boundary */
+			short len = skb->len;
+
+			skb_put(skb, 3);
+			memmove(skb->data + 3, skb->data, len);
+			skb_pull(skb, 2)[0] = 0;
+		}
 	} else {
 		if (skb->len < 2)
 			goto err;
@@ -890,6 +899,12 @@ ppp_async_input(struct asyncppp *ap, con
 				if (skb == 0)
 					goto nomem;
 				/* Try to get the payload 4-byte aligned */
+				/* This should match the
+				** PPP_ALLSTATIONS/PPP_UI/compressed tests
+				** in process_input_packet,
+				** but we do not have enough chars here and
+				** now to test buf[1] and buf[2].
+				*/
 				if (buf[0] != PPP_ALLSTATIONS)
 					skb_reserve(skb, 2 + (buf[0] & 1));
 				ap->rpkt = skb;
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/