From: Pu Lehui Syzkaller reported a storage null-ptr-deref issue after replacing prog. This occurs in the following scenario: 1. prog A, an empty prog, is attached to a cgrp. 2. prog B uses BPF_MAP_TYPE_PERCPU_CGROUP_STORAGE and calls the bpf_get_local_storage helper. 3. link_update is called to replace prog A with prog B. The reason is that __cgroup_bpf_replace fails to alloc and assign the required cgrp storage for the incoming replacement prog. Consequently, the new prog inherits an uninit storage, leading to null-ptr-deref panic when kick the new prog. Fix this by properly allocating the storage and comparing the old and new storage pointers. If the storage changed, fallback to update_effective_progs which performs a RCU-safe update of the entire array. If the storage remains unchanged, we can safely retain the fast-path in-place update. Fixes: 0c991ebc8c69 ("bpf: Implement bpf_prog replacement for an active bpf_cgroup_link") Signed-off-by: Pu Lehui --- kernel/bpf/cgroup.c | 37 ++++++++++++++++++++++++++++++++++++- 1 file changed, 36 insertions(+), 1 deletion(-) diff --git a/kernel/bpf/cgroup.c b/kernel/bpf/cgroup.c index 70757ae45934..dad2d6a1d96d 100644 --- a/kernel/bpf/cgroup.c +++ b/kernel/bpf/cgroup.c @@ -1035,11 +1035,17 @@ static int __cgroup_bpf_replace(struct cgroup *cgrp, struct bpf_cgroup_link *link, struct bpf_prog *new_prog) { + struct bpf_cgroup_storage *new_storage[MAX_BPF_CGROUP_STORAGE_TYPE] = {}; + struct bpf_cgroup_storage *old_storage[MAX_BPF_CGROUP_STORAGE_TYPE] = {}; + struct bpf_cgroup_storage *storage[MAX_BPF_CGROUP_STORAGE_TYPE] = {}; + enum bpf_cgroup_storage_type stype; enum cgroup_bpf_attach_type atype; + bool storage_changed = false; struct bpf_prog *old_prog; struct bpf_prog_list *pl; struct hlist_head *progs; bool found = false; + int err; atype = bpf_cgroup_atype_find(link->link.attach_type, new_prog->aux->attach_btf_id); if (atype < 0) @@ -1059,10 +1065,39 @@ static int __cgroup_bpf_replace(struct cgroup *cgrp, if (!found) return -ENOENT; + if (bpf_cgroup_storages_alloc(storage, new_storage, link->link.attach_type, + new_prog, cgrp)) + return -ENOMEM; + + for_each_cgroup_storage_type(stype) { + if (storage[stype] != pl->storage[stype]) { + storage_changed = true; + break; + } + } + cgrp->bpf.revisions[atype] += 1; old_prog = xchg(&link->link.prog, new_prog); - replace_effective_prog(cgrp, atype, pl); + + if (!storage_changed) { + replace_effective_prog(cgrp, atype, pl); + bpf_prog_put(old_prog); + return 0; + } + + bpf_cgroup_storages_assign(old_storage, pl->storage); + bpf_cgroup_storages_assign(pl->storage, storage); + err = update_effective_progs(cgrp, atype); + if (err) { + xchg(&link->link.prog, old_prog); + bpf_cgroup_storages_assign(pl->storage, old_storage); + bpf_cgroup_storages_free(new_storage); + cgrp->bpf.revisions[atype] -= 1; + return err; + } + bpf_prog_put(old_prog); + bpf_cgroup_storages_link(new_storage, cgrp, link->link.attach_type); return 0; } -- 2.34.1