Skip to contentSkip to conversation

pasemi_mac: Fix reuse of free'd skb

PATCH In review Olof Johansson 3 participants 4 messages Dec 4, 2007

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 pasemi_mac: Fix reuse of free'd skb

Turns out we're freeing the skb when we detect CRC error, but we're not clearing out info->skb. We could either clear it and have the stack reallocate it, or just leave it and the rx ring refill code will reuse the one that was allocated.

Reusing a freed skb obviously caused some nasty crashes of various kind, as reported by Brent Baude and David Woodhouse.

---

Jeff, I'd like to see this in 2.6.24, it's causing some real problems out there. It's not needed in the 2.6.25 queue since the other changes there have already covered these cases.

My test network at home is quiet enough to not cause CRC errors, we mainly get those during interface bringup before speed is configured.

drivers/net/pasemi_mac.c +1 -1
index 09b4fde..6617e24 100644
@@ -586,7 +586,7 @@ static int pasemi_mac_clean_rx(struct pasemi_mac *mac, int limit)
 			/* CRC error flagged */
 			mac->netdev->stats.rx_errors++;
 			mac->netdev->stats.rx_crc_errors++;
-			dev_kfree_skb_irq(skb);
+			/* No need to free skb, it'll be reused */
 			goto next;
 		}
 
#1 lore raw
Oops: Exception in kernel mode, sig: 5 [#1]                                     
nfs: server pmac not responding, still trying                                   
SMP NR_CPUS=128 NUMA PA Semi PWRficient                                         
Modules linked in: appletouch cbc blkcipher dm_crypt dm_emc dm_round_robin dm_mu

ltipath dm_snapshot dm_mirror dm_zero dm_mod xfs jfs reiserfs lock_nolock gfs2 m sdos linear raid10 raid456 async_memcpy async_tx async_xor xor raid1 raid0 sata_ mv libata pasemi_mac gpio_mdio libphy ehci_hcd ohci_hcd iscsi_tcp libiscsi scsi_ transport_iscsi sr_mod sd_mod scsi_mod ide_cd cdrom ipv6 ext2 ext4dev crc16 jbd2
ext3 mbcache jbd squashfs loop nfs nfs_acl lockd sunrpc vfat fat cramfs
NIP: c0000000003af2f0 LR: c0000000003af2ec CTR: c0000000000791c0
REGS: c00000000ffff9c0 TRAP: 0700 Not tainted (2.6.24-0.67.rc3.git7.fc9)
MSR: 9000000000029032 <EE,ME,IR,DR> CR: 24000028 XER: 00000000 TASK = c000000000677720[0] 'swapper' THREAD: c000000000758000 CPU: 0
GPR00: c0000000003af2ec c00000000ffffc40 c000000000753d98 0000000000000087
GPR04: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
GPR08: c000000000677720 0000000000000000 0000000000000004 0000000000000000
GPR12: 0000000000000000 c000000000678200 0000000000000000 0000000000000000
GPR16: 0000000000000000 0000000000000000 0000000000000000 4000000001400000
GPR20: 0000000000000040 00000000000006e0 00000000000006e8 0000000000000037
GPR24: 0000000000000000 c00000003ba40370 00000000000000dc 540045ee0111a3a8
GPR28: 00000000000005ee c00000003d104900 c000000000712b18 c00000003b939800 NIP [c0000000003af2f0] .skb_over_panic+0x50/0x58
LR [c0000000003af2ec] .skb_over_panic+0x4c/0x58

Call Trace:                                                                     
[c00000000ffffc40] [c0000000003af2ec] .skb_over_panic+0x4c/0x58 (unreliable)    
[c00000000ffffcd0] [d0000000002dfb10] .pasemi_mac_clean_rx+0x2f0/0x480 [pasemi_m

ac]
[c00000000ffffda0] [d0000000002e008c] .pasemi_mac_poll+0x44/0xe4 [pasemi_mac] [c00000000ffffe40] [c0000000003bba54] .net_rx_action+0xf8/0x214 [c00000000ffffef0] [c00000000008eb58] .__do_softirq+0xa8/0x164 [c00000000fffff90] [c00000000002b468] .call_do_softirq+0x14/0x24 [c00000000075b940] [c00000000000d4d0] .do_softirq+0x68/0xac [c00000000075b9d0] [c00000000008ecc8] .irq_exit+0x60/0xb0 [c00000000075ba50] [c00000000000db00] .do_IRQ+0x174/0x1e8 [c00000000075baf0] [c000000000004c24] hardware_interrupt_entry+0x24/0x28 --- Exception: 501 at .ppc64_runlatch_off+0x0/0x54

    LR = .cpu_idle+0x70/0x1f0                                                   

[c00000000075bde0] [c0000000000135c4] .cpu_idle+0x11c/0x1f0 (unreliable) [c00000000075be60] [c000000000009b08] .rest_init+0x78/0x90 [c00000000075bee0] [c0000000005c59ec] .start_kernel+0x3f8/0x41c [c00000000075bf90] [c000000000008590] .start_here_common+0x60/0xd0 Instruction dump:
80a30068 e8e300c8 e90300d0 812300bc 814300c0 2fa00000 409e0008 e81e8018 e87e8028 f8010070 4bcd92cd 60000000 <0fe00000> 48000000 7c0802a6 faa1ffa8

Kernel panic - not syncing: Fatal exception in interrupt                        

Rebooting in 180 seconds..

Signature · 1 line
dwmw2
#2 lore raw
Quoted 2 lines · David Woodhouse
Oops: Exception in kernel mode, sig: 5 [#1]                                     

nfs: server pmac not responding, still trying

Oops, sorry. That was meant to be just to Olof. And it helps if I'm actually running the kernel in which his patch is applied, too...

Signature · 1 line
dwmw2
#3 lore raw
In reply to Olof Johansson
Quoted 25 lines · Olof Johansson

Turns out we're freeing the skb when we detect CRC error, but we're not clearing out info->skb. We could either clear it and have the stack reallocate it, or just leave it and the rx ring refill code will reuse the one that was allocated.

Reusing a freed skb obviously caused some nasty crashes of various kind, as reported by Brent Baude and David Woodhouse.

---

Jeff, I'd like to see this in 2.6.24, it's causing some real problems out there. It's not needed in the 2.6.25 queue since the other changes there have already covered these cases.

My test network at home is quiet enough to not cause CRC errors, we mainly get those during interface bringup before speed is configured.

drivers/net/pasemi_mac.c +0 -0
index 09b4fde..6617e24 100644

drivers/net/pasemi_mac.c static int pasemi_mac_clean_rx(struct pasemi_mac *mac, int limit) · line 590

Quoted 6 lines
drivers/net/pasemi_mac.c +1 -1
@@ -586,7 +586,7 @@ static int pasemi_mac_clean_rx(struct pasemi_mac *mac, int limit)
 			/* CRC error flagged */
 			mac->netdev->stats.rx_errors++;
 			mac->netdev->stats.rx_crc_errors++;
-			dev_kfree_skb_irq(skb);
+			/* No need to free skb, it'll be reused */
 			goto next;

applied #upstream-fixes