Jeff Garzik writes:

> From: "Philippe De Muyter" <[EMAIL PROTECTED]>
> 
> Avoid ppp-generated kernel crashes on machines where unaligned accesses are
> forbidden (ie: 68000-based CPUs)

This patch seems a bit strange and/or incomplete.  Are we trying to
get 2-byte alignment or 4-byte alignment of the payload?  It seems
that if the protocol field is uncompressed, we don't do anything to
the alignment, but if it is compressed, we do this:

>               /* 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;

I'm puzzled that we are not testing ((unsigned long)skb->data & 2) if
we are really trying to achieve 4-byte alignment.  In fact, if the
skb->data that we get from dev_alloc_skb is 4-byte aligned to start
with, this will end up with the payload starting at the original
skb->data + 6, i.e. 2-byte aligned but not 4-byte aligned AFAICS.

Can we assume that dev_alloc_skb will give us a 4-byte aligned
skb->data?  If we can then I suggest we change 3 to 1 in the skb_put
and memmove above, and get rid of the if (since its condition will
always be false).

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

Reply via email to