[PATCH] bpf: Fix cgroup link update after a failed effective array rebuild

[PATCH] bpf: Fix cgroup link update after a failed effective array rebuild

[Shihuang Liu]

replace_effective_prog() uses the position of a cgroup program list
entry as a physical index into the effective program array.  When an
OOM detach falls back to purge_effective_progs(), the detached entry is
replaced by a dummy but the other entries keep their position, so the
list position no longer matches the array index.  A later link update
then writes the new program into the wrong slot, leaving the old
program pointer in the array.  The array does not hold a reference, so
the old program can be freed while it is still published.

Translate the position to the corresponding non-dummy entry before
writing, and use the same helper for deletion and replacement.  This
covers both regular programs and cgroup struct_ops entries.

Fixes: 4c46091ee985 ("bpf: Fix KASAN use-after-free Read in compute_effective_progs")
Cc: stable@vger.kernel.org
Signed-off-by: Shihuang Liu <shlomojune6@gmail.com>
---
 kernel/bpf/cgroup.c | 42 +++++++++++++++++++++++++++++++-----------
 1 file changed, 31 insertions(+), 11 deletions(-)

diff --git a/kernel/bpf/cgroup.c b/kernel/bpf/cgroup.c
index 57a785241d42fb..db3f173e83c693 100644
--- a/kernel/bpf/cgroup.c
+++ b/kernel/bpf/cgroup.c
@@ -403,6 +403,28 @@ static int bpf_cgroup_array_length(struct bpf_prog_array *array,
 	return cnt;
 }
 
+/* Return the entry at logical position @index in an effective program
+ * array.  Dummy entries left behind by a failed rebuild are skipped because
+ * they do not correspond to an entry in the program list.
+ */
+static struct bpf_prog_array_item *
+bpf_cgroup_array_item_at(struct bpf_prog_array *array, int index,
+			 enum cgroup_bpf_attach_type atype)
+{
+	void *dummy = bpf_cgroup_array_dummy(atype);
+	struct bpf_prog_array_item *item;
+
+	for (item = array->items; item->prog; item++) {
+		if (item->prog == dummy)
+			continue;
+		if (!index)
+			return item;
+		index--;
+	}
+
+	return NULL;
+}
+
 static int bpf_cgroup_array_copy_to_user(struct bpf_prog_array *array,
 					 __u32 __user *prog_ids, int cnt,
 					 enum cgroup_bpf_attach_type atype)
@@ -432,16 +454,12 @@ static int bpf_cgroup_array_delete_safe_at(struct bpf_prog_array *array,
 {
 	struct bpf_prog_array_item *item;
 
-	for (item = array->items; item->prog; item++) {
-		if (item->prog == bpf_cgroup_array_dummy(atype))
-			continue;
-		if (!index) {
-			WRITE_ONCE(item->prog, bpf_cgroup_array_dummy(atype));
-			return 0;
-		}
-		index--;
-	}
-	return -ENOENT;
+	item = bpf_cgroup_array_item_at(array, index, atype);
+	if (!item)
+		return -ENOENT;
+
+	WRITE_ONCE(item->prog, bpf_cgroup_array_dummy(atype));
+	return 0;
 }
 
 /**
@@ -1175,7 +1193,9 @@ static void replace_effective_prog(struct cgroup *cgrp,
 		progs = rcu_dereference_protected(
 				desc->bpf.effective[atype],
 				lockdep_is_held(&cgroup_mutex));
-		item = &progs->items[pos];
+		item = bpf_cgroup_array_item_at(progs, pos, atype);
+		if (WARN_ON_ONCE(!item))
+			continue;
 		prog_list_replace_item(pl, item);
 	}
 }
-- 
2.43.0