On Mon, Aug 22, 2016 at 09:20:14AM +0800, fgao@xxxxxxxxxx wrote: > From: Gao Feng <fgao@xxxxxxxxxx> > > PPP channel holds one spinlock before send frame. But the skb may > select the same PPP channel with wrong route policy. As a result, > the skb reaches the same channel path. It tries to get the same > spinlock which is held before. Bang, the deadlock comes out. > Thanks for following up on this case. On my side, I've thought a bit more about it in the weekend and cooked this patch. It's experimental and requires cleanup and further testing, but it should fix all issues I could think of (at least for PPP over L2TP). The main idea is to use a per-cpu variable to detect recursion in selected points of PPP and L2TP xmit path. --- drivers/net/ppp/ppp_generic.c | 49 ++++++++++++++++++++++++++++++++----------- net/l2tp/l2tp_core.c | 28 +++++++++++++++++++++---- 2 files changed, 61 insertions(+), 16 deletions(-) diff --git a/drivers/net/ppp/ppp_generic.c b/drivers/net/ppp/ppp_generic.c index f226db4..c33036bf 100644 --- a/drivers/net/ppp/ppp_generic.c +++ b/drivers/net/ppp/ppp_generic.c @@ -1354,6 +1354,8 @@ static void ppp_setup(struct net_device *dev) dev->netdev_ops = &ppp_netdev_ops; SET_NETDEV_DEVTYPE(dev, &ppp_type); + dev->features |= NETIF_F_LLTX; + dev->hard_header_len = PPP_HDRLEN; dev->mtu = PPP_MRU; dev->addr_len = 0; @@ -1367,12 +1369,7 @@ static void ppp_setup(struct net_device *dev) * Transmit-side routines. */ -/* - * Called to do any work queued up on the transmit side - * that can now be done. - */ -static void -ppp_xmit_process(struct ppp *ppp) +static void __ppp_xmit_process(struct ppp *ppp) { struct sk_buff *skb; @@ -1392,6 +1389,23 @@ ppp_xmit_process(struct ppp *ppp) ppp_xmit_unlock(ppp); } +static DEFINE_PER_CPU(int, ppp_xmit_recursion); + +/* Called to do any work queued up on the transmit side that can now be done */ +static void ppp_xmit_process(struct ppp *ppp) +{ + if (unlikely(__this_cpu_read(ppp_xmit_recursion))) { + WARN(1, "recursion detected\n"); + return; + } + + __this_cpu_inc(ppp_xmit_recursion); + local_bh_disable(); + __ppp_xmit_process(ppp); + local_bh_enable(); + __this_cpu_dec(ppp_xmit_recursion); +} + static inline struct sk_buff * pad_compress_skb(struct ppp *ppp, struct sk_buff *skb) { @@ -1847,11 +1861,7 @@ static int ppp_mp_explode(struct ppp *ppp, struct sk_buff *skb) } #endif /* CONFIG_PPP_MULTILINK */ -/* - * Try to send data out on a channel. - */ -static void -ppp_channel_push(struct channel *pch) +static void __ppp_channel_push(struct channel *pch) { struct sk_buff *skb; struct ppp *ppp; @@ -1876,11 +1886,26 @@ ppp_channel_push(struct channel *pch) read_lock_bh(&pch->upl); ppp = pch->ppp; if (ppp) - ppp_xmit_process(ppp); + __ppp_xmit_process(ppp); read_unlock_bh(&pch->upl); } } +/* Try to send data out on a channel */ +static void ppp_channel_push(struct channel *pch) +{ + if (unlikely(__this_cpu_read(ppp_xmit_recursion))) { + WARN(1, "recursion detected\n"); + return; + } + + __this_cpu_inc(ppp_xmit_recursion); + local_bh_disable(); + __ppp_channel_push(pch); + local_bh_enable(); + __this_cpu_dec(ppp_xmit_recursion); +} + /* * Receive-side routines. */ diff --git a/net/l2tp/l2tp_core.c b/net/l2tp/l2tp_core.c index 1e40dac..bdfb1be 100644 --- a/net/l2tp/l2tp_core.c +++ b/net/l2tp/l2tp_core.c @@ -1096,10 +1096,8 @@ static int l2tp_xmit_core(struct l2tp_session *session, struct sk_buff *skb, return 0; } -/* If caller requires the skb to have a ppp header, the header must be - * inserted in the skb data before calling this function. - */ -int l2tp_xmit_skb(struct l2tp_session *session, struct sk_buff *skb, int hdr_len) +static int __l2tp_xmit_skb(struct l2tp_session *session, struct sk_buff *skb, + int hdr_len) { int data_len = skb->len; struct l2tp_tunnel *tunnel = session->tunnel; @@ -1178,6 +1176,28 @@ out_unlock: return ret; } + +static DEFINE_PER_CPU(int, l2tp_xmit_recursion); + +/* If caller requires the skb to have a ppp header, the header must be + * inserted in the skb data before calling this function. + */ +int l2tp_xmit_skb(struct l2tp_session *session, struct sk_buff *skb, + int hdr_len) +{ + int ret; + + if (unlikely(__this_cpu_read(l2tp_xmit_recursion))) { + WARN(1, "recursion detected\n"); + return NET_XMIT_DROP; + } + + __this_cpu_inc(l2tp_xmit_recursion); + ret = __l2tp_xmit_skb(session, skb, hdr_len); + __this_cpu_dec(l2tp_xmit_recursion); + + return ret; +} EXPORT_SYMBOL_GPL(l2tp_xmit_skb); /***************************************************************************** -- 2.9.3 -- 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