netdev - [PATCH v3] bpf: Fix KASAN use-after-free Read in compute_effective

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <20220513190821.431762-1-tadeusz.struk@linaro.org>
Date:   Fri, 13 May 2022 12:08:21 -0700
From:   Tadeusz Struk <tadeusz.struk@...aro.org>
To:     andrii.nakryiko@...il.com
Cc:     Tadeusz Struk <tadeusz.struk@...aro.org>,
        "Alexei Starovoitov" <ast@...nel.org>,
        "Daniel Borkmann" <daniel@...earbox.net>,
        "Andrii Nakryiko" <andrii@...nel.org>,
        "Martin KaFai Lau" <kafai@...com>,
        "Song Liu" <songliubraving@...com>, "Yonghong Song" <yhs@...com>,
        "John Fastabend" <john.fastabend@...il.com>,
        "KP Singh" <kpsingh@...nel.org>, netdev@...r.kernel.org,
        bpf@...r.kernel.org, stable@...r.kernel.org,
        linux-kernel@...r.kernel.org,
        syzbot+f264bffdfbd5614f3bb2@...kaller.appspotmail.com
Subject: [PATCH v3] bpf: Fix KASAN use-after-free Read in compute_effective_progs

Syzbot found a Use After Free bug in compute_effective_progs().
The reproducer creates a number of BPF links, and causes a fault
injected alloc to fail, while calling bpf_link_detach on them.
Link detach triggers the link to be freed by bpf_link_free(),
which calls __cgroup_bpf_detach() and update_effective_progs().
If the memory allocation in this function fails, the function restores
the pointer to the bpf_cgroup_link on the cgroup list, but the memory
gets freed just after it returns. After this, every subsequent call to
update_effective_progs() causes this already deallocated pointer to be
dereferenced in prog_list_length(), and triggers KASAN UAF error.

To fix this issue don't preserve the pointer to the prog or link in the
list, but remove it and replace it with a dummy prog without shrinking
the table. The subsequent call to __cgroup_bpf_detach() or
__cgroup_bpf_detach() will correct it.

Cc: "Alexei Starovoitov" <ast@...nel.org>
Cc: "Daniel Borkmann" <daniel@...earbox.net>
Cc: "Andrii Nakryiko" <andrii@...nel.org>
Cc: "Martin KaFai Lau" <kafai@...com>
Cc: "Song Liu" <songliubraving@...com>
Cc: "Yonghong Song" <yhs@...com>
Cc: "John Fastabend" <john.fastabend@...il.com>
Cc: "KP Singh" <kpsingh@...nel.org>
Cc: <netdev@...r.kernel.org>
Cc: <bpf@...r.kernel.org>
Cc: <stable@...r.kernel.org>
Cc: <linux-kernel@...r.kernel.org>

Link: https://syzkaller.appspot.com/bug?id=8ebf179a95c2a2670f7cf1ba62429ec044369db4
Fixes: af6eea57437a ("bpf: Implement bpf_link-based cgroup BPF program attachment")
Reported-by: <syzbot+f264bffdfbd5614f3bb2@...kaller.appspotmail.com>
Signed-off-by: Tadeusz Struk <tadeusz.struk@...aro.org>
---
v2: Add a fall back path that removes a prog from the effective progs
    table in case detach fails to allocate memory in compute_effective_progs().

v3: Implement the fallback in a separate function purge_effective_progs
---
 kernel/bpf/cgroup.c | 64 +++++++++++++++++++++++++++++++++++++++------
 1 file changed, 56 insertions(+), 8 deletions(-)

diff --git a/kernel/bpf/cgroup.c b/kernel/bpf/cgroup.c
index 128028efda64..9d3af4d6c055 100644
--- a/kernel/bpf/cgroup.c
+++ b/kernel/bpf/cgroup.c
@@ -681,6 +681,57 @@ static struct bpf_prog_list *find_detach_entry(struct list_head *progs,
 	return ERR_PTR(-ENOENT);
 }
 
+/**
+ * purge_effective_progs() - After compute_effective_progs fails to alloc new
+ *                           cgrp->bpf.inactive table we can recover by
+ *                           recomputing the array in place.
+ *
+ * @cgrp: The cgroup which descendants to traverse
+ * @link: A link to detach
+ * @atype: Type of detach operation
+ */
+static void purge_effective_progs(struct cgroup *cgrp, struct bpf_prog *prog,
+				  enum cgroup_bpf_attach_type atype)
+{
+	struct cgroup_subsys_state *css;
+	struct bpf_prog_array_item *item;
+	struct bpf_prog *tmp;
+	struct bpf_prog_array *array;
+	int index = 0, index_purge = -1;
+
+	if (!prog)
+		return;
+
+	/* recompute effective prog array in place */
+	css_for_each_descendant_pre(css, &cgrp->self) {
+		struct cgroup *desc = container_of(css, struct cgroup, self);
+
+		array = desc->bpf.effective[atype];
+		item = &array->items[0];
+
+		/* Find the index of the prog to purge */
+		while ((tmp = READ_ONCE(item->prog))) {
+			if (tmp == prog) {
+				index_purge = index;
+				break;
+			}
+			item++;
+			index++;
+		}
+
+		/* Check if we found what's needed for removing the prog */
+		if (index_purge == -1 || index_purge == index - 1)
+			continue;
+
+		/* Remove the program from the array */
+		WARN_ONCE(bpf_prog_array_delete_safe_at(array, index_purge),
+			  "Failed to purge a prog from array at index %d", index_purge);
+
+		index = 0;
+		index_purge = -1;
+	}
+}
+
 /**
  * __cgroup_bpf_detach() - Detach the program or link from a cgroup, and
  *                         propagate the change to descendants
@@ -723,8 +774,11 @@ static int __cgroup_bpf_detach(struct cgroup *cgrp, struct bpf_prog *prog,
 	pl->link = NULL;
 
 	err = update_effective_progs(cgrp, atype);
-	if (err)
-		goto cleanup;
+	if (err) {
+		struct bpf_prog *prog_purge = prog ? prog : link->link.prog;
+
+		purge_effective_progs(cgrp, prog_purge, atype);
+	}
 
 	/* now can actually delete it from this cgroup list */
 	list_del(&pl->node);
@@ -736,12 +790,6 @@ static int __cgroup_bpf_detach(struct cgroup *cgrp, struct bpf_prog *prog,
 		bpf_prog_put(old_prog);
 	static_branch_dec(&cgroup_bpf_enabled_key[atype]);
 	return 0;
-
-cleanup:
-	/* restore back prog or link */
-	pl->prog = old_prog;
-	pl->link = link;
-	return err;
 }
 
 static int cgroup_bpf_detach(struct cgroup *cgrp, struct bpf_prog *prog,
-- 
2.36.1