From e0cbee484435ecddeeebd1068ce5a0e10d6bfebd Mon Sep 17 00:00:00 2001 From: Chen Qun Date: Tue, 8 Jun 2021 09:07:17 +0800 Subject: [PATCH 1/3] 9pfs: Fully restart unreclaim loop (CVE-2021-20181) Fix CVE-2021-20181 Depending on the client activity, the server can be asked to open a huge number of file descriptors and eventually hit RLIMIT_NOFILE. This is currently mitigated using a reclaim logic : the server closes the file descriptors of idle fids, based on the assumption that it will be able to re-open them later. This assumption doesn't hold of course if the client requests the file to be unlinked. In this case, we loop on the entire fid list and mark all related fids as unreclaimable (the reclaim logic will just ignore them) and, of course, we open or re-open their file descriptors if needed since we're about to unlink the file. This is the purpose of v9fs_mark_fids_unreclaim(). Since the actual opening of a file can cause the coroutine to yield, another client request could possibly add a new fid that we may want to mark as non-reclaimable as well. The loop is thus restarted if the re-open request was actually transmitted to the backend. This is achieved by keeping a reference on the first fid (head) before traversing the list. This is wrong in several ways: - a potential clunk request from the client could tear the first fid down and cause the reference to be stale. This leads to a use-after-free error that can be detected with ASAN, using a custom 9p client - fids are added at the head of the list : restarting from the previous head will always miss fids added by a some other potential request All these problems could be avoided if fids were being added at the end of the list. This can be achieved with a QSIMPLEQ, but this is probably too much change for a bug fix. For now let's keep it simple and just restart the loop from the current head. Fixes: CVE-2021-20181 Buglink: https://bugs.launchpad.net/qemu/+bug/1911666 Reported-by: Zero Day Initiative Reviewed-by: Christian Schoenebeck Reviewed-by: Stefano Stabellini Message-Id: <161064025265.1838153.15185571283519390907.stgit@bahia.lan> Signed-off-by: Greg Kurz Signed-off-by: Jiajie Li --- ...estart-unreclaim-loop-CVE-2021-20181.patch | 80 +++++++++++++++++++ 1 file changed, 80 insertions(+) create mode 100644 9pfs-Fully-restart-unreclaim-loop-CVE-2021-20181.patch diff --git a/9pfs-Fully-restart-unreclaim-loop-CVE-2021-20181.patch b/9pfs-Fully-restart-unreclaim-loop-CVE-2021-20181.patch new file mode 100644 index 0000000..59f695a --- /dev/null +++ b/9pfs-Fully-restart-unreclaim-loop-CVE-2021-20181.patch @@ -0,0 +1,80 @@ +From 31012acf8fcf51131d1553bc3e04176ae51e8228 Mon Sep 17 00:00:00 2001 +From: Greg Kurz +Date: Tue, 8 Jun 2021 09:07:17 +0800 +Subject: [PATCH] 9pfs: Fully restart unreclaim loop (CVE-2021-20181) + +Fix CVE-2021-20181 + +Depending on the client activity, the server can be asked to open a huge +number of file descriptors and eventually hit RLIMIT_NOFILE. This is +currently mitigated using a reclaim logic : the server closes the file +descriptors of idle fids, based on the assumption that it will be able +to re-open them later. This assumption doesn't hold of course if the +client requests the file to be unlinked. In this case, we loop on the +entire fid list and mark all related fids as unreclaimable (the reclaim +logic will just ignore them) and, of course, we open or re-open their +file descriptors if needed since we're about to unlink the file. + +This is the purpose of v9fs_mark_fids_unreclaim(). Since the actual +opening of a file can cause the coroutine to yield, another client +request could possibly add a new fid that we may want to mark as +non-reclaimable as well. The loop is thus restarted if the re-open +request was actually transmitted to the backend. This is achieved +by keeping a reference on the first fid (head) before traversing +the list. + +This is wrong in several ways: +- a potential clunk request from the client could tear the first + fid down and cause the reference to be stale. This leads to a + use-after-free error that can be detected with ASAN, using a + custom 9p client +- fids are added at the head of the list : restarting from the + previous head will always miss fids added by a some other + potential request + +All these problems could be avoided if fids were being added at the +end of the list. This can be achieved with a QSIMPLEQ, but this is +probably too much change for a bug fix. For now let's keep it +simple and just restart the loop from the current head. + +Fixes: CVE-2021-20181 +Buglink: https://bugs.launchpad.net/qemu/+bug/1911666 +Reported-by: Zero Day Initiative +Reviewed-by: Christian Schoenebeck +Reviewed-by: Stefano Stabellini +Message-Id: <161064025265.1838153.15185571283519390907.stgit@bahia.lan> +Signed-off-by: Greg Kurz + +Signed-off-by: Jiajie Li +--- + hw/9pfs/9p.c | 6 +++--- + 1 file changed, 3 insertions(+), 3 deletions(-) + +diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c +index 55821343e5..289d00b01a 100644 +--- a/hw/9pfs/9p.c ++++ b/hw/9pfs/9p.c +@@ -498,9 +498,9 @@ static int coroutine_fn v9fs_mark_fids_unreclaim(V9fsPDU *pdu, V9fsPath *path) + { + int err; + V9fsState *s = pdu->s; +- V9fsFidState *fidp, head_fid; ++ V9fsFidState *fidp; + +- head_fid.next = s->fid_list; ++again: + for (fidp = s->fid_list; fidp; fidp = fidp->next) { + if (fidp->path.size != path->size) { + continue; +@@ -520,7 +520,7 @@ static int coroutine_fn v9fs_mark_fids_unreclaim(V9fsPDU *pdu, V9fsPath *path) + * switched to the worker thread + */ + if (err == 0) { +- fidp = &head_fid; ++ goto again; + } + } + } +-- +2.27.0 + -- Gitee From e2f94e7de5071baff0288b45fa6d6d2bf83be549 Mon Sep 17 00:00:00 2001 From: Chen Qun Date: Tue, 8 Jun 2021 21:27:10 +0800 Subject: [PATCH 2/3] spec: Update patch and changelog with !138 fix CVE-2021-20181 #I3UFOQ !138 9pfs: Fully restart unreclaim loop (CVE-2021-20181) Signed-off-by: Chen Qun --- qemu.spec | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/qemu.spec b/qemu.spec index dc3ef55..993ea55 100644 --- a/qemu.spec +++ b/qemu.spec @@ -310,6 +310,7 @@ Patch0297: spapr_pci-add-spapr-msi-read-method.patch Patch0298: tz-ppc-add-dummy-read-write-methods.patch Patch0299: imx7-ccm-add-digprog-mmio-write-method.patch Patch0300: bugfix-fix-Uninitialized-Free-Vulnerability.patch +Patch0301: 9pfs-Fully-restart-unreclaim-loop-CVE-2021-20181.patch BuildRequires: flex BuildRequires: bison @@ -698,6 +699,9 @@ getent passwd qemu >/dev/null || \ %endif %changelog +* Tue Jun 08 2021 Chen Qun +- 9pfs: Fully restart unreclaim loop (CVE-2021-20181) + * Wed Jun 02 2021 Chen Qun - bugfix: fix Uninitialized Free Vulnerability -- Gitee From c73e2bf7cbe7ac26f53bae327f915e5e58a69340 Mon Sep 17 00:00:00 2001 From: Chen Qun Date: Tue, 8 Jun 2021 21:27:10 +0800 Subject: [PATCH 3/3] spec: Update release version with !138 increase release verison by one Signed-off-by: Chen Qun --- qemu.spec | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/qemu.spec b/qemu.spec index 993ea55..81a1cdc 100644 --- a/qemu.spec +++ b/qemu.spec @@ -1,6 +1,6 @@ Name: qemu Version: 4.1.0 -Release: 48 +Release: 49 Epoch: 2 Summary: QEMU is a generic and open source machine emulator and virtualizer License: GPLv2 and BSD and MIT and CC-BY-SA-4.0 -- Gitee