[PATCH net] ppp_synctty: ensure a writeable skb header

Qingfang Deng posted 1 patch 2 weeks, 3 days ago
drivers/net/ppp/ppp_synctty.c | 14 +++-----------
1 file changed, 3 insertions(+), 11 deletions(-)
[PATCH net] ppp_synctty: ensure a writeable skb header
Posted by Qingfang Deng 2 weeks, 3 days ago
ppp_sync_txmunge() checks headroom before prepending the address and
control bytes, but does not ensure that the skb header is writable.
A received skb can reach this function through PPP channel bridging
without passing through ppp_start_xmit(), which calls skb_cow_head().

For example, a PPPoE frame may share its buffer with a clone queued to
an AF_PACKET socket. If it is bridged to a synchronous tty channel, the
address/control bytes can overwrite data still visible to that socket.

Use skb_cow_head() to ensure both sufficient headroom and a writable
header.

Fixes: 4cf476ced45d ("ppp: add PPPIOCBRIDGECHAN and PPPIOCUNBRIDGECHAN ioctls")
Signed-off-by: Qingfang Deng <qingfang.deng@linux.dev>
---
 drivers/net/ppp/ppp_synctty.c | 14 +++-----------
 1 file changed, 3 insertions(+), 11 deletions(-)

diff --git a/drivers/net/ppp/ppp_synctty.c b/drivers/net/ppp/ppp_synctty.c
index f87d43faeeab..ebd62a7ab54b 100644
--- a/drivers/net/ppp/ppp_synctty.c
+++ b/drivers/net/ppp/ppp_synctty.c
@@ -455,17 +455,9 @@ ppp_sync_txmunge(struct syncppp *ap, struct sk_buff *skb)
 
 	/* prepend address/control fields if necessary */
 	if ((ap->flags & SC_COMP_AC) == 0 || islcp) {
-		if (skb_headroom(skb) < 2) {
-			struct sk_buff *npkt = dev_alloc_skb(skb->len + 2);
-			if (npkt == NULL) {
-				kfree_skb(skb);
-				return NULL;
-			}
-			skb_reserve(npkt,2);
-			skb_copy_from_linear_data(skb,
-				      skb_put(npkt, skb->len), skb->len);
-			consume_skb(skb);
-			skb = npkt;
+		if (skb_cow_head(skb, 2)) {
+			kfree_skb(skb);
+			return NULL;
 		}
 		skb_push(skb,2);
 		skb->data[0] = PPP_ALLSTATIONS;
-- 
2.43.0
Re: [PATCH net] ppp_synctty: ensure a writeable skb header
Posted by Eric Dumazet 2 weeks, 3 days ago
On Tue, Sep 8, 2026 at 9:21 AM Qingfang Deng <qingfang.deng@linux.dev> wrote:
>
> ppp_sync_txmunge() checks headroom before prepending the address and
> control bytes, but does not ensure that the skb header is writable.
> A received skb can reach this function through PPP channel bridging
> without passing through ppp_start_xmit(), which calls skb_cow_head().
>
> For example, a PPPoE frame may share its buffer with a clone queued to
> an AF_PACKET socket. If it is bridged to a synchronous tty channel, the
> address/control bytes can overwrite data still visible to that socket.
>
> Use skb_cow_head() to ensure both sufficient headroom and a writable
> header.
>
> Fixes: 4cf476ced45d ("ppp: add PPPIOCBRIDGECHAN and PPPIOCUNBRIDGECHAN ioctls")
> Signed-off-by: Qingfang Deng <qingfang.deng@linux.dev>

SGTM. I will fix pppoatm_send() in net/atm/pppoatm.c which has a similar bug.

Reviewed-by: Eric Dumazet <edumazet@google.com>