[PATCH bpf v6] bpf, cgroup: Fix storage null-ptr-deref after replacing prog

Pu Lehui <[email protected]>
Newsgroups org.kernel.vger.bpf,org.kernel.vger.linux-kernel
Message-ID <[email protected]>
From: Pu Lehui <[email protected]>

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 rejecting a link update if new_prog's cgroup storage is
incompatible with link->prog.

Fixes: 0c991ebc8c69 ("bpf: Implement bpf_prog replacement for an active bpf_cgroup_link")
Reviewed-by: Amery Hung <[email protected]>
Acked-by: Leon Hwang <[email protected]>
Signed-off-by: Pu Lehui <[email protected]>
---
v6:
- Update inconsistent commit msg.
- Add Acked-by tag by Leon.

v5: https://lore.kernel.org/bpf/[email protected]
- Reject link update if new_prog's storage is incompatible with link->prog. (Andrii)

v4: https://lore.kernel.org/bpf/[email protected]
- Extract the fix for __cgroup_bpf_attach() into a standalone patch [0].
- Add Reviewed-by tag by Amery.
- Separate from patchset [1]. (Andrii)

Link: https://lore.kernel.org/bpf/[email protected] [0]
Link: https://lore.kernel.org/bpf/[email protected] [1]

v3: https://lore.kernel.org/bpf/[email protected]
- Include the storage and flags rollbacks to patch4 for sake of code rigor,
  as it's hard to make update_effective_progs fail in __cgroup_bpf_attach.

v2: https://lore.kernel.org/bpf/[email protected]
- Fix invalid access for in-place update when storage changed. (Sashiko)

v1: https://lore.kernel.org/bpf/[email protected]

 kernel/bpf/cgroup.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/kernel/bpf/cgroup.c b/kernel/bpf/cgroup.c
index 57fd1d784e44..31c63ff276fa 100644
--- a/kernel/bpf/cgroup.c
+++ b/kernel/bpf/cgroup.c
@@ -1026,6 +1026,20 @@ static void replace_effective_prog(struct cgroup *cgrp,
 	}
 }
 
+static bool cgroup_bpf_storages_compatible(struct bpf_prog *old_prog,
+					   struct bpf_prog *new_prog)
+{
+	enum bpf_cgroup_storage_type stype;
+
+	for_each_cgroup_storage_type(stype) {
+		if (old_prog->aux->cgroup_storage[stype] !=
+		    new_prog->aux->cgroup_storage[stype])
+			return false;
+	}
+
+	return true;
+}
+
 /**
  * __cgroup_bpf_replace() - Replace link's program and propagate the change
  *                          to descendants
@@ -1064,6 +1078,9 @@ static int __cgroup_bpf_replace(struct cgroup *cgrp,
 	if (!found)
 		return -ENOENT;
 
+	if (!cgroup_bpf_storages_compatible(link->link.prog, new_prog))
+		return -EINVAL;
+
 	cgrp->bpf.revisions[atype] += 1;
 	old_prog = xchg(&link->link.prog, new_prog);
 	replace_effective_prog(cgrp, atype, pl);
-- 
2.34.1
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.