]>
Commit | Line | Data |
---|---|---|
335f7cc0 SL |
1 | From ef13a9f8d56090e3bdee96e50e97e871fb26f6b2 Mon Sep 17 00:00:00 2001 |
2 | From: Sasha Levin <sashal@kernel.org> | |
3 | Date: Fri, 16 Feb 2024 12:36:57 +0100 | |
4 | Subject: net: skbuff: add overflow debug check to pull/push helpers | |
5 | ||
6 | From: Florian Westphal <fw@strlen.de> | |
7 | ||
8 | [ Upstream commit 219eee9c0d16f1b754a8b85275854ab17df0850a ] | |
9 | ||
10 | syzbot managed to trigger following splat: | |
11 | BUG: KASAN: use-after-free in __skb_flow_dissect+0x4a3b/0x5e50 | |
12 | Read of size 1 at addr ffff888208a4000e by task a.out/2313 | |
13 | [..] | |
14 | __skb_flow_dissect+0x4a3b/0x5e50 | |
15 | __skb_get_hash+0xb4/0x400 | |
16 | ip_tunnel_xmit+0x77e/0x26f0 | |
17 | ipip_tunnel_xmit+0x298/0x410 | |
18 | .. | |
19 | ||
20 | Analysis shows that the skb has a valid ->head, but bogus ->data | |
21 | pointer. | |
22 | ||
23 | skb->data gets its bogus value via the neigh layer, which does: | |
24 | ||
25 | 1556 __skb_pull(skb, skb_network_offset(skb)); | |
26 | ||
27 | ... and the skb was already dodgy at this point: | |
28 | ||
29 | skb_network_offset(skb) returns a negative value due to an | |
30 | earlier overflow of skb->network_header (u16). __skb_pull thus | |
31 | "adjusts" skb->data by a huge offset, pointing outside skb->head | |
32 | area. | |
33 | ||
34 | Allow debug builds to splat when we try to pull/push more than | |
35 | INT_MAX bytes. | |
36 | ||
37 | After this, the syzkaller reproducer yields a more precise splat | |
38 | before the flow dissector attempts to read off skb->data memory: | |
39 | ||
40 | WARNING: CPU: 5 PID: 2313 at include/linux/skbuff.h:2653 neigh_connected_output+0x28e/0x400 | |
41 | ip_finish_output2+0xb25/0xed0 | |
42 | iptunnel_xmit+0x4ff/0x870 | |
43 | ipgre_xmit+0x78e/0xbb0 | |
44 | ||
45 | Signed-off-by: Florian Westphal <fw@strlen.de> | |
46 | Reviewed-by: Simon Horman <horms@kernel.org> | |
47 | Link: https://lore.kernel.org/r/20240216113700.23013-1-fw@strlen.de | |
48 | Signed-off-by: Paolo Abeni <pabeni@redhat.com> | |
49 | Signed-off-by: Sasha Levin <sashal@kernel.org> | |
50 | --- | |
51 | include/linux/skbuff.h | 6 ++++++ | |
52 | 1 file changed, 6 insertions(+) | |
53 | ||
54 | diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h | |
55 | index 227f4514476b1..741115aeca2f1 100644 | |
56 | --- a/include/linux/skbuff.h | |
57 | +++ b/include/linux/skbuff.h | |
58 | @@ -2637,6 +2637,8 @@ static inline void skb_put_u8(struct sk_buff *skb, u8 val) | |
59 | void *skb_push(struct sk_buff *skb, unsigned int len); | |
60 | static inline void *__skb_push(struct sk_buff *skb, unsigned int len) | |
61 | { | |
62 | + DEBUG_NET_WARN_ON_ONCE(len > INT_MAX); | |
63 | + | |
64 | skb->data -= len; | |
65 | skb->len += len; | |
66 | return skb->data; | |
67 | @@ -2645,6 +2647,8 @@ static inline void *__skb_push(struct sk_buff *skb, unsigned int len) | |
68 | void *skb_pull(struct sk_buff *skb, unsigned int len); | |
69 | static inline void *__skb_pull(struct sk_buff *skb, unsigned int len) | |
70 | { | |
71 | + DEBUG_NET_WARN_ON_ONCE(len > INT_MAX); | |
72 | + | |
73 | skb->len -= len; | |
74 | if (unlikely(skb->len < skb->data_len)) { | |
75 | #if defined(CONFIG_DEBUG_NET) | |
76 | @@ -2669,6 +2673,8 @@ void *__pskb_pull_tail(struct sk_buff *skb, int delta); | |
77 | static inline enum skb_drop_reason | |
78 | pskb_may_pull_reason(struct sk_buff *skb, unsigned int len) | |
79 | { | |
80 | + DEBUG_NET_WARN_ON_ONCE(len > INT_MAX); | |
81 | + | |
82 | if (likely(len <= skb_headlen(skb))) | |
83 | return SKB_NOT_DROPPED_YET; | |
84 | ||
85 | -- | |
86 | 2.43.0 | |
87 |