Skip to content

Commit 7a2fb91

Browse files
committed
Alexei Starovoitov says: ==================== pull-request: bpf 2022-02-17 We've added 8 non-merge commits during the last 7 day(s) which contain a total of 8 files changed, 119 insertions(+), 15 deletions(-). The main changes are: 1) Add schedule points in map batch ops, from Eric. 2) Fix bpf_msg_push_data with len 0, from Felix. 3) Fix crash due to incorrect copy_map_value, from Kumar. 4) Fix crash due to out of bounds access into reg2btf_ids, from Kumar. 5) Fix a bpf_timer initialization issue with clang, from Yonghong. * https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf: bpf: Add schedule points in batch ops bpf: Fix crash due to out of bounds access into reg2btf_ids. selftests: bpf: Check bpf_msg_push_data return value bpf: Fix a bpf_timer initialization issue bpf: Emit bpf_timer in vmlinux BTF selftests/bpf: Add test for bpf_timer overwriting crash bpf: Fix crash due to incorrect copy_map_value bpf: Do not try bpf_msg_push_data with len 0 ==================== Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Jakub Kicinski <[email protected]>
2 parents 8b97cae + 75134f1 commit 7a2fb91

File tree

8 files changed

+119
-15
lines changed

8 files changed

+119
-15
lines changed

include/linux/bpf.h

+4-5
Original file line numberDiff line numberDiff line change
@@ -209,11 +209,9 @@ static inline bool map_value_has_timer(const struct bpf_map *map)
209209
static inline void check_and_init_map_value(struct bpf_map *map, void *dst)
210210
{
211211
if (unlikely(map_value_has_spin_lock(map)))
212-
*(struct bpf_spin_lock *)(dst + map->spin_lock_off) =
213-
(struct bpf_spin_lock){};
212+
memset(dst + map->spin_lock_off, 0, sizeof(struct bpf_spin_lock));
214213
if (unlikely(map_value_has_timer(map)))
215-
*(struct bpf_timer *)(dst + map->timer_off) =
216-
(struct bpf_timer){};
214+
memset(dst + map->timer_off, 0, sizeof(struct bpf_timer));
217215
}
218216

219217
/* copy everything but bpf_spin_lock and bpf_timer. There could be one of each. */
@@ -224,7 +222,8 @@ static inline void copy_map_value(struct bpf_map *map, void *dst, void *src)
224222
if (unlikely(map_value_has_spin_lock(map))) {
225223
s_off = map->spin_lock_off;
226224
s_sz = sizeof(struct bpf_spin_lock);
227-
} else if (unlikely(map_value_has_timer(map))) {
225+
}
226+
if (unlikely(map_value_has_timer(map))) {
228227
t_off = map->timer_off;
229228
t_sz = sizeof(struct bpf_timer);
230229
}

kernel/bpf/btf.c

+3-2
Original file line numberDiff line numberDiff line change
@@ -5688,7 +5688,8 @@ static int btf_check_func_arg_match(struct bpf_verifier_env *env,
56885688
}
56895689
if (check_ptr_off_reg(env, reg, regno))
56905690
return -EINVAL;
5691-
} else if (is_kfunc && (reg->type == PTR_TO_BTF_ID || reg2btf_ids[reg->type])) {
5691+
} else if (is_kfunc && (reg->type == PTR_TO_BTF_ID ||
5692+
(reg2btf_ids[base_type(reg->type)] && !type_flag(reg->type)))) {
56925693
const struct btf_type *reg_ref_t;
56935694
const struct btf *reg_btf;
56945695
const char *reg_ref_tname;
@@ -5706,7 +5707,7 @@ static int btf_check_func_arg_match(struct bpf_verifier_env *env,
57065707
reg_ref_id = reg->btf_id;
57075708
} else {
57085709
reg_btf = btf_vmlinux;
5709-
reg_ref_id = *reg2btf_ids[reg->type];
5710+
reg_ref_id = *reg2btf_ids[base_type(reg->type)];
57105711
}
57115712

57125713
reg_ref_t = btf_type_skip_modifiers(reg_btf, reg_ref_id,

kernel/bpf/helpers.c

+2
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
/* Copyright (c) 2011-2014 PLUMgrid, http://plumgrid.com
33
*/
44
#include <linux/bpf.h>
5+
#include <linux/btf.h>
56
#include <linux/bpf-cgroup.h>
67
#include <linux/rcupdate.h>
78
#include <linux/random.h>
@@ -1075,6 +1076,7 @@ static enum hrtimer_restart bpf_timer_cb(struct hrtimer *hrtimer)
10751076
void *key;
10761077
u32 idx;
10771078

1079+
BTF_TYPE_EMIT(struct bpf_timer);
10781080
callback_fn = rcu_dereference_check(t->callback_fn, rcu_read_lock_bh_held());
10791081
if (!callback_fn)
10801082
goto out;

kernel/bpf/syscall.c

+3
Original file line numberDiff line numberDiff line change
@@ -1355,6 +1355,7 @@ int generic_map_delete_batch(struct bpf_map *map,
13551355
maybe_wait_bpf_programs(map);
13561356
if (err)
13571357
break;
1358+
cond_resched();
13581359
}
13591360
if (copy_to_user(&uattr->batch.count, &cp, sizeof(cp)))
13601361
err = -EFAULT;
@@ -1412,6 +1413,7 @@ int generic_map_update_batch(struct bpf_map *map,
14121413

14131414
if (err)
14141415
break;
1416+
cond_resched();
14151417
}
14161418

14171419
if (copy_to_user(&uattr->batch.count, &cp, sizeof(cp)))
@@ -1509,6 +1511,7 @@ int generic_map_lookup_batch(struct bpf_map *map,
15091511
swap(prev_key, key);
15101512
retry = MAP_LOOKUP_RETRIES;
15111513
cp++;
1514+
cond_resched();
15121515
}
15131516

15141517
if (err == -EFAULT)

net/core/filter.c

+3
Original file line numberDiff line numberDiff line change
@@ -2710,6 +2710,9 @@ BPF_CALL_4(bpf_msg_push_data, struct sk_msg *, msg, u32, start,
27102710
if (unlikely(flags))
27112711
return -EINVAL;
27122712

2713+
if (unlikely(len == 0))
2714+
return 0;
2715+
27132716
/* First find the starting scatterlist element */
27142717
i = msg->sg.start;
27152718
do {
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
// SPDX-License-Identifier: GPL-2.0
2+
#include <test_progs.h>
3+
#include "timer_crash.skel.h"
4+
5+
enum {
6+
MODE_ARRAY,
7+
MODE_HASH,
8+
};
9+
10+
static void test_timer_crash_mode(int mode)
11+
{
12+
struct timer_crash *skel;
13+
14+
skel = timer_crash__open_and_load();
15+
if (!ASSERT_OK_PTR(skel, "timer_crash__open_and_load"))
16+
return;
17+
skel->bss->pid = getpid();
18+
skel->bss->crash_map = mode;
19+
if (!ASSERT_OK(timer_crash__attach(skel), "timer_crash__attach"))
20+
goto end;
21+
usleep(1);
22+
end:
23+
timer_crash__destroy(skel);
24+
}
25+
26+
void test_timer_crash(void)
27+
{
28+
if (test__start_subtest("array"))
29+
test_timer_crash_mode(MODE_ARRAY);
30+
if (test__start_subtest("hash"))
31+
test_timer_crash_mode(MODE_HASH);
32+
}

tools/testing/selftests/bpf/progs/test_sockmap_kern.h

+18-8
Original file line numberDiff line numberDiff line change
@@ -235,7 +235,7 @@ SEC("sk_msg1")
235235
int bpf_prog4(struct sk_msg_md *msg)
236236
{
237237
int *bytes, zero = 0, one = 1, two = 2, three = 3, four = 4, five = 5;
238-
int *start, *end, *start_push, *end_push, *start_pop, *pop;
238+
int *start, *end, *start_push, *end_push, *start_pop, *pop, err = 0;
239239

240240
bytes = bpf_map_lookup_elem(&sock_apply_bytes, &zero);
241241
if (bytes)
@@ -249,8 +249,11 @@ int bpf_prog4(struct sk_msg_md *msg)
249249
bpf_msg_pull_data(msg, *start, *end, 0);
250250
start_push = bpf_map_lookup_elem(&sock_bytes, &two);
251251
end_push = bpf_map_lookup_elem(&sock_bytes, &three);
252-
if (start_push && end_push)
253-
bpf_msg_push_data(msg, *start_push, *end_push, 0);
252+
if (start_push && end_push) {
253+
err = bpf_msg_push_data(msg, *start_push, *end_push, 0);
254+
if (err)
255+
return SK_DROP;
256+
}
254257
start_pop = bpf_map_lookup_elem(&sock_bytes, &four);
255258
pop = bpf_map_lookup_elem(&sock_bytes, &five);
256259
if (start_pop && pop)
@@ -263,6 +266,7 @@ int bpf_prog6(struct sk_msg_md *msg)
263266
{
264267
int zero = 0, one = 1, two = 2, three = 3, four = 4, five = 5, key = 0;
265268
int *bytes, *start, *end, *start_push, *end_push, *start_pop, *pop, *f;
269+
int err = 0;
266270
__u64 flags = 0;
267271

268272
bytes = bpf_map_lookup_elem(&sock_apply_bytes, &zero);
@@ -279,8 +283,11 @@ int bpf_prog6(struct sk_msg_md *msg)
279283

280284
start_push = bpf_map_lookup_elem(&sock_bytes, &two);
281285
end_push = bpf_map_lookup_elem(&sock_bytes, &three);
282-
if (start_push && end_push)
283-
bpf_msg_push_data(msg, *start_push, *end_push, 0);
286+
if (start_push && end_push) {
287+
err = bpf_msg_push_data(msg, *start_push, *end_push, 0);
288+
if (err)
289+
return SK_DROP;
290+
}
284291

285292
start_pop = bpf_map_lookup_elem(&sock_bytes, &four);
286293
pop = bpf_map_lookup_elem(&sock_bytes, &five);
@@ -338,7 +345,7 @@ SEC("sk_msg5")
338345
int bpf_prog10(struct sk_msg_md *msg)
339346
{
340347
int *bytes, *start, *end, *start_push, *end_push, *start_pop, *pop;
341-
int zero = 0, one = 1, two = 2, three = 3, four = 4, five = 5;
348+
int zero = 0, one = 1, two = 2, three = 3, four = 4, five = 5, err = 0;
342349

343350
bytes = bpf_map_lookup_elem(&sock_apply_bytes, &zero);
344351
if (bytes)
@@ -352,8 +359,11 @@ int bpf_prog10(struct sk_msg_md *msg)
352359
bpf_msg_pull_data(msg, *start, *end, 0);
353360
start_push = bpf_map_lookup_elem(&sock_bytes, &two);
354361
end_push = bpf_map_lookup_elem(&sock_bytes, &three);
355-
if (start_push && end_push)
356-
bpf_msg_push_data(msg, *start_push, *end_push, 0);
362+
if (start_push && end_push) {
363+
err = bpf_msg_push_data(msg, *start_push, *end_push, 0);
364+
if (err)
365+
return SK_PASS;
366+
}
357367
start_pop = bpf_map_lookup_elem(&sock_bytes, &four);
358368
pop = bpf_map_lookup_elem(&sock_bytes, &five);
359369
if (start_pop && pop)
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
// SPDX-License-Identifier: GPL-2.0
2+
3+
#include <vmlinux.h>
4+
#include <bpf/bpf_tracing.h>
5+
#include <bpf/bpf_helpers.h>
6+
7+
struct map_elem {
8+
struct bpf_timer timer;
9+
struct bpf_spin_lock lock;
10+
};
11+
12+
struct {
13+
__uint(type, BPF_MAP_TYPE_ARRAY);
14+
__uint(max_entries, 1);
15+
__type(key, int);
16+
__type(value, struct map_elem);
17+
} amap SEC(".maps");
18+
19+
struct {
20+
__uint(type, BPF_MAP_TYPE_HASH);
21+
__uint(max_entries, 1);
22+
__type(key, int);
23+
__type(value, struct map_elem);
24+
} hmap SEC(".maps");
25+
26+
int pid = 0;
27+
int crash_map = 0; /* 0 for amap, 1 for hmap */
28+
29+
SEC("fentry/do_nanosleep")
30+
int sys_enter(void *ctx)
31+
{
32+
struct map_elem *e, value = {};
33+
void *map = crash_map ? (void *)&hmap : (void *)&amap;
34+
35+
if (bpf_get_current_task_btf()->tgid != pid)
36+
return 0;
37+
38+
*(void **)&value = (void *)0xdeadcaf3;
39+
40+
bpf_map_update_elem(map, &(int){0}, &value, 0);
41+
/* For array map, doing bpf_map_update_elem will do a
42+
* check_and_free_timer_in_array, which will trigger the crash if timer
43+
* pointer was overwritten, for hmap we need to use bpf_timer_cancel.
44+
*/
45+
if (crash_map == 1) {
46+
e = bpf_map_lookup_elem(map, &(int){0});
47+
if (!e)
48+
return 0;
49+
bpf_timer_cancel(&e->timer);
50+
}
51+
return 0;
52+
}
53+
54+
char _license[] SEC("license") = "GPL";

0 commit comments

Comments
 (0)