Skip to content

Commit 4c2d14c

Browse files
mrprekuba-moo
authored andcommitted
ppp: Fix KMSAN uninit-value warning with bpf
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> Link: https://patch.msgid.link/20250228141408.393864-1-jiayuan.chen@linux.dev Signed-off-by: Jakub Kicinski <kuba@kernel.org>
1 parent 78da8ab commit 4c2d14c

File tree

1 file changed

+19
-9
lines changed

1 file changed

+19
-9
lines changed

drivers/net/ppp/ppp_generic.c

Lines changed: 19 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,17 @@
7272
#define PPP_PROTO_LEN 2
7373
#define PPP_LCP_HDRLEN 4
7474

75+
/* The filter instructions generated by libpcap are constructed
76+
* assuming a four-byte PPP header on each packet, where the last
77+
* 2 bytes are the protocol field defined in the RFC and the first
78+
* byte of the first 2 bytes indicates the direction.
79+
* The second byte is currently unused, but we still need to initialize
80+
* it to prevent crafted BPF programs from reading them which would
81+
* cause reading of uninitialized data.
82+
*/
83+
#define PPP_FILTER_OUTBOUND_TAG 0x0100
84+
#define PPP_FILTER_INBOUND_TAG 0x0000
85+
7586
/*
7687
* An instance of /dev/ppp can be associated with either a ppp
7788
* 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)
17621773

17631774
if (proto < 0x8000) {
17641775
#ifdef CONFIG_PPP_FILTER
1765-
/* check if we should pass this packet */
1766-
/* the filter instructions are constructed assuming
1767-
a four-byte PPP header on each packet */
1768-
*(u8 *)skb_push(skb, 2) = 1;
1776+
/* check if the packet passes the pass and active filters.
1777+
* See comment for PPP_FILTER_OUTBOUND_TAG above.
1778+
*/
1779+
*(__be16 *)skb_push(skb, 2) = htons(PPP_FILTER_OUTBOUND_TAG);
17691780
if (ppp->pass_filter &&
17701781
bpf_prog_run(ppp->pass_filter, skb) == 0) {
17711782
if (ppp->debug & 1)
@@ -2482,14 +2493,13 @@ ppp_receive_nonmp_frame(struct ppp *ppp, struct sk_buff *skb)
24822493
/* network protocol frame - give it to the kernel */
24832494

24842495
#ifdef CONFIG_PPP_FILTER
2485-
/* check if the packet passes the pass and active filters */
2486-
/* the filter instructions are constructed assuming
2487-
a four-byte PPP header on each packet */
24882496
if (ppp->pass_filter || ppp->active_filter) {
24892497
if (skb_unclone(skb, GFP_ATOMIC))
24902498
goto err;
2491-
2492-
*(u8 *)skb_push(skb, 2) = 0;
2499+
/* Check if the packet passes the pass and active filters.
2500+
* See comment for PPP_FILTER_INBOUND_TAG above.
2501+
*/
2502+
*(__be16 *)skb_push(skb, 2) = htons(PPP_FILTER_INBOUND_TAG);
24932503
if (ppp->pass_filter &&
24942504
bpf_prog_run(ppp->pass_filter, skb) == 0) {
24952505
if (ppp->debug & 1)

0 commit comments

Comments
 (0)