| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Mon, 09 Mar 2009 22:19:35 +0100
From: Eric Dumazet <dada1@...mosbay.com>
To: Ron Yorgason <yorgasor@...il.com>
CC: netdev@...r.kernel.org
Subject: Re: Kernel Oops in UDP w/ ARM architecture
Ron Yorgason a écrit :
> On Mon, Mar 9, 2009 at 3:24 PM, Eric Dumazet <dada1@...mosbay.com> wrote:
>> Ron Yorgason a écrit :
>> - Show quoted text -
>>> On Mon, Mar 9, 2009 at 1:21 PM, Eric Dumazet <dada1@...mosbay.com> wrote:
>>>> Please dont top post on this mailing list
>>>>
>>>> Ron Yorgason a écrit :
>>>>> We're using the fec driver, found in drivers/net/fec.c. I modified
>>>>> this driver slightly to get the MAC address from the redboot
>>>>> configuration stored in flash memory, but it's otherwise untouched. I
>>>>> can send my version of the file if that would help.
>>>>>
>>>>> --Ron
>>>>>
>>>>>
>>>> Given that ARM seems to be picky about non aligned accesses, you might
>>>> try this patch. This should force IP header to be aligned.
>>>>
>>>> diff -u linux-2.6.19/drivers/net/fec.c.old linux-2.6.19/drivers/net/fec.c
>>>> --- linux-2.6.19/drivers/net/fec.c.old
>>>> +++ linux-2.6.19/drivers/net/fec.c
>>>> @@ -641,13 +641,14 @@
>>>> * include that when passing upstream as it messes up
>>>> * bridging applications.
>>>> */
>>>> - skb = dev_alloc_skb(pkt_len-4);
>>>> + skb = dev_alloc_skb((pkt_len - 4) + 2);
>>>>
>>>> if (skb == NULL) {
>>>> printk("%s: Memory squeeze, dropping packet.\n", dev->name);
>>>> fep->stats.rx_dropped++;
>>>> } else {
>>>> skb->dev = dev;
>>>> + skb_reserve(skb, 2); /* Align IP on 16 byte boundaries */
>>>> skb_put(skb,pkt_len-4); /* Make room */
>>>> eth_copy_and_sum(skb, data, pkt_len-4, 0);
>>>> skb->protocol=eth_type_trans(skb,dev);
>>>>
>>>>
>>> Thanks for the patch. It looks like Freescale modified this section
>>> of the code in our BSP, so I'm looking at how to best merge things in.
>>> Also, since we're allocating an extra 2 bytes in dev_alloc_skb(), do
>>> we need to account for those bytes in skb_put() and
>>> eth_copy_and_sum(), or does the skb_reserve() call handle that?
>> We allocate 2 extra bytes, because skb_reserve() will advance skb->data pointer
>> by two bytes. Those two bytes plus 14 bytes ethernet header : total 16 bytes,
>> so that IP header is aligned on 16 byte boundaries.
>>
>> No other changes are necessary.
>>
>> Then, maybe this patch is not necessary at all on your platform, since crash happens
>> a *long* time after boot. It may be a shoot in the dark...
>>
>>
>>
>
> The code as supplied by Freescale looks like this:
>
> if ((pkt_len - 4) < fec_copy_threshold) {
> skb = dev_alloc_skb(pkt_len);
> } else {
> skb = dev_alloc_skb(FEC_ENET_RX_FRSIZE);
> }
>
> if (skb == NULL) {
> printk("%s: Memory squeeze, dropping packet.\n", dev->name);
> fep->stats.rx_dropped++;
> } else {
> if ((pkt_len - 4) < fec_copy_threshold) {
> skb_reserve(skb, 2); /*skip 2bytes, so ipheader is align 4bytes*/
> skb_put(skb,pkt_len-4); /* Make room */
> eth_copy_and_sum(skb, (unsigned char *)data,
> pkt_len-4, 0);
> } else {
> struct sk_buff *pskb = fep->rx_skbuff[rx_index];
>
> fep->rx_skbuff[rx_index] = skb;
> skb->data = FEC_ADDR_ALIGNMENT(skb->data);
...
> bdp->cbd_bufaddr = __pa(skb->data);
...
> skb_put(pskb,pkt_len-4); /* Make room */
> skb = pskb;
> }
> skb->dev = dev;
> skb->protocol=eth_type_trans(skb,dev);
> netif_rx(skb);
> }
>
>
> fec_copy_threshold was defined with this comment:
> /*
> * fec_copy_threshold controls the copy when recieving ethernet frame.
> * If ethernet header aligns 4bytes, the ip header and upper
> header will not aligns 4bytes.
> * The resean is ethernet header is 14bytes.
> * And the max size of tcp & ip header is 128bytes. Normally it is 40bytes.
> * So I set the default value between 128 to 256.
> */
> static int fec_copy_threshold = 192;
>
>
> And the FEC_ADDR_ALIGNMENT uses these definitions:
>
> ...
> #elif defined(CONFIG_ARCH_MXC)
> #include <asm/arch/hardware.h>
> #include <asm/arch/iim.h>
> #include "fec.h"
> #define FEC_ALIGNMENT (0x0F) /*FEC needs 128bits(32bytes) alignment*/
> #else
> ...
>
> #define FEC_ADDR_ALIGNMENT(x) ((unsigned char *)(((unsigned long )(x)
> + (FEC_ALIGNMENT)) & (~FEC_ALIGNMENT)))
>
>
> It looks like for a packet smaller than 196 byes will get allocated
> with its actual size, but skb_reserve() skips 2 bytes and now it's a
> little short. For a larger packet, they just allocate 2k, they're not
> skipping 2 bytes and they're aligning the data but not the ip headers.
> However, everything works 99.999% of the time, so I assume there's
> just some border condition that's not being covered correctly.
>
>
This driver seems bugy, and not part of standard kernel tree.
It should not change skb->data without changing skb->tail.
Fortunatly, dev_alloc_skb() already provides an address with a 16bytes alignement
(unless patched by Freescale...)
I suggest you redefine fec_copy_threshold to 1600 to get back a working driver.
My patch is not necessary on this driver (skb_reserve() already called)
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists