Commit 8ebe667c41e054384df19f2f382bc415badfaee1
Committed by
David S. Miller
David S. Miller
1 parent
600ddd6825
bpf: rcu lock must not be held when calling copy_to_user()
BUG: sleeping function called from invalid context at mm/memory.c:3732
in_atomic(): 0, irqs_disabled(): 0, pid: 671, name: test_maps
1 lock held by test_maps/671:
#0: (rcu_read_lock){......}, at: [<0000000000264190>] map_lookup_elem+0xe8/0x260
Call Trace:
([<0000000000115b7e>] show_trace+0x12e/0x150)
[<0000000000115c40>] show_stack+0xa0/0x100
[<00000000009b163c>] dump_stack+0x74/0xc8
[<000000000017424a>] ___might_sleep+0x23a/0x248
[<00000000002b58e8>] might_fault+0x70/0xe8
[<0000000000264230>] map_lookup_elem+0x188/0x260
[<0000000000264716>] SyS_bpf+0x20e/0x840
Fix it by allocating temporary buffer to store map element value.
Fixes: db20fd2b0108 ("bpf: add lookup/update/delete/iterate methods to BPF maps")
Reported-by: Michael Holzheu <holzheu@linux.vnet.ibm.com>
Signed-off-by: Alexei Starovoitov <ast@plumgrid.com>
Acked-by: Daniel Borkmann <dborkman@redhat.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
Showing 1 changed file with 17 additions and 8 deletions Side-by-side Diff
kernel/bpf/syscall.c
| ... | ... | @@ -150,7 +150,7 @@ |
| 150 | 150 | int ufd = attr->map_fd; |
| 151 | 151 | struct fd f = fdget(ufd); |
| 152 | 152 | struct bpf_map *map; |
| 153 | - void *key, *value; | |
| 153 | + void *key, *value, *ptr; | |
| 154 | 154 | int err; |
| 155 | 155 | |
| 156 | 156 | if (CHECK_ATTR(BPF_MAP_LOOKUP_ELEM)) |
| 157 | 157 | |
| 158 | 158 | |
| 159 | 159 | |
| 160 | 160 | |
| ... | ... | @@ -169,20 +169,29 @@ |
| 169 | 169 | if (copy_from_user(key, ukey, map->key_size) != 0) |
| 170 | 170 | goto free_key; |
| 171 | 171 | |
| 172 | - err = -ENOENT; | |
| 173 | - rcu_read_lock(); | |
| 174 | - value = map->ops->map_lookup_elem(map, key); | |
| 172 | + err = -ENOMEM; | |
| 173 | + value = kmalloc(map->value_size, GFP_USER); | |
| 175 | 174 | if (!value) |
| 176 | - goto err_unlock; | |
| 175 | + goto free_key; | |
| 177 | 176 | |
| 177 | + rcu_read_lock(); | |
| 178 | + ptr = map->ops->map_lookup_elem(map, key); | |
| 179 | + if (ptr) | |
| 180 | + memcpy(value, ptr, map->value_size); | |
| 181 | + rcu_read_unlock(); | |
| 182 | + | |
| 183 | + err = -ENOENT; | |
| 184 | + if (!ptr) | |
| 185 | + goto free_value; | |
| 186 | + | |
| 178 | 187 | err = -EFAULT; |
| 179 | 188 | if (copy_to_user(uvalue, value, map->value_size) != 0) |
| 180 | - goto err_unlock; | |
| 189 | + goto free_value; | |
| 181 | 190 | |
| 182 | 191 | err = 0; |
| 183 | 192 | |
| 184 | -err_unlock: | |
| 185 | - rcu_read_unlock(); | |
| 193 | +free_value: | |
| 194 | + kfree(value); | |
| 186 | 195 | free_key: |
| 187 | 196 | kfree(key); |
| 188 | 197 | err_put: |