All of lore.kernel.org
 help / color / mirror / Atom feed
From: Qingfang Deng <qingfang.deng@linux.dev>
To: linux-ppp@vger.kernel.org, Andrew Lunn <andrew+netdev@lunn.ch>,
	"David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	Qingfang Deng <qingfang.deng@linux.dev>,
	Kees Cook <kees@kernel.org>, Eric Woudstra <ericwouds@gmail.com>,
	Asim Viladi Oglu Manizada <manizada@pm.me>,
	Felix Fietkau <nbd@nbd.name>,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org
Cc: Norbert Szetei <norbert@doyensec.com>,
	Guillaume Nault <gnault@redhat.com>
Subject: [PATCH net-next] pppoe: pass bound packets directly to generic PPP
Date: Tue,  4 Aug 2026 15:44:50 +0800	[thread overview]
Message-ID: <20260804074452.75548-1-qingfang.deng@linux.dev> (raw)

Bound PPPoE sockets pass received frames to the generic PPP layer. They
currently do so through __sk_receive_skb(), which takes the socket BH
lock and serializes calls to ppp_input().

The lock originally prevented ppp_input() from racing with
ppp_unregister_channel(). Commit ec4215683e47 ("ppp: defer channel free
to an RCU grace period to fix pppol2tp RX UAF") now keeps the generic
PPP channel alive until in-flight RCU readers have completed, so bound
packets can be passed directly to ppp_input() from pppoe_rcv().

That lifetime guarantee does not cover reuse of the ppp_channel embedded
in struct pppox_sock. An RX handler can find the old session before it
is unhashed, then resume after disconnect and reconnect have cleared and
re-registered po->chan. It could then race initialization of the new
channel or pass an old-session packet through it.

After unhashing an old session, call synchronize_net() before clearing
and reusing po->chan. This drains every receive path that could have
found the old binding while retaining concurrent delivery for the active
session.

Assisted-by: Codex:GPT-5.6
Signed-off-by: Qingfang Deng <qingfang.deng@linux.dev>
---
 drivers/net/ppp/pppoe.c | 19 +++++++++++++++----
 1 file changed, 15 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ppp/pppoe.c b/drivers/net/ppp/pppoe.c
index 6874a1a8edaf..062411624182 100644
--- a/drivers/net/ppp/pppoe.c
+++ b/drivers/net/ppp/pppoe.c
@@ -345,10 +345,10 @@ static struct notifier_block pppoe_notifier = {
 
 /************************************************************************
  *
- * Do the real work of receiving a PPPoE Session frame.
+ * Backlog receive a PPPoE Session frame and deliver to userspace.
  *
  ***********************************************************************/
-static int pppoe_rcv_core(struct sock *sk, struct sk_buff *skb)
+static int pppoe_backlog_rcv(struct sock *sk, struct sk_buff *skb)
 {
 	struct pppox_sock *po = pppox_sk(sk);
 
@@ -373,7 +373,7 @@ static int pppoe_rcv_core(struct sock *sk, struct sk_buff *skb)
 
 /************************************************************************
  *
- * Receive wrapper called in BH context.
+ * Receive a PPPoE Session frame.
  *
  ***********************************************************************/
 static int pppoe_rcv(struct sk_buff *skb, struct net_device *dev,
@@ -420,6 +420,10 @@ static int pppoe_rcv(struct sk_buff *skb, struct net_device *dev,
 	if (!po)
 		goto drop;
 
+	if (likely(po->sk.sk_state & PPPOX_BOUND)) {
+		ppp_input(&po->chan, skb);
+		return NET_RX_SUCCESS;
+	}
 	return __sk_receive_skb(&po->sk, skb, 0, 1, false);
 
 drop:
@@ -524,7 +528,7 @@ static int pppoe_create(struct net *net, struct socket *sock, int kern)
 	sock->state	= SS_UNCONNECTED;
 	sock->ops	= &pppoe_ops;
 
-	sk->sk_backlog_rcv	= pppoe_rcv_core;
+	sk->sk_backlog_rcv	= pppoe_backlog_rcv;
 	sk->sk_destruct		= pppoe_destruct;
 	sk->sk_state		= PPPOX_NONE;
 	sk->sk_type		= SOCK_STREAM;
@@ -625,6 +629,13 @@ static int pppoe_connect(struct socket *sock, struct sockaddr_unsized *uservaddr
 		pn = pppoe_pernet(sock_net(sk));
 		delete_item(pn, po->pppoe_pa.sid,
 			    po->pppoe_pa.remote, po->pppoe_ifindex);
+
+		/* pppoe_rcv() can call ppp_input() without taking the socket
+		 * lock. Once the socket is unhashed, wait for any receive path
+		 * that found it earlier before clearing and reusing po->chan.
+		 */
+		synchronize_net();
+
 		if (po->pppoe_dev) {
 			dev_put(po->pppoe_dev);
 			po->pppoe_dev = NULL;
-- 
2.43.0


             reply	other threads:[~2026-08-04  7:45 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-04  7:44 Qingfang Deng [this message]
2026-08-06  1:28 ` [PATCH net-next] pppoe: pass bound packets directly to generic PPP Jakub Kicinski
2026-08-06  3:11   ` Qingfang Deng

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260804074452.75548-1-qingfang.deng@linux.dev \
    --to=qingfang.deng@linux.dev \
    --cc=andrew+netdev@lunn.ch \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=ericwouds@gmail.com \
    --cc=gnault@redhat.com \
    --cc=kees@kernel.org \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-ppp@vger.kernel.org \
    --cc=manizada@pm.me \
    --cc=nbd@nbd.name \
    --cc=netdev@vger.kernel.org \
    --cc=norbert@doyensec.com \
    --cc=pabeni@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.