Merge branch 'ppp-recursion' (127661a4) · Commits · 戴 / test

Guillaume Nault says: ==================== ppp: fix deadlock upon recursive xmit This series fixes the issue reported by Feng where packets looping through a ppp device makes the module deadlock: https://marc.info/?l=linux-netdev&m=147134567319038&w=2 The problem can occur on virtual interfaces (e.g. PPP over L2TP, or PPPoE on vxlan devices), when a PPP packet is routed back to the PPP interface. PPP's xmit path isn't reentrant, so patch #1 uses a per-cpu variable to detect and break recursion. Patch #2 sets the NETIF_F_LLTX flag to avoid lock inversion issues between ppp and txqueue locks. There are multiple entry points to the PPP xmit path. This series has been tested with lockdep and should address recursion issues no matter how the packet entered the path. A similar issue in L2TP is not covered by this series: l2tp_xmit_skb() also isn't reentrant, and it can be called as part of PPP's xmit path (pppol2tp_xmit()), or directly from the L2TP socket (l2tp_ppp_sendmsg()). If a packet is sent by l2tp_ppp_sendmsg() and routed to the parent PPP interface, then it's going to hit l2tp_xmit_skb() again. Breaking recursion as done in ppp_generic is not enough, because we'd still have a lock inversion issue (locking in l2tp_xmit_skb() can happen before or after locking in ppp_generic). The best approach would be to use the ip_tunnel functions and remove the socket locking in l2tp_xmit_skb(). But that'd be something for net-next. ==================== Signed-off-by:

David S. Miller <davem@davemloft.net>

drivers/net/ppp/ppp_generic.c

+42 −12

Original line number	Diff line number	Diff line
		@@ -1363,6 +1363,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;
		@@ -1376,12 +1378,8 @@ 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)
		/* Called to do any work queued up on the transmit side that can now be done */
		static void __ppp_xmit_process(struct ppp *ppp)
		{
		struct sk_buff *skb;

		@@ -1401,6 +1399,30 @@ ppp_xmit_process(struct ppp *ppp)
		ppp_xmit_unlock(ppp);
		}

		static DEFINE_PER_CPU(int, ppp_xmit_recursion);

		static void ppp_xmit_process(struct ppp *ppp)
		{
		local_bh_disable();

		if (unlikely(__this_cpu_read(ppp_xmit_recursion)))
		goto err;

		__this_cpu_inc(ppp_xmit_recursion);
		__ppp_xmit_process(ppp);
		__this_cpu_dec(ppp_xmit_recursion);

		local_bh_enable();

		return;

		err:
		local_bh_enable();

		if (net_ratelimit())
		netdev_err(ppp->dev, "recursion detected\n");
		}

		static inline struct sk_buff *
		pad_compress_skb(struct ppp ppp, struct sk_buff skb)
		{
		@@ -1856,11 +1878,8 @@ 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)
		/* Try to send data out on a channel */
		static void __ppp_channel_push(struct channel *pch)
		{
		struct sk_buff *skb;
		struct ppp *ppp;
		@@ -1885,11 +1904,22 @@ 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);
		}
		}

		static void ppp_channel_push(struct channel *pch)
		{
		local_bh_disable();

		__this_cpu_inc(ppp_xmit_recursion);
		__ppp_channel_push(pch);
		__this_cpu_dec(ppp_xmit_recursion);

		local_bh_enable();
		}

		/*
		* Receive-side routines.
		*/

Admin message