Skip to contentSkip to conversation

ppp_synctty: ensure a writeable skb header

PATCH Applied Qingfang Deng 3 participants 3 messages Sep 8 – Sep 10

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 ppp_synctty: ensure a writeable skb header

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")
---
 drivers/net/ppp/ppp_synctty.c | 14 +++-----------
 1 file changed, 3 insertions(+), 11 deletions(-)
drivers/net/ppp/ppp_synctty.c +3 -11
index f87d43faeeab..ebd62a7ab54b 100644
@@ -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;
Signature · 1 line
2.43.0
Reviewed-by on v1 1/1
#1 lore raw
Quoted 10 lines · Qingfang Deng

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.

Quoted 5 lines

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

Fixes 4cf476ced45d ("ppp: add PPPIOCBRIDGECHAN and PPPIOCUNBRIDGECHAN ioctls")

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

#2 lore raw
In reply to Qingfang Deng
This patch was applied to netdev/net.git (main) by Jakub Kicinski <[email protected]>:

Hello:

This patch was applied to netdev/net.git (main)
by Jakub Kicinski <[email protected]>:

Quoted 10 lines · On Tue, 8 Sep 2026 15:21:31 +0800 you 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.

[...]

Here is the summary with links:
- [net] ppp_synctty: ensure a writeable skb header

    https://git.kernel.org/netdev/net/c/8aaeb56aff2a

You are awesome, thank you!

Signature · 2 lines
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html