提交 349aeec5 编写于 作者: T Toke Høiland-Jørgensen 提交者: Zheng Zengkai

bpf: Fix potential race in tail call compatibility check

stable inclusion
from stable-5.10.77
commit dd2260ec643d309d8c58ceac3aa77f80db765488
bugzilla: 185677 https://gitee.com/openeuler/kernel/issues/I4IAP7

Reference: https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?id=dd2260ec643d309d8c58ceac3aa77f80db765488

--------------------------------

commit 54713c85 upstream.

Lorenzo noticed that the code testing for program type compatibility of
tail call maps is potentially racy in that two threads could encounter a
map with an unset type simultaneously and both return true even though they
are inserting incompatible programs.

The race window is quite small, but artificially enlarging it by adding a
usleep_range() inside the check in bpf_prog_array_compatible() makes it
trivial to trigger from userspace with a program that does, essentially:

        map_fd = bpf_create_map(BPF_MAP_TYPE_PROG_ARRAY, 4, 4, 2, 0);
        pid = fork();
        if (pid) {
                key = 0;
                value = xdp_fd;
        } else {
                key = 1;
                value = tc_fd;
        }
        err = bpf_map_update_elem(map_fd, &key, &value, 0);

While the race window is small, it has potentially serious ramifications in
that triggering it would allow a BPF program to tail call to a program of a
different type. So let's get rid of it by protecting the update with a
spinlock. The commit in the Fixes tag is the last commit that touches the
code in question.

v2:
- Use a spinlock instead of an atomic variable and cmpxchg() (Alexei)
v3:
- Put lock and the members it protects into an embedded 'owner' struct (Daniel)

Fixes: 3324b584 ("ebpf: misc core cleanup")
Reported-by: NLorenzo Bianconi <lorenzo.bianconi@redhat.com>
Signed-off-by: NToke Høiland-Jørgensen <toke@redhat.com>
Signed-off-by: NAlexei Starovoitov <ast@kernel.org>
Link: https://lore.kernel.org/bpf/20211026110019.363464-1-toke@redhat.comSigned-off-by: NGreg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: NChen Jun <chenjun102@huawei.com>
Acked-by: NWeilong Chen <chenweilong@huawei.com>
Signed-off-by: NChen Jun <chenjun102@huawei.com>
Signed-off-by: NZheng Zengkai <zhengzengkai@huawei.com>
上级 4338600a
...@@ -862,8 +862,11 @@ struct bpf_array_aux { ...@@ -862,8 +862,11 @@ struct bpf_array_aux {
* stored in the map to make sure that all callers and callees have * stored in the map to make sure that all callers and callees have
* the same prog type and JITed flag. * the same prog type and JITed flag.
*/ */
enum bpf_prog_type type; struct {
bool jited; spinlock_t lock;
enum bpf_prog_type type;
bool jited;
} owner;
/* Programs with direct jumps into programs part of this array. */ /* Programs with direct jumps into programs part of this array. */
struct list_head poke_progs; struct list_head poke_progs;
struct bpf_map *map; struct bpf_map *map;
......
...@@ -1025,6 +1025,7 @@ static struct bpf_map *prog_array_map_alloc(union bpf_attr *attr) ...@@ -1025,6 +1025,7 @@ static struct bpf_map *prog_array_map_alloc(union bpf_attr *attr)
INIT_WORK(&aux->work, prog_array_map_clear_deferred); INIT_WORK(&aux->work, prog_array_map_clear_deferred);
INIT_LIST_HEAD(&aux->poke_progs); INIT_LIST_HEAD(&aux->poke_progs);
mutex_init(&aux->poke_mutex); mutex_init(&aux->poke_mutex);
spin_lock_init(&aux->owner.lock);
map = array_map_alloc(attr); map = array_map_alloc(attr);
if (IS_ERR(map)) { if (IS_ERR(map)) {
......
...@@ -1775,20 +1775,26 @@ static unsigned int __bpf_prog_ret0_warn(const void *ctx, ...@@ -1775,20 +1775,26 @@ static unsigned int __bpf_prog_ret0_warn(const void *ctx,
bool bpf_prog_array_compatible(struct bpf_array *array, bool bpf_prog_array_compatible(struct bpf_array *array,
const struct bpf_prog *fp) const struct bpf_prog *fp)
{ {
bool ret;
if (fp->kprobe_override) if (fp->kprobe_override)
return false; return false;
if (!array->aux->type) { spin_lock(&array->aux->owner.lock);
if (!array->aux->owner.type) {
/* There's no owner yet where we could check for /* There's no owner yet where we could check for
* compatibility. * compatibility.
*/ */
array->aux->type = fp->type; array->aux->owner.type = fp->type;
array->aux->jited = fp->jited; array->aux->owner.jited = fp->jited;
return true; ret = true;
} else {
ret = array->aux->owner.type == fp->type &&
array->aux->owner.jited == fp->jited;
} }
spin_unlock(&array->aux->owner.lock);
return array->aux->type == fp->type && return ret;
array->aux->jited == fp->jited;
} }
static int bpf_check_tail_call(const struct bpf_prog *fp) static int bpf_check_tail_call(const struct bpf_prog *fp)
......
...@@ -535,8 +535,10 @@ static void bpf_map_show_fdinfo(struct seq_file *m, struct file *filp) ...@@ -535,8 +535,10 @@ static void bpf_map_show_fdinfo(struct seq_file *m, struct file *filp)
if (map->map_type == BPF_MAP_TYPE_PROG_ARRAY) { if (map->map_type == BPF_MAP_TYPE_PROG_ARRAY) {
array = container_of(map, struct bpf_array, map); array = container_of(map, struct bpf_array, map);
type = array->aux->type; spin_lock(&array->aux->owner.lock);
jited = array->aux->jited; type = array->aux->owner.type;
jited = array->aux->owner.jited;
spin_unlock(&array->aux->owner.lock);
} }
seq_printf(m, seq_printf(m,
......
Markdown is supported
0% .
You are about to add 0 people to the discussion. Proceed with caution.
先完成此消息的编辑!
想要评论请 注册