Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
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/20220217190000.37925-1-alexei.starovoitov@gmail.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
  • Loading branch information
kuba-moo committed Feb 17, 2022
2 parents 8b97cae + 75134f1 commit 7a2fb91
Show file tree
Hide file tree
Showing 8 changed files with 119 additions and 15 deletions.
9 changes: 4 additions & 5 deletions include/linux/bpf.h
Original file line number Diff line number Diff line change
Expand Up @@ -209,11 +209,9 @@ static inline bool map_value_has_timer(const struct bpf_map *map)
static inline void check_and_init_map_value(struct bpf_map *map, void *dst)
{
if (unlikely(map_value_has_spin_lock(map)))
*(struct bpf_spin_lock *)(dst + map->spin_lock_off) =
(struct bpf_spin_lock){};
memset(dst + map->spin_lock_off, 0, sizeof(struct bpf_spin_lock));
if (unlikely(map_value_has_timer(map)))
*(struct bpf_timer *)(dst + map->timer_off) =
(struct bpf_timer){};
memset(dst + map->timer_off, 0, sizeof(struct bpf_timer));
}

/* copy everything but bpf_spin_lock and bpf_timer. There could be one of each. */
Expand All @@ -224,7 +222,8 @@ static inline void copy_map_value(struct bpf_map *map, void *dst, void *src)
if (unlikely(map_value_has_spin_lock(map))) {
s_off = map->spin_lock_off;
s_sz = sizeof(struct bpf_spin_lock);
} else if (unlikely(map_value_has_timer(map))) {
}
if (unlikely(map_value_has_timer(map))) {
t_off = map->timer_off;
t_sz = sizeof(struct bpf_timer);
}
Expand Down
5 changes: 3 additions & 2 deletions kernel/bpf/btf.c
Original file line number Diff line number Diff line change
Expand Up @@ -5688,7 +5688,8 @@ static int btf_check_func_arg_match(struct bpf_verifier_env *env,
}
if (check_ptr_off_reg(env, reg, regno))
return -EINVAL;
} else if (is_kfunc && (reg->type == PTR_TO_BTF_ID || reg2btf_ids[reg->type])) {
} else if (is_kfunc && (reg->type == PTR_TO_BTF_ID ||
(reg2btf_ids[base_type(reg->type)] && !type_flag(reg->type)))) {
const struct btf_type *reg_ref_t;
const struct btf *reg_btf;
const char *reg_ref_tname;
Expand All @@ -5706,7 +5707,7 @@ static int btf_check_func_arg_match(struct bpf_verifier_env *env,
reg_ref_id = reg->btf_id;
} else {
reg_btf = btf_vmlinux;
reg_ref_id = *reg2btf_ids[reg->type];
reg_ref_id = *reg2btf_ids[base_type(reg->type)];
}

reg_ref_t = btf_type_skip_modifiers(reg_btf, reg_ref_id,
Expand Down
2 changes: 2 additions & 0 deletions kernel/bpf/helpers.c
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
/* Copyright (c) 2011-2014 PLUMgrid, http://plumgrid.com
*/
#include <linux/bpf.h>
#include <linux/btf.h>
#include <linux/bpf-cgroup.h>
#include <linux/rcupdate.h>
#include <linux/random.h>
Expand Down Expand Up @@ -1075,6 +1076,7 @@ static enum hrtimer_restart bpf_timer_cb(struct hrtimer *hrtimer)
void *key;
u32 idx;

BTF_TYPE_EMIT(struct bpf_timer);
callback_fn = rcu_dereference_check(t->callback_fn, rcu_read_lock_bh_held());
if (!callback_fn)
goto out;
Expand Down
3 changes: 3 additions & 0 deletions kernel/bpf/syscall.c
Original file line number Diff line number Diff line change
Expand Up @@ -1355,6 +1355,7 @@ int generic_map_delete_batch(struct bpf_map *map,
maybe_wait_bpf_programs(map);
if (err)
break;
cond_resched();
}
if (copy_to_user(&uattr->batch.count, &cp, sizeof(cp)))
err = -EFAULT;
Expand Down Expand Up @@ -1412,6 +1413,7 @@ int generic_map_update_batch(struct bpf_map *map,

if (err)
break;
cond_resched();
}

if (copy_to_user(&uattr->batch.count, &cp, sizeof(cp)))
Expand Down Expand Up @@ -1509,6 +1511,7 @@ int generic_map_lookup_batch(struct bpf_map *map,
swap(prev_key, key);
retry = MAP_LOOKUP_RETRIES;
cp++;
cond_resched();
}

if (err == -EFAULT)
Expand Down
3 changes: 3 additions & 0 deletions net/core/filter.c
Original file line number Diff line number Diff line change
Expand Up @@ -2710,6 +2710,9 @@ BPF_CALL_4(bpf_msg_push_data, struct sk_msg *, msg, u32, start,
if (unlikely(flags))
return -EINVAL;

if (unlikely(len == 0))
return 0;

/* First find the starting scatterlist element */
i = msg->sg.start;
do {
Expand Down
32 changes: 32 additions & 0 deletions tools/testing/selftests/bpf/prog_tests/timer_crash.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
// SPDX-License-Identifier: GPL-2.0
#include <test_progs.h>
#include "timer_crash.skel.h"

enum {
MODE_ARRAY,
MODE_HASH,
};

static void test_timer_crash_mode(int mode)
{
struct timer_crash *skel;

skel = timer_crash__open_and_load();
if (!ASSERT_OK_PTR(skel, "timer_crash__open_and_load"))
return;
skel->bss->pid = getpid();
skel->bss->crash_map = mode;
if (!ASSERT_OK(timer_crash__attach(skel), "timer_crash__attach"))
goto end;
usleep(1);
end:
timer_crash__destroy(skel);
}

void test_timer_crash(void)
{
if (test__start_subtest("array"))
test_timer_crash_mode(MODE_ARRAY);
if (test__start_subtest("hash"))
test_timer_crash_mode(MODE_HASH);
}
26 changes: 18 additions & 8 deletions tools/testing/selftests/bpf/progs/test_sockmap_kern.h
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,7 @@ SEC("sk_msg1")
int bpf_prog4(struct sk_msg_md *msg)
{
int *bytes, zero = 0, one = 1, two = 2, three = 3, four = 4, five = 5;
int *start, *end, *start_push, *end_push, *start_pop, *pop;
int *start, *end, *start_push, *end_push, *start_pop, *pop, err = 0;

bytes = bpf_map_lookup_elem(&sock_apply_bytes, &zero);
if (bytes)
Expand All @@ -249,8 +249,11 @@ int bpf_prog4(struct sk_msg_md *msg)
bpf_msg_pull_data(msg, *start, *end, 0);
start_push = bpf_map_lookup_elem(&sock_bytes, &two);
end_push = bpf_map_lookup_elem(&sock_bytes, &three);
if (start_push && end_push)
bpf_msg_push_data(msg, *start_push, *end_push, 0);
if (start_push && end_push) {
err = bpf_msg_push_data(msg, *start_push, *end_push, 0);
if (err)
return SK_DROP;
}
start_pop = bpf_map_lookup_elem(&sock_bytes, &four);
pop = bpf_map_lookup_elem(&sock_bytes, &five);
if (start_pop && pop)
Expand All @@ -263,6 +266,7 @@ int bpf_prog6(struct sk_msg_md *msg)
{
int zero = 0, one = 1, two = 2, three = 3, four = 4, five = 5, key = 0;
int *bytes, *start, *end, *start_push, *end_push, *start_pop, *pop, *f;
int err = 0;
__u64 flags = 0;

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

start_push = bpf_map_lookup_elem(&sock_bytes, &two);
end_push = bpf_map_lookup_elem(&sock_bytes, &three);
if (start_push && end_push)
bpf_msg_push_data(msg, *start_push, *end_push, 0);
if (start_push && end_push) {
err = bpf_msg_push_data(msg, *start_push, *end_push, 0);
if (err)
return SK_DROP;
}

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

bytes = bpf_map_lookup_elem(&sock_apply_bytes, &zero);
if (bytes)
Expand All @@ -352,8 +359,11 @@ int bpf_prog10(struct sk_msg_md *msg)
bpf_msg_pull_data(msg, *start, *end, 0);
start_push = bpf_map_lookup_elem(&sock_bytes, &two);
end_push = bpf_map_lookup_elem(&sock_bytes, &three);
if (start_push && end_push)
bpf_msg_push_data(msg, *start_push, *end_push, 0);
if (start_push && end_push) {
err = bpf_msg_push_data(msg, *start_push, *end_push, 0);
if (err)
return SK_PASS;
}
start_pop = bpf_map_lookup_elem(&sock_bytes, &four);
pop = bpf_map_lookup_elem(&sock_bytes, &five);
if (start_pop && pop)
Expand Down
54 changes: 54 additions & 0 deletions tools/testing/selftests/bpf/progs/timer_crash.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
// SPDX-License-Identifier: GPL-2.0

#include <vmlinux.h>
#include <bpf/bpf_tracing.h>
#include <bpf/bpf_helpers.h>

struct map_elem {
struct bpf_timer timer;
struct bpf_spin_lock lock;
};

struct {
__uint(type, BPF_MAP_TYPE_ARRAY);
__uint(max_entries, 1);
__type(key, int);
__type(value, struct map_elem);
} amap SEC(".maps");

struct {
__uint(type, BPF_MAP_TYPE_HASH);
__uint(max_entries, 1);
__type(key, int);
__type(value, struct map_elem);
} hmap SEC(".maps");

int pid = 0;
int crash_map = 0; /* 0 for amap, 1 for hmap */

SEC("fentry/do_nanosleep")
int sys_enter(void *ctx)
{
struct map_elem *e, value = {};
void *map = crash_map ? (void *)&hmap : (void *)&amap;

if (bpf_get_current_task_btf()->tgid != pid)
return 0;

*(void **)&value = (void *)0xdeadcaf3;

bpf_map_update_elem(map, &(int){0}, &value, 0);
/* For array map, doing bpf_map_update_elem will do a
* check_and_free_timer_in_array, which will trigger the crash if timer
* pointer was overwritten, for hmap we need to use bpf_timer_cancel.
*/
if (crash_map == 1) {
e = bpf_map_lookup_elem(map, &(int){0});
if (!e)
return 0;
bpf_timer_cancel(&e->timer);
}
return 0;
}

char _license[] SEC("license") = "GPL";

0 comments on commit 7a2fb91

Please sign in to comment.