bpf.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net-next v4 0/1] ppp: Fix KMSAN uninit-value warning with bpf
@ 2025-02-26  1:36 Jiayuan Chen
  2025-02-26  1:36 ` [PATCH net-next v4 1/1] ppp: Fix KMSAN warning by initializing 2-byte header Jiayuan Chen
  0 siblings, 1 reply; 5+ messages in thread
From: Jiayuan Chen @ 2025-02-26  1:36 UTC (permalink / raw)
  To: horms, kuba
  Cc: bpf, netdev, andrew+netdev, davem, edumazet, pabeni, ricardo,
	viro, dmantipov, aleksander.lobakin, linux-ppp, linux-kernel,
	mrpre, Jiayuan Chen, Paul Mackerras

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

Here's a detailed explanation:

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.


3. Current problem
The problem is that the skb->data generated by ppp_write() starts from the
'Protocol' field.

But the BPF program skips 2 bytes of data and then reads the 'Protocol'
field to determine if it's an IP packet just like the comment in
'drivers/net/ppp/ppp_generic.c':
/* the filter instructions are constructed assuming
   a four-byte PPP header on each packet */

In the current PPP driver implementation, to correctly use the BPF filter
program, a 2-byte header is added, after running the socket filter, it's
restored:
'''
1768 *(u8 *)skb_push(skb, 2) = 1;
1770 bpf_prog_run()
1782 skb_pull(skb, 2);
'''

The issue is that only the first byte indicating direction is initialized,
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.

4. Fix
The fix is simple: initialize the entire 2-byte header.

Cc: Paul Mackerras <paulus@samba.org>

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

---
v3 -> v4:
Use macro instead.
Use __be16 to suppress compilation warnings.

Jiayuan Chen (1):
  ppp: Fix KMSAN warning by initializing 2-byte header

 drivers/net/ppp/ppp_generic.c | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

-- 
2.47.1


^ permalink raw reply	[flat|nested] 5+ messages in thread
end of thread, other threads:[~2025-02-28 22:02 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-02-26  1:36 [PATCH net-next v4 0/1] ppp: Fix KMSAN uninit-value warning with bpf Jiayuan Chen
2025-02-26  1:36 ` [PATCH net-next v4 1/1] ppp: Fix KMSAN warning by initializing 2-byte header Jiayuan Chen
2025-02-28  1:48   ` Jakub Kicinski
2025-02-28  4:55     ` Jiayuan Chen
2025-02-28 22:02       ` Jakub Kicinski

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).