Skip to content

Commit cfc34c6

Browse files
mrpresfX-bot
authored andcommitted
ppp: Fix KMSAN uninit-value warning with bpf
[ Upstream commit 4c2d14c40a68678d885eab4008a0129646805bae ] 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 <[email protected]> Fixes: 1da177e ("Linux-2.6.12-rc2") Reported-by: [email protected] Closes: https://lore.kernel.org/bpf/[email protected]/ Signed-off-by: Jiayuan Chen <[email protected]> Reviewed-by: Simon Horman <[email protected]> Link: https://patch.msgid.link/[email protected] Signed-off-by: Jakub Kicinski <[email protected]> Signed-off-by: Sasha Levin <[email protected]>
1 parent e54848d commit cfc34c6

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
@@ -1627,10 +1638,10 @@ ppp_send_frame(struct ppp *ppp, struct sk_buff *skb)
16271638

16281639
if (proto < 0x8000) {
16291640
#ifdef CONFIG_PPP_FILTER
1630-
/* check if we should pass this packet */
1631-
/* the filter instructions are constructed assuming
1632-
a four-byte PPP header on each packet */
1633-
*(u8 *)skb_push(skb, 2) = 1;
1641+
/* check if the packet passes the pass and active filters.
1642+
* See comment for PPP_FILTER_OUTBOUND_TAG above.
1643+
*/
1644+
*(__be16 *)skb_push(skb, 2) = htons(PPP_FILTER_OUTBOUND_TAG);
16341645
if (ppp->pass_filter &&
16351646
BPF_PROG_RUN(ppp->pass_filter, skb) == 0) {
16361647
if (ppp->debug & 1)
@@ -2309,14 +2320,13 @@ ppp_receive_nonmp_frame(struct ppp *ppp, struct sk_buff *skb)
23092320
/* network protocol frame - give it to the kernel */
23102321

23112322
#ifdef CONFIG_PPP_FILTER
2312-
/* check if the packet passes the pass and active filters */
2313-
/* the filter instructions are constructed assuming
2314-
a four-byte PPP header on each packet */
23152323
if (ppp->pass_filter || ppp->active_filter) {
23162324
if (skb_unclone(skb, GFP_ATOMIC))
23172325
goto err;
2318-
2319-
*(u8 *)skb_push(skb, 2) = 0;
2326+
/* Check if the packet passes the pass and active filters.
2327+
* See comment for PPP_FILTER_INBOUND_TAG above.
2328+
*/
2329+
*(__be16 *)skb_push(skb, 2) = htons(PPP_FILTER_INBOUND_TAG);
23202330
if (ppp->pass_filter &&
23212331
BPF_PROG_RUN(ppp->pass_filter, skb) == 0) {
23222332
if (ppp->debug & 1)

0 commit comments

Comments
 (0)