Here's my theory on this after an inital look...
Looking at the oops report and disassembly of the actual module binary
that caused the oops, one can deduce that:
Execution was in pppoe_flush_dev(). %ebx contained the pointer "struct
pppox_sock *po", which is what we faulted on, excuting "cmp %eax, 0x190(%ebx)".
%ebx value was 0xffffffff (hence we got "NULL pointer dereference at 0x18f").
At this point "i" (stored in %esi) is 15 (valid), meaning that we got a value
of 0xffffffff in pn->hash_table[i].
>From this I'd hypothesize that the combination of dev_put() and release_sock()
may have allowed us to free "pn". At the bottom of the loop we alreayd
recognize that since locks are dropped we're responsible for handling
invalidation of objects, and perhaps that should be extended to "pn" as well.
--
Michal Ostrowski
mostrows@xxxxxxxxx
---
drivers/net/pppoe.c | 86 ++++++++++++++++++++++++++----
--------------------
1 files changed, 45 insertions(+), 41 deletions(-)
diff --git a/drivers/net/pppoe.c b/drivers/net/pppoe.c
index 7cbf6f9..720c4ea 100644
--- a/drivers/net/pppoe.c
+++ b/drivers/net/pppoe.c
@@ -296,6 +296,7 @@ static void pppoe_flush_dev(struct net_device *dev)
BUG_ON(dev == NULL);
+restart:
pn = pppoe_pernet(dev_net(dev));
if (!pn) /* already freed */
return;
@@ -303,48 +304,51 @@ static void pppoe_flush_dev(struct net_device *dev)
write_lock_bh(&pn->hash_lock);
for (i = 0; i < PPPOE_HASH_SIZE; i++) {
struct pppox_sock *po = pn->hash_table[i];
+ struct sock *sk;
- while (po != NULL) {
- struct sock *sk;
- if (po->pppoe_dev != dev) {
- po = po->next;
- continue;
- }
- sk = sk_pppox(po);
- spin_lock(&flush_lock);
- po->pppoe_dev = NULL;
- spin_unlock(&flush_lock);
- dev_put(dev);
-
- /* We always grab the socket lock, followed by the
- * hash_lock, in that order. Since we should
- * hold the sock lock while doing any unbinding,
- * we need to release the lock we're holding.
- * Hold a reference to the sock so it doesn't disappear
- * as we're jumping between locks.
- */
+ while (po && po->pppoe_dev != dev) {
+ po = po->next;
+ }
- sock_hold(sk);
+ if (po == NULL) {
+ continue;
+ }
- write_unlock_bh(&pn->hash_lock);
- lock_sock(sk);
+ sk = sk_pppox(po);
- if (sk->sk_state & (PPPOX_CONNECTED | PPPOX_BOUND)) {
- pppox_unbind_sock(sk);
- sk->sk_state = PPPOX_ZOMBIE;
- sk->sk_state_change(sk);
- }
+ spin_lock(&flush_lock);
+ po->pppoe_dev = NULL;
+ spin_unlock(&flush_lock);
- release_sock(sk);
- sock_put(sk);
+ dev_put(dev);
- /* Restart scan at the beginning of this hash chain.
- * While the lock was dropped the chain contents may
- * have changed.
- */
- write_lock_bh(&pn->hash_lock);
- po = pn->hash_table[i];
- }
+ /* We always grab the socket lock, followed by the
+ * hash_lock, in that order. Since we should
+ * hold the sock lock while doing any unbinding,
+ * we need to release the lock we're holding.
+ * Hold a reference to the sock so it doesn't disappear
+ * as we're jumping between locks.
+ */
+
+ sock_hold(sk);
+
+ write_unlock_bh(&pn->hash_lock);
+ lock_sock(sk);
+
+ if (sk->sk_state & (PPPOX_CONNECTED | PPPOX_BOUND)) {
+ pppox_unbind_sock(sk);
+ sk->sk_state = PPPOX_ZOMBIE;
+ sk->sk_state_change(sk);
+ }
+
+ release_sock(sk);
+ sock_put(sk);
+
+ /* Restart the flush process from the beginning. While
+ * the lock was dropped the chain contents may have
+ * changed, and sock_put may have made things go away.
+ */
+ goto restart;
}
write_unlock_bh(&pn->hash_lock);
}
--
1.6.3.3
On Sun, Oct 18, 2009 at 4:02 PM, Denys Fedoryschenko <denys@xxxxxxxxxxx> wrote:
>
> I have server running as pppoe NAS.
> Tried to rename customers without dropping pppd connections first, got panic
> after few seconds.
> Panic triggerable at 2.6.30.4 and 2.6.31.4
> pppoe users running on eth2
> pppoe flags:
> 1457 root /usr/sbin/pppoe-server -I eth2 -k -L 172.16.1.1 -R
> 172.16.1.2 -N 253 -C gpzone -S gpzone
>
>
> Commands sequence that i think triggered that:
>
> ip link set eth0 down
> ip link set eth1 down
> ip link set eth2 down
> nameif etherx 00:16:76:8D:83:BA
> nameif eth0 00:19:e0:72:4a:37
> nameif eth1 00:19:e0:72:4a:4b
>
> ip addr flush dev eth0
> ip addr flush dev eth1
> ip addr add X.X.X.X/29 dev eth0
> ip addr add 192.168.2.177/24 dev eth0
> ip addr add 192.168.0.1/32 dev eth1
> ip addr add 127.0.0.0/8 dev lo
> #ip link set eth0 up
> ip link set eth0 up
> ip link set eth1 up
> ip link set lo up
> ip route add 0.0.0.0/0 via X.X.X.X
>
>
> [ 103.428591] r8169: eth0: link up
> [ 103.430360] r8169: eth1: link up
> [ 113.361528] BUG: unable to handle kernel
> NULL pointer dereference
> at 0000018f
> [ 113.361717] IP:
> [<f8868269>] pppoe_device_event+0x80/0x12c [pppoe]
> [ 113.361853] *pdpt = 000000003411a001
> *pde = 0000000000000000
> Oct 18 23:59:40 194.146.153.93
> [ 113.362012] Oops: 0000 [#1]
> SMP
> Oct 18 23:59:40 194.146.153.93
> [ 113.362166] last sysfs file: /sys/devices/virtual/vc/vcs3/dev
> [ 113.362246] Modules linked in:
> netconsole
> configfs
> act_skbedit
> sch_ingress
> sch_prio
> cls_flow
> cls_u32
> em_meta
> cls_basic
> xt_dscp
> xt_DSCP
> ipt_REJECT
> ts_bm
> xt_string
> xt_hl
> ifb
> cls_fw
> sch_tbf
> sch_htb
> act_ipt
> act_mirred
> xt_MARK
> pppoe
> pppox
> ppp_generic
> slhc
> xt_TCPMSS
> xt_mark
> xt_tcpudp
> iptable_mangle
> iptable_nat
> nf_nat
> rtc_cmos
> nf_conntrack_ipv4
> rtc_core
> nf_conntrack
> rtc_lib
> nf_defrag_ipv4
> iptable_filter
> ip_tables
> x_tables
> 8021q
> garp
> stp
> llc
> loop
> sata_sil
> pata_atiixp
> pata_acpi
> ata_generic
> libata
> 8139cp
> usb_storage
> mtdblock
> mtd_blkdevs
> mtd
> sr_mod
> cdrom
> tulip
> r8169
> sky2
> via_velocity
> via_rhine
> sis900
> ne2k_pci
> 8390
> skge
> tg3
> libphy
> 8139too
> e1000
> e100
> usbhid
> ohci_hcd
> uhci_hcd
> ehci_hcd
> usbcore
> nls_base
> Oct 18 23:59:40 194.146.153.93
> [ 113.362344]
> [ 113.362344] Pid: 2858, comm: pppd Not tainted (2.6.31.4-build-0047 #7)
> [ 113.362344] EIP: 0060:[<f8868269>] EFLAGS: 00010286 CPU: 0
> [ 113.362344] EIP is at pppoe_device_event+0x80/0x12c [pppoe]
> [ 113.362344] EAX: f4fbe000 EBX: ffffffff ECX: f6cea5a0 EDX: f7403680
> [ 113.362344] ESI: 0000000f EDI: f6cea5e0 EBP: f4145e34 ESP: f4145e1c
> [ 113.362344] DS: 007b ES: 007b FS: 00d8 GS: 0033 SS: 0068
> [ 113.362344] Process pppd (pid: 2858, ti=f4145000 task=f4112ff0
> task.ti=f4145000)
> [ 113.362344] Stack:
> [ 113.362344] f4fbe220
> f4fbe000
> f6cea5a0
> f886a430
> fffffff5
> 00000000
> f4145e54
> c01422b3
> Oct 18 23:59:40 194.146.153.93
> [ 113.362344] <0>
> f4fbe000
> 00000009
> f8a457d8
> f4fbe000
> f8850190
> 00001091
> f4145e64
> c0142361
> Oct 18 23:59:40 194.146.153.93
> [ 113.362344] <0>
> ffffffff
> 00000000
> f4145e74
> c029ffbf
> f4fbe000
> 000010d0
> f4145e90
> c029fa70
> Oct 18 23:59:40 194.146.153.93
> [ 113.362344] Call Trace:
> [ 113.362344] [<c01422b3>] ? notifier_call_chain+0x2b/0x4a
> [ 113.362344] [<c0142361>] ? raw_notifier_call_chain+0xc/0xe
> [ 113.362344] [<c029ffbf>] ? dev_close+0x4c/0x8c
> [ 113.362344] [<c029fa70>] ? dev_change_flags+0xa5/0x158
> [ 113.362344] [<c02da633>] ? devinet_ioctl+0x21a/0x503
> [ 113.362344] [<c02db693>] ? inet_ioctl+0x8d/0xa6
> [ 113.362344] [<c0292b21>] ? sock_ioctl+0x1c8/0x1ec
> [ 113.362344] [<c0292959>] ? sock_ioctl+0x0/0x1ec
> [ 113.362344] [<c019af2b>] ? vfs_ioctl+0x22/0x69
> [ 113.362344] [<c019b435>] ? do_vfs_ioctl+0x41f/0x459
> [ 113.362344] [<c02934eb>] ? sys_send+0x18/0x1a
> [ 113.362344] [<c011942f>] ? do_page_fault+0x242/0x26f
> [ 113.362344] [<c019b49b>] ? sys_ioctl+0x2c/0x45
> [ 113.362344] [<c0102975>] ? syscall_call+0x7/0xb
> [ 113.362344] Code:
> c9
> 00
> 00
> 00
> 89
> c7
> 31
> f6
> 83
> c7
> 40
> 89
> f8
> e8
> cc
> 60
> a9
> c7
> 8b
> 45
> ec
> 05
> 20
> 02
> 00
> 00
> 89
> 45
> e8
> 8b
> 4d
> f0
> 8b
> 1c
> b1
> e9
> 8c
> 00
> 00
> 00
> 8b
> 45
> ec
> Oct 18 23:59:40 194.146.153.93
> 83
> 90
> 01
> 00
> 00
> 74
> 08
> 8b
> 9b
> 8c
> 01
> 00
> 00
> eb
> 79
> b8
> c0
> a6
> 86
> f8
> Oct 18 23:59:40 194.146.153.93
> [ 113.362344] EIP: [<f8868269>]
> pppoe_device_event+0x80/0x12c [pppoe]
> SS:ESP 0068:f4145e1c
> [ 113.362344] CR2: 000000000000018f
> [ 113.373124] ---[ end trace f6fe64a307e97f3b ]---
> [ 113.373203] Kernel panic - not syncing: Fatal exception in interrupt
> [ 113.373286] Pid: 2858, comm: pppd Tainted: G D 2.6.31.4-build-0047
> #7
> [ 113.373379] Call Trace:
> [ 113.373479] [<c02fc496>] ? printk+0xf/0x11
> [ 113.373561] [<c02fc3e7>] panic+0x39/0xd9
> [ 113.373656] [<c01059b7>] oops_end+0x8b/0x9a
> [ 113.373727] [<c0118f6d>] no_context+0x13d/0x147
> [ 113.373800] [<c011908a>] __bad_area_nosemaphore+0x113/0x11b
> [ 113.373881] [<c02953b3>] ? sock_alloc_send_pskb+0x8b/0x24a
> [ 113.373959] [<c0121801>] ? __wake_up_sync_key+0x3b/0x45
> [ 113.374030] [<c0131967>] ? irq_exit+0x39/0x5c
> [ 113.374107] [<c0104393>] ? do_IRQ+0x80/0x96
> [ 113.374183] [<c0102f49>] ? common_interrupt+0x29/0x30
> [ 113.374259] [<c011909f>] bad_area_nosemaphore+0xd/0x10
> [ 113.374348] [<c0119301>] do_page_fault+0x114/0x26f
> [ 113.374526] [<c01191ed>] ? do_page_fault+0x0/0x26f
> [ 113.374605] [<c02fe506>] error_code+0x66/0x6c
> [ 113.374683] [<c02d007b>] ? tcp_v4_send_ack+0xa3/0x10e
> [ 113.374764] [<c01191ed>] ? do_page_fault+0x0/0x26f
> [ 113.374850] [<f8868269>] ? pppoe_device_event+0x80/0x12c [pppoe]
> [ 113.374928] [<c01422b3>] notifier_call_chain+0x2b/0x4a
> [ 113.375012] [<c0142361>] raw_notifier_call_chain+0xc/0xe
> [ 113.375097] [<c029ffbf>] dev_close+0x4c/0x8c
> [ 113.375169] [<c029fa70>] dev_change_flags+0xa5/0x158
> [ 113.375239] [<c02da633>] devinet_ioctl+0x21a/0x503
> [ 113.375318] [<c02db693>] inet_ioctl+0x8d/0xa6
> [ 113.375411] [<c0292b21>] sock_ioctl+0x1c8/0x1ec
> [ 113.375491] [<c0292959>] ? sock_ioctl+0x0/0x1ec
> [ 113.375574] [<c019af2b>] vfs_ioctl+0x22/0x69
> [ 113.375653] [<c019b435>] do_vfs_ioctl+0x41f/0x459
> [ 113.375734] [<c02934eb>] ? sys_send+0x18/0x1a
> [ 113.375813] [<c011942f>] ? do_page_fault+0x242/0x26f
> [ 113.375884] [<c019b49b>] sys_ioctl+0x2c/0x45
> [ 113.375960] [<c0102975>] syscall_call+0x7/0xb
> [ 113.376041] Rebooting in 5 seconds..
--
To unsubscribe from this list: send the line "unsubscribe linux-ppp" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
