]>
Commit | Line | Data |
---|---|---|
6092349c GKH |
1 | From ff91059932401894e6c86341915615c5eb0eca48 Mon Sep 17 00:00:00 2001 |
2 | From: Jakub Sitnicki <jakub@cloudflare.com> | |
3 | Date: Tue, 2 Apr 2024 12:46:21 +0200 | |
4 | Subject: bpf, sockmap: Prevent lock inversion deadlock in map delete elem | |
5 | ||
6 | From: Jakub Sitnicki <jakub@cloudflare.com> | |
7 | ||
8 | commit ff91059932401894e6c86341915615c5eb0eca48 upstream. | |
9 | ||
10 | syzkaller started using corpuses where a BPF tracing program deletes | |
11 | elements from a sockmap/sockhash map. Because BPF tracing programs can be | |
12 | invoked from any interrupt context, locks taken during a map_delete_elem | |
13 | operation must be hardirq-safe. Otherwise a deadlock due to lock inversion | |
14 | is possible, as reported by lockdep: | |
15 | ||
16 | CPU0 CPU1 | |
17 | ---- ---- | |
18 | lock(&htab->buckets[i].lock); | |
19 | local_irq_disable(); | |
20 | lock(&host->lock); | |
21 | lock(&htab->buckets[i].lock); | |
22 | <Interrupt> | |
23 | lock(&host->lock); | |
24 | ||
25 | Locks in sockmap are hardirq-unsafe by design. We expects elements to be | |
26 | deleted from sockmap/sockhash only in task (normal) context with interrupts | |
27 | enabled, or in softirq context. | |
28 | ||
29 | Detect when map_delete_elem operation is invoked from a context which is | |
30 | _not_ hardirq-unsafe, that is interrupts are disabled, and bail out with an | |
31 | error. | |
32 | ||
33 | Note that map updates are not affected by this issue. BPF verifier does not | |
34 | allow updating sockmap/sockhash from a BPF tracing program today. | |
35 | ||
36 | Fixes: 604326b41a6f ("bpf, sockmap: convert to generic sk_msg interface") | |
37 | Reported-by: xingwei lee <xrivendell7@gmail.com> | |
38 | Reported-by: yue sun <samsun1006219@gmail.com> | |
39 | Reported-by: syzbot+bc922f476bd65abbd466@syzkaller.appspotmail.com | |
40 | Reported-by: syzbot+d4066896495db380182e@syzkaller.appspotmail.com | |
41 | Signed-off-by: Jakub Sitnicki <jakub@cloudflare.com> | |
42 | Signed-off-by: Daniel Borkmann <daniel@iogearbox.net> | |
43 | Tested-by: syzbot+d4066896495db380182e@syzkaller.appspotmail.com | |
44 | Acked-by: John Fastabend <john.fastabend@gmail.com> | |
45 | Closes: https://syzkaller.appspot.com/bug?extid=d4066896495db380182e | |
46 | Closes: https://syzkaller.appspot.com/bug?extid=bc922f476bd65abbd466 | |
47 | Link: https://lore.kernel.org/bpf/20240402104621.1050319-1-jakub@cloudflare.com | |
48 | Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> | |
49 | --- | |
50 | net/core/sock_map.c | 6 ++++++ | |
51 | 1 file changed, 6 insertions(+) | |
52 | ||
53 | --- a/net/core/sock_map.c | |
54 | +++ b/net/core/sock_map.c | |
55 | @@ -413,6 +413,9 @@ static int __sock_map_delete(struct bpf_ | |
56 | struct sock *sk; | |
57 | int err = 0; | |
58 | ||
59 | + if (irqs_disabled()) | |
60 | + return -EOPNOTSUPP; /* locks here are hardirq-unsafe */ | |
61 | + | |
62 | raw_spin_lock_bh(&stab->lock); | |
63 | sk = *psk; | |
64 | if (!sk_test || sk_test == sk) | |
65 | @@ -926,6 +929,9 @@ static int sock_hash_delete_elem(struct | |
66 | struct bpf_shtab_elem *elem; | |
67 | int ret = -ENOENT; | |
68 | ||
69 | + if (irqs_disabled()) | |
70 | + return -EOPNOTSUPP; /* locks here are hardirq-unsafe */ | |
71 | + | |
72 | hash = sock_hash_bucket_hash(key, key_size); | |
73 | bucket = sock_hash_select_bucket(htab, hash); | |
74 |