81 lines
3.1 KiB
Diff
81 lines
3.1 KiB
Diff
|
From 31012acf8fcf51131d1553bc3e04176ae51e8228 Mon Sep 17 00:00:00 2001
|
||
|
|
From: Greg Kurz <groug@kaod.org>
|
||
|
|
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 <zdi-disclosures@trendmicro.com>
|
||
|
|
Reviewed-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
|
||
|
|
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
|
||
|
|
Message-Id: <161064025265.1838153.15185571283519390907.stgit@bahia.lan>
|
||
|
|
Signed-off-by: Greg Kurz <groug@kaod.org>
|
||
|
|
|
||
|
|
Signed-off-by: Jiajie Li <lijiajie11@huawei.com>
|
||
|
|
---
|
||
|
|
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
|
||
|
|
|