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. Additionally, handle the error path in __cgroup_bpf_attach strictly. Although it is rare for update_effective_progs to fail in this context, proper rollbacks for storage and flags are added for code rigor. Fixes: 0c991ebc8c69 ("bpf: Implement bpf_prog replacement for an active bpf_cgroup_link") Reviewed-by: Amery Hung Signed-off-by: Pu Lehui --- kernel/bpf/cgroup.c | 44 +++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 43 insertions(+), 1 deletion(-) diff --git a/kernel/bpf/cgroup.c b/kernel/bpf/cgroup.c index 4355ccb78a9c..56d538f05520 100644 --- a/kernel/bpf/cgroup.c +++ b/kernel/bpf/cgroup.c @@ -813,10 +813,12 @@ static int __cgroup_bpf_attach(struct cgroup *cgrp, struct bpf_prog *old_prog = NULL; struct bpf_cgroup_storage *storage[MAX_BPF_CGROUP_STORAGE_TYPE] = {}; struct bpf_cgroup_storage *new_storage[MAX_BPF_CGROUP_STORAGE_TYPE] = {}; + struct bpf_cgroup_storage *old_storage[MAX_BPF_CGROUP_STORAGE_TYPE] = {}; struct bpf_prog *new_prog = prog ? : link->link.prog; enum cgroup_bpf_attach_type atype; struct bpf_prog_list *pl; struct hlist_head *progs; + u8 old_flags; int err; if (((flags & BPF_F_ALLOW_OVERRIDE) && (flags & BPF_F_ALLOW_MULTI)) || @@ -883,7 +885,10 @@ static int __cgroup_bpf_attach(struct cgroup *cgrp, pl->prog = prog; pl->link = link; pl->flags = flags; + if (old_prog) + bpf_cgroup_storages_assign(old_storage, pl->storage); bpf_cgroup_storages_assign(pl->storage, storage); + old_flags = cgrp->bpf.flags[atype]; cgrp->bpf.flags[atype] = saved_flags; if (type == BPF_LSM_CGROUP) { @@ -915,12 +920,14 @@ static int __cgroup_bpf_attach(struct cgroup *cgrp, if (old_prog) { pl->prog = old_prog; pl->link = NULL; + bpf_cgroup_storages_assign(pl->storage, old_storage); } bpf_cgroup_storages_free(new_storage); if (!old_prog) { hlist_del(&pl->node); kfree(pl); } + cgrp->bpf.flags[atype] = old_flags; return err; } @@ -1032,11 +1039,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) @@ -1056,10 +1069,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