[PATCH net] pptp: ensure minimal skb length in pptp_xmit()

[PATCH net] pptp: ensure minimal skb length in pptp_xmit()

[Eric Dumazet]

Commit aabc6596ffb3 ("net: ppp: Add bound checking for skb data
on ppp_sync_txmung") fixed ppp_sync_txmunge()

We need a similar fix in pptp_xmit(), otherwise we might
read uninit data as reported by syzbot.

BUG: KMSAN: uninit-value in pptp_xmit+0xc34/0x2720 drivers/net/ppp/pptp.c:193
  pptp_xmit+0xc34/0x2720 drivers/net/ppp/pptp.c:193
  ppp_channel_bridge_input drivers/net/ppp/ppp_generic.c:2290 [inline]
  ppp_input+0x1d6/0xe60 drivers/net/ppp/ppp_generic.c:2314
  pppoe_rcv_core+0x1e8/0x760 drivers/net/ppp/pppoe.c:379
  sk_backlog_rcv+0x142/0x420 include/net/sock.h:1148
  __release_sock+0x1d3/0x330 net/core/sock.c:3213
  release_sock+0x6b/0x270 net/core/sock.c:3767
  pppoe_sendmsg+0x15d/0xcb0 drivers/net/ppp/pppoe.c:904
  sock_sendmsg_nosec net/socket.c:712 [inline]
  __sock_sendmsg+0x330/0x3d0 net/socket.c:727
  ____sys_sendmsg+0x893/0xd80 net/socket.c:2566
  ___sys_sendmsg+0x271/0x3b0 net/socket.c:2620
  __sys_sendmmsg+0x2d9/0x7c0 net/socket.c:2709

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Reported-by: syzbot+afad90ffc8645324afe5@syzkaller.appspotmail.com
Closes: https://lore.kernel.org/netdev/68887d86.a00a0220.b12ec.00cd.GAE@google.com/T/#u
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 drivers/net/ppp/pptp.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ppp/pptp.c b/drivers/net/ppp/pptp.c
index 5feaa70b5f47e6cd33fbaff33f6715f42c4d71b5..4cd6f67bd5d3520308ee4f8d68547a1bc8a7bfd3 100644
--- a/drivers/net/ppp/pptp.c
+++ b/drivers/net/ppp/pptp.c
@@ -159,9 +159,7 @@ static int pptp_xmit(struct ppp_channel *chan, struct sk_buff *skb)
 	int len;
 	unsigned char *data;
 	__u32 seq_recv;
-
-
-	struct rtable *rt;
+	struct rtable *rt = NULL;
 	struct net_device *tdev;
 	struct iphdr  *iph;
 	int    max_headroom;
@@ -179,16 +177,20 @@ static int pptp_xmit(struct ppp_channel *chan, struct sk_buff *skb)
 
 	if (skb_headroom(skb) < max_headroom || skb_cloned(skb) || skb_shared(skb)) {
 		struct sk_buff *new_skb = skb_realloc_headroom(skb, max_headroom);
-		if (!new_skb) {
-			ip_rt_put(rt);
+
+		if (!new_skb)
 			goto tx_error;
-		}
+
 		if (skb->sk)
 			skb_set_owner_w(new_skb, skb->sk);
 		consume_skb(skb);
 		skb = new_skb;
 	}
 
+	/* Ensure we can safely access protocol field and LCP code */
+	if (!pskb_may_pull(skb, 3))
+		goto tx_error;
+
 	data = skb->data;
 	islcp = ((data[0] << 8) + data[1]) == PPP_LCP && 1 <= data[2] && data[2] <= 7;
 
@@ -262,6 +264,7 @@ static int pptp_xmit(struct ppp_channel *chan, struct sk_buff *skb)
 	return 1;
 
 tx_error:
+	ip_rt_put(rt);
 	kfree_skb(skb);
 	return 1;
 }
-- 
2.50.1.487.gc89ff58d15-goog

Re: [PATCH net] pptp: ensure minimal skb length in pptp_xmit()

[Dawid Osuchowski]

On 2025-07-29 10:02 AM, Eric Dumazet wrote:
> Commit aabc6596ffb3 ("net: ppp: Add bound checking for skb data
> on ppp_sync_txmung") fixed ppp_sync_txmunge()
> 
> We need a similar fix in pptp_xmit(), otherwise we might
> read uninit data as reported by syzbot.
> 
> BUG: KMSAN: uninit-value in pptp_xmit+0xc34/0x2720 drivers/net/ppp/pptp.c:193
>    pptp_xmit+0xc34/0x2720 drivers/net/ppp/pptp.c:193
>    ppp_channel_bridge_input drivers/net/ppp/ppp_generic.c:2290 [inline]
>    ppp_input+0x1d6/0xe60 drivers/net/ppp/ppp_generic.c:2314
>    pppoe_rcv_core+0x1e8/0x760 drivers/net/ppp/pppoe.c:379
>    sk_backlog_rcv+0x142/0x420 include/net/sock.h:1148
>    __release_sock+0x1d3/0x330 net/core/sock.c:3213
>    release_sock+0x6b/0x270 net/core/sock.c:3767
>    pppoe_sendmsg+0x15d/0xcb0 drivers/net/ppp/pppoe.c:904
>    sock_sendmsg_nosec net/socket.c:712 [inline]
>    __sock_sendmsg+0x330/0x3d0 net/socket.c:727
>    ____sys_sendmsg+0x893/0xd80 net/socket.c:2566
>    ___sys_sendmsg+0x271/0x3b0 net/socket.c:2620
>    __sys_sendmmsg+0x2d9/0x7c0 net/socket.c:2709
> 
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Reported-by: syzbot+afad90ffc8645324afe5@syzkaller.appspotmail.com
> Closes: https://lore.kernel.org/netdev/68887d86.a00a0220.b12ec.00cd.GAE@google.com/T/#u
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> ---
>   drivers/net/ppp/pptp.c | 15 +++++++++------
>   1 file changed, 9 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/net/ppp/pptp.c b/drivers/net/ppp/pptp.c
> index 5feaa70b5f47e6cd33fbaff33f6715f42c4d71b5..4cd6f67bd5d3520308ee4f8d68547a1bc8a7bfd3 100644
> --- a/drivers/net/ppp/pptp.c
> +++ b/drivers/net/ppp/pptp.c
> @@ -159,9 +159,7 @@ static int pptp_xmit(struct ppp_channel *chan, struct sk_buff *skb)
>   	int len;
>   	unsigned char *data;
>   	__u32 seq_recv;

nit: the declarations do not follow RCT, but I guess it might be 
additional churn to reorder in this patch

> -
> -
> -	struct rtable *rt;
> +	struct rtable *rt = NULL;
>   	struct net_device *tdev;
>   	struct iphdr  *iph;
>   	int    max_headroom;
> @@ -179,16 +177,20 @@ static int pptp_xmit(struct ppp_channel *chan, struct sk_buff *skb)
>   
>   	if (skb_headroom(skb) < max_headroom || skb_cloned(skb) || skb_shared(skb)) {
>   		struct sk_buff *new_skb = skb_realloc_headroom(skb, max_headroom);
> -		if (!new_skb) {
> -			ip_rt_put(rt);
> +
> +		if (!new_skb)
>   			goto tx_error;
> -		}
> +
>   		if (skb->sk)
>   			skb_set_owner_w(new_skb, skb->sk);
>   		consume_skb(skb);
>   		skb = new_skb;
>   	}
>   
> +	/* Ensure we can safely access protocol field and LCP code */
> +	if (!pskb_may_pull(skb, 3))
> +		goto tx_error;
> +
>   	data = skb->data;
>   	islcp = ((data[0] << 8) + data[1]) == PPP_LCP && 1 <= data[2] && data[2] <= 7;
>   
> @@ -262,6 +264,7 @@ static int pptp_xmit(struct ppp_channel *chan, struct sk_buff *skb)
>   	return 1;
>   
>   tx_error:
> +	ip_rt_put(rt);
>   	kfree_skb(skb);
>   	return 1;
>   }

Reviewed-by: Dawid Osuchowski <dawid.osuchowski@linux.intel.com>

Thanks,
Dawid

Re: [PATCH net] pptp: ensure minimal skb length in pptp_xmit()

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Tue, 29 Jul 2025 08:02:07 +0000 you wrote:
> Commit aabc6596ffb3 ("net: ppp: Add bound checking for skb data
> on ppp_sync_txmung") fixed ppp_sync_txmunge()
> 
> We need a similar fix in pptp_xmit(), otherwise we might
> read uninit data as reported by syzbot.
> 
> BUG: KMSAN: uninit-value in pptp_xmit+0xc34/0x2720 drivers/net/ppp/pptp.c:193
>   pptp_xmit+0xc34/0x2720 drivers/net/ppp/pptp.c:193
>   ppp_channel_bridge_input drivers/net/ppp/ppp_generic.c:2290 [inline]
>   ppp_input+0x1d6/0xe60 drivers/net/ppp/ppp_generic.c:2314
>   pppoe_rcv_core+0x1e8/0x760 drivers/net/ppp/pppoe.c:379
>   sk_backlog_rcv+0x142/0x420 include/net/sock.h:1148
>   __release_sock+0x1d3/0x330 net/core/sock.c:3213
>   release_sock+0x6b/0x270 net/core/sock.c:3767
>   pppoe_sendmsg+0x15d/0xcb0 drivers/net/ppp/pppoe.c:904
>   sock_sendmsg_nosec net/socket.c:712 [inline]
>   __sock_sendmsg+0x330/0x3d0 net/socket.c:727
>   ____sys_sendmsg+0x893/0xd80 net/socket.c:2566
>   ___sys_sendmsg+0x271/0x3b0 net/socket.c:2620
>   __sys_sendmmsg+0x2d9/0x7c0 net/socket.c:2709
> 
> [...]

Here is the summary with links:
  - [net] pptp: ensure minimal skb length in pptp_xmit()
    https://git.kernel.org/netdev/net/c/de9c4861fb42

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html