FAILED: patch "[PATCH] bpf: Fix potential race in tail call compatibility check" failed to apply to 4.19-stable tree
Subsystems (3)
[email protected]
maintainerThe patch below does not apply to the 4.19-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.
thanks,
greg k-h
------------------ original commit in Linus's tree ------------------
From 54713c85f536048e685258f880bf298a74c3620d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Toke=20H=C3=B8iland-J=C3=B8rgensen?= <[email protected]>
Date: Tue, 26 Oct 2021 13:00:19 +0200
Subject: [PATCH] bpf: Fix potential race in tail call compatibility check
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
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)
include/linux/bpf.h +5 -2
@@ -929,8 +929,11 @@ struct bpf_array_aux {* stored in the map to make sure that all callers and callees have * the same prog type and JITed flag. */ -enum bpf_prog_type type; -bool jited; +struct { +spinlock_t lock; +enum bpf_prog_type type; +bool jited; + } owner;/* Programs with direct jumps into programs part of this array. */ struct list_head poke_progs;struct bpf_map *map;
kernel/bpf/arraymap.c +1 -0
@@ -1072,6 +1072,7 @@ static struct bpf_map *prog_array_map_alloc(union bpf_attr *attr)INIT_WORK (&aux->work, prog_array_map_clear_deferred);INIT_LIST_HEAD (&aux->poke_progs);mutex_init (&aux->poke_mutex); +spin_lock_init (&aux->owner.lock); map =array_map_alloc (attr);if (IS_ERR (map)) {
kernel/bpf/core.c +13 -7
@@ -1823,20 +1823,26 @@ static unsigned int __bpf_prog_ret0_warn(const void *ctx,bool bpf_prog_array_compatible (struct bpf_array *array,const struct bpf_prog *fp) { +bool ret; +if (fp->kprobe_override)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 * compatibility. */ - array->aux->type = fp->type; - array->aux->jited = fp->jited; -return true; + array->aux->owner.type = fp->type; + array->aux->owner.jited = fp->jited; + ret = true; + }else { + ret = array->aux->owner.type == fp->type && + array->aux->owner.jited == fp->jited; } - -return array->aux->type == fp->type && - array->aux->jited == fp->jited; +spin_unlock (&array->aux->owner.lock); +return ret; }static int bpf_check_tail_call (const struct bpf_prog *fp)
kernel/bpf/syscall.c +4 -2
@@ -543,8 +543,10 @@ static void bpf_map_show_fdinfo(struct seq_file *m, struct file *filp)if (map->map_type ==BPF_MAP_TYPE_PROG_ARRAY ) { array = container_of(map, struct bpf_array, map); - type = array->aux->type; - jited = array->aux->jited; +spin_lock (&array->aux->owner.lock); + type = array->aux->owner.type; + jited = array->aux->owner.jited; +spin_unlock (&array->aux->owner.lock); } seq_printf(m,