[PATCH net-next v5] ppp: Fix KMSAN uninit-value warning with bpf

[PATCH net-next v5] ppp: Fix KMSAN uninit-value warning with bpf

[Jiayuan Chen]

Syzbot caught an "KMSAN: uninit-value" warning [1], which is caused by the
ppp driver not initializing a 2-byte header when using socket filter.

The following code can generate a PPP filter BPF program:
'''
struct bpf_program fp;
pcap_t *handle;
handle = pcap_open_dead(DLT_PPP_PPPD, 65535);
pcap_compile(handle, &fp, "ip and outbound", 0, 0);
bpf_dump(&fp, 1);
'''
Its output is:
'''
(000) ldh [2]
(001) jeq #0x21 jt 2 jf 5
(002) ldb [0]
(003) jeq #0x1 jt 4 jf 5
(004) ret #65535
(005) ret #0
'''
Wen can find similar code at the following link:
https://github.com/ppp-project/ppp/blob/master/pppd/options.c#L1680
The maintainer of this code repository is also the original maintainer
of the ppp driver.

As you can see the BPF program skips 2 bytes of data and then reads the
'Protocol' field to determine if it's an IP packet. Then it read the first
byte of the first 2 bytes to determine the direction.

The issue is that only the first byte indicating direction is initialized
in current ppp driver code while the second byte is not initialized.

For normal BPF programs generated by libpcap, uninitialized data won't be
used, so it's not a problem. However, for carefully crafted BPF programs,
such as those generated by syzkaller [2], which start reading from offset
0, the uninitialized data will be used and caught by KMSAN.

[1] https://syzkaller.appspot.com/bug?extid=853242d9c9917165d791
[2] https://syzkaller.appspot.com/text?tag=ReproC&x=11994913980000

Cc: Paul Mackerras <paulus@samba.org>
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Reported-by: syzbot+853242d9c9917165d791@syzkaller.appspotmail.com
Closes: https://lore.kernel.org/bpf/000000000000dea025060d6bc3bc@google.com/
Signed-off-by: Jiayuan Chen <jiayuan.chen@linux.dev>
---
 drivers/net/ppp/ppp_generic.c | 28 +++++++++++++++++++---------
 1 file changed, 19 insertions(+), 9 deletions(-)

diff --git a/drivers/net/ppp/ppp_generic.c b/drivers/net/ppp/ppp_generic.c
index 4583e15ad03a..1420c4efa48e 100644
--- a/drivers/net/ppp/ppp_generic.c
+++ b/drivers/net/ppp/ppp_generic.c
@@ -72,6 +72,17 @@
 #define PPP_PROTO_LEN	2
 #define PPP_LCP_HDRLEN	4
 
+/* The filter instructions generated by libpcap are constructed
+ * assuming a four-byte PPP header on each packet, where the last
+ * 2 bytes are the protocol field defined in the RFC and the first
+ * byte of the first 2 bytes indicates the direction.
+ * The second byte is currently unused, but we still need to initialize
+ * it to prevent crafted BPF programs from reading them which would
+ * cause reading of uninitialized data.
+ */
+#define PPP_FILTER_OUTBOUND_TAG 0x0100
+#define PPP_FILTER_INBOUND_TAG  0x0000
+
 /*
  * An instance of /dev/ppp can be associated with either a ppp
  * interface unit or a ppp channel.  In both cases, file->private_data
@@ -1762,10 +1773,10 @@ ppp_send_frame(struct ppp *ppp, struct sk_buff *skb)
 
 	if (proto < 0x8000) {
 #ifdef CONFIG_PPP_FILTER
-		/* check if we should pass this packet */
-		/* the filter instructions are constructed assuming
-		   a four-byte PPP header on each packet */
-		*(u8 *)skb_push(skb, 2) = 1;
+		/* check if the packet passes the pass and active filters.
+		 * See comment for PPP_FILTER_OUTBOUND_TAG above.
+		 */
+		*(__be16 *)skb_push(skb, 2) = htons(PPP_FILTER_OUTBOUND_TAG);
 		if (ppp->pass_filter &&
 		    bpf_prog_run(ppp->pass_filter, skb) == 0) {
 			if (ppp->debug & 1)
@@ -2482,14 +2493,13 @@ ppp_receive_nonmp_frame(struct ppp *ppp, struct sk_buff *skb)
 		/* network protocol frame - give it to the kernel */
 
 #ifdef CONFIG_PPP_FILTER
-		/* check if the packet passes the pass and active filters */
-		/* the filter instructions are constructed assuming
-		   a four-byte PPP header on each packet */
 		if (ppp->pass_filter || ppp->active_filter) {
 			if (skb_unclone(skb, GFP_ATOMIC))
 				goto err;
-
-			*(u8 *)skb_push(skb, 2) = 0;
+			/* Check if the packet passes the pass and active filters.
+			 * See comment for PPP_FILTER_INBOUND_TAG above.
+			 */
+			*(__be16 *)skb_push(skb, 2) = htons(PPP_FILTER_INBOUND_TAG);
 			if (ppp->pass_filter &&
 			    bpf_prog_run(ppp->pass_filter, skb) == 0) {
 				if (ppp->debug & 1)
-- 
2.47.1

Re: [PATCH net-next v5] ppp: Fix KMSAN uninit-value warning with bpf

[Simon Horman]

On Fri, Feb 28, 2025 at 10:14:08PM +0800, Jiayuan Chen wrote:
> Syzbot caught an "KMSAN: uninit-value" warning [1], which is caused by the
> ppp driver not initializing a 2-byte header when using socket filter.
> 
> The following code can generate a PPP filter BPF program:
> '''
> struct bpf_program fp;
> pcap_t *handle;
> handle = pcap_open_dead(DLT_PPP_PPPD, 65535);
> pcap_compile(handle, &fp, "ip and outbound", 0, 0);
> bpf_dump(&fp, 1);
> '''
> Its output is:
> '''
> (000) ldh [2]
> (001) jeq #0x21 jt 2 jf 5
> (002) ldb [0]
> (003) jeq #0x1 jt 4 jf 5
> (004) ret #65535
> (005) ret #0
> '''
> Wen can find similar code at the following link:
> https://github.com/ppp-project/ppp/blob/master/pppd/options.c#L1680
> The maintainer of this code repository is also the original maintainer
> of the ppp driver.
> 
> As you can see the BPF program skips 2 bytes of data and then reads the
> 'Protocol' field to determine if it's an IP packet. Then it read the first
> byte of the first 2 bytes to determine the direction.
> 
> The issue is that only the first byte indicating direction is initialized
> in current ppp driver code while the second byte is not initialized.
> 
> For normal BPF programs generated by libpcap, uninitialized data won't be
> used, so it's not a problem. However, for carefully crafted BPF programs,
> such as those generated by syzkaller [2], which start reading from offset
> 0, the uninitialized data will be used and caught by KMSAN.
> 
> [1] https://syzkaller.appspot.com/bug?extid=853242d9c9917165d791
> [2] https://syzkaller.appspot.com/text?tag=ReproC&x=11994913980000
> 
> Cc: Paul Mackerras <paulus@samba.org>
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Reported-by: syzbot+853242d9c9917165d791@syzkaller.appspotmail.com
> Closes: https://lore.kernel.org/bpf/000000000000dea025060d6bc3bc@google.com/
> Signed-off-by: Jiayuan Chen <jiayuan.chen@linux.dev>

Reviewed-by: Simon Horman <horms@kernel.org>

Re: [PATCH net-next v5] ppp: Fix KMSAN uninit-value warning with bpf

[patchwork-bot+netdevbpf]

Hello:

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

On Fri, 28 Feb 2025 22:14:08 +0800 you wrote:
> Syzbot caught an "KMSAN: uninit-value" warning [1], which is caused by the
> ppp driver not initializing a 2-byte header when using socket filter.
> 
> The following code can generate a PPP filter BPF program:
> '''
> struct bpf_program fp;
> pcap_t *handle;
> handle = pcap_open_dead(DLT_PPP_PPPD, 65535);
> pcap_compile(handle, &fp, "ip and outbound", 0, 0);
> bpf_dump(&fp, 1);
> '''
> Its output is:
> '''
> (000) ldh [2]
> (001) jeq #0x21 jt 2 jf 5
> (002) ldb [0]
> (003) jeq #0x1 jt 4 jf 5
> (004) ret #65535
> (005) ret #0
> '''
> Wen can find similar code at the following link:
> https://github.com/ppp-project/ppp/blob/master/pppd/options.c#L1680
> The maintainer of this code repository is also the original maintainer
> of the ppp driver.
> 
> [...]

Here is the summary with links:
  - [net-next,v5] ppp: Fix KMSAN uninit-value warning with bpf
    https://git.kernel.org/netdev/net/c/4c2d14c40a68

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