netdev - Re: Kernel Oops in UDP w/ ARM architecture

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
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <93d1fdd10903091357i51d2a883r45efeb662a6859eb@mail.gmail.com>
Date:	Mon, 9 Mar 2009 15:57:37 -0500
From:	Ron Yorgason <yorgasor@...il.com>
To:	Eric Dumazet <dada1@...mosbay.com>
Cc:	netdev@...r.kernel.org
Subject: Re: Kernel Oops in UDP w/ ARM architecture

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.
--
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