Skip to content

Commit 55454a5

Browse files
Guillaume Naultdavem330
Guillaume Nault
authored and
davem330
committed
ppp: avoid dealock on recursive xmit
In case of misconfiguration, a virtual PPP channel might send packets back to their parent PPP interface. This typically happens in misconfigured L2TP setups, where PPP's peer IP address is set with the IP of the L2TP peer. When that happens the system hangs due to PPP trying to recursively lock its xmit path. [ 243.332155] BUG: spinlock recursion on CPU#1, accel-pppd/926 [ 243.333272] lock: 0xffff880033d90f18, .magic: dead4ead, .owner: accel-pppd/926, .owner_cpu: 1 [ 243.334859] CPU: 1 PID: 926 Comm: accel-pppd Not tainted 4.8.0-rc2 #1 [ 243.336010] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Debian-1.8.2-1 04/01/2014 [ 243.336018] ffff7fffffffffff ffff8800319a77a0 ffffffff8128de85 ffff880033d90f18 [ 243.336018] ffff880033ad8000 ffff8800319a77d8 ffffffff810ad7c0 ffffffff0000039e [ 243.336018] ffff880033d90f18 ffff880033d90f60 ffff880033d90f18 ffff880033d90f28 [ 243.336018] Call Trace: [ 243.336018] [<ffffffff8128de85>] dump_stack+0x4f/0x65 [ 243.336018] [<ffffffff810ad7c0>] spin_dump+0xe1/0xeb [ 243.336018] [<ffffffff810ad7f0>] spin_bug+0x26/0x28 [ 243.336018] [<ffffffff810ad8b9>] do_raw_spin_lock+0x5c/0x160 [ 243.336018] [<ffffffff815522aa>] _raw_spin_lock_bh+0x35/0x3c [ 243.336018] [<ffffffffa01a88e2>] ? ppp_push+0xa7/0x82d [ppp_generic] [ 243.336018] [<ffffffffa01a88e2>] ppp_push+0xa7/0x82d [ppp_generic] [ 243.336018] [<ffffffff810adada>] ? do_raw_spin_unlock+0xc2/0xcc [ 243.336018] [<ffffffff81084962>] ? preempt_count_sub+0x13/0xc7 [ 243.336018] [<ffffffff81552438>] ? _raw_spin_unlock_irqrestore+0x34/0x49 [ 243.336018] [<ffffffffa01ac657>] ppp_xmit_process+0x48/0x877 [ppp_generic] [ 243.336018] [<ffffffff81084962>] ? preempt_count_sub+0x13/0xc7 [ 243.336018] [<ffffffff81408cd3>] ? skb_queue_tail+0x71/0x7c [ 243.336018] [<ffffffffa01ad1c5>] ppp_start_xmit+0x21b/0x22a [ppp_generic] [ 243.336018] [<ffffffff81426af1>] dev_hard_start_xmit+0x15e/0x32c [ 243.336018] [<ffffffff81454ed7>] sch_direct_xmit+0xd6/0x221 [ 243.336018] [<ffffffff814273a8>] __dev_queue_xmit+0x52a/0x820 [ 243.336018] [<ffffffff814276a9>] dev_queue_xmit+0xb/0xd [ 243.336018] [<ffffffff81430a3c>] neigh_direct_output+0xc/0xe [ 243.336018] [<ffffffff8146b5d7>] ip_finish_output2+0x4d2/0x548 [ 243.336018] [<ffffffff8146a8e6>] ? dst_mtu+0x29/0x2e [ 243.336018] [<ffffffff8146d49c>] ip_finish_output+0x152/0x15e [ 243.336018] [<ffffffff8146df84>] ? ip_output+0x74/0x96 [ 243.336018] [<ffffffff8146df9c>] ip_output+0x8c/0x96 [ 243.336018] [<ffffffff8146d55e>] ip_local_out+0x41/0x4a [ 243.336018] [<ffffffff8146dd15>] ip_queue_xmit+0x531/0x5c5 [ 243.336018] [<ffffffff814a82cd>] ? udp_set_csum+0x207/0x21e [ 243.336018] [<ffffffffa01f2f04>] l2tp_xmit_skb+0x582/0x5d7 [l2tp_core] [ 243.336018] [<ffffffffa01ea458>] pppol2tp_xmit+0x1eb/0x257 [l2tp_ppp] [ 243.336018] [<ffffffffa01acf17>] ppp_channel_push+0x91/0x102 [ppp_generic] [ 243.336018] [<ffffffffa01ad2d8>] ppp_write+0x104/0x11c [ppp_generic] [ 243.336018] [<ffffffff811a3c1e>] __vfs_write+0x56/0x120 [ 243.336018] [<ffffffff81239801>] ? fsnotify_perm+0x27/0x95 [ 243.336018] [<ffffffff8123ab01>] ? security_file_permission+0x4d/0x54 [ 243.336018] [<ffffffff811a4ca4>] vfs_write+0xbd/0x11b [ 243.336018] [<ffffffff811a5a0a>] SyS_write+0x5e/0x96 [ 243.336018] [<ffffffff81552a1b>] entry_SYSCALL_64_fastpath+0x13/0x94 The main entry points for sending packets over a PPP unit are the .write() and .ndo_start_xmit() callbacks (simplified view): .write(unit fd) or .ndo_start_xmit() \ CALL ppp_xmit_process() \ LOCK unit's xmit path (ppp->wlock) | CALL ppp_push() \ LOCK channel's xmit path (chan->downl) | CALL lower layer's .start_xmit() callback \ ... might recursively call .ndo_start_xmit() ... / RETURN from .start_xmit() | UNLOCK channel's xmit path / RETURN from ppp_push() | UNLOCK unit's xmit path / RETURN from ppp_xmit_process() Packets can also be directly sent on channels (e.g. LCP packets): .write(channel fd) or ppp_output_wakeup() \ CALL ppp_channel_push() \ LOCK channel's xmit path (chan->downl) | CALL lower layer's .start_xmit() callback \ ... might call .ndo_start_xmit() ... / RETURN from .start_xmit() | UNLOCK channel's xmit path / RETURN from ppp_channel_push() Key points about the lower layer's .start_xmit() callback: * It can be called directly by a channel fd .write() or by ppp_output_wakeup() or indirectly by a unit fd .write() or by .ndo_start_xmit(). * In any case, it's always called with chan->downl held. * It might route the packet back to its parent unit using .ndo_start_xmit() as entry point. This patch detects and breaks recursion in ppp_xmit_process(). This function is a good candidate for the task because it's called early enough after .ndo_start_xmit(), it's always part of the recursion loop and it's on the path of whatever entry point is used to send a packet on a PPP unit. Recursion detection is done using the per-cpu ppp_xmit_recursion variable. Since ppp_channel_push() too locks the channel's xmit path and calls the lower layer's .start_xmit() callback, we need to also increment ppp_xmit_recursion there. However there's no need to check for recursion, as it's out of the recursion loop. Reported-by: Feng Gao <gfree.wind@gmail.com> Signed-off-by: Guillaume Nault <g.nault@alphalink.fr> Signed-off-by: David S. Miller <davem@davemloft.net>
1 parent ce0b15d commit 55454a5

File tree

1 file changed

+40
-12
lines changed

1 file changed

+40
-12
lines changed

‎drivers/net/ppp/ppp_generic.c

Lines changed: 40 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1376,12 +1376,8 @@ static void ppp_setup(struct net_device *dev)
13761376
* Transmit-side routines.
13771377
*/
13781378

1379-
/*
1380-
* Called to do any work queued up on the transmit side
1381-
* that can now be done.
1382-
*/
1383-
static void
1384-
ppp_xmit_process(struct ppp *ppp)
1379+
/* Called to do any work queued up on the transmit side that can now be done */
1380+
static void __ppp_xmit_process(struct ppp *ppp)
13851381
{
13861382
struct sk_buff *skb;
13871383

@@ -1401,6 +1397,30 @@ ppp_xmit_process(struct ppp *ppp)
14011397
ppp_xmit_unlock(ppp);
14021398
}
14031399

1400+
static DEFINE_PER_CPU(int, ppp_xmit_recursion);
1401+
1402+
static void ppp_xmit_process(struct ppp *ppp)
1403+
{
1404+
local_bh_disable();
1405+
1406+
if (unlikely(__this_cpu_read(ppp_xmit_recursion)))
1407+
goto err;
1408+
1409+
__this_cpu_inc(ppp_xmit_recursion);
1410+
__ppp_xmit_process(ppp);
1411+
__this_cpu_dec(ppp_xmit_recursion);
1412+
1413+
local_bh_enable();
1414+
1415+
return;
1416+
1417+
err:
1418+
local_bh_enable();
1419+
1420+
if (net_ratelimit())
1421+
netdev_err(ppp->dev, "recursion detected\n");
1422+
}
1423+
14041424
static inline struct sk_buff *
14051425
pad_compress_skb(struct ppp *ppp, struct sk_buff *skb)
14061426
{
@@ -1856,11 +1876,8 @@ static int ppp_mp_explode(struct ppp *ppp, struct sk_buff *skb)
18561876
}
18571877
#endif /* CONFIG_PPP_MULTILINK */
18581878

1859-
/*
1860-
* Try to send data out on a channel.
1861-
*/
1862-
static void
1863-
ppp_channel_push(struct channel *pch)
1879+
/* Try to send data out on a channel */
1880+
static void __ppp_channel_push(struct channel *pch)
18641881
{
18651882
struct sk_buff *skb;
18661883
struct ppp *ppp;
@@ -1885,11 +1902,22 @@ ppp_channel_push(struct channel *pch)
18851902
read_lock_bh(&pch->upl);
18861903
ppp = pch->ppp;
18871904
if (ppp)
1888-
ppp_xmit_process(ppp);
1905+
__ppp_xmit_process(ppp);
18891906
read_unlock_bh(&pch->upl);
18901907
}
18911908
}
18921909

1910+
static void ppp_channel_push(struct channel *pch)
1911+
{
1912+
local_bh_disable();
1913+
1914+
__this_cpu_inc(ppp_xmit_recursion);
1915+
__ppp_channel_push(pch);
1916+
__this_cpu_dec(ppp_xmit_recursion);
1917+
1918+
local_bh_enable();
1919+
}
1920+
18931921
/*
18941922
* Receive-side routines.
18951923
*/

0 commit comments

Comments
 (0)