| From c2d2d14e8deece958bbc4fc649d22c3564bc4e7e Mon Sep 17 00:00:00 2001 |
| From: Greg Kurz <groug@kaod.org> |
| Date: Thu, 14 Jan 2021 17:04:12 +0100 |
| Subject: [PATCH] 9pfs: Fully restart unreclaim loop (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> |
| |
| Upstream-Status: Backport [89fbea8737e8f7b954745a1ffc4238d377055305] |
| CVE: CVE-2021-20181 |
| |
| Signed-off-by: Sakib Sajal <sakib.sajal@windriver.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 94df440fc..6026b51a1 100644 |
| --- a/hw/9pfs/9p.c |
| +++ b/hw/9pfs/9p.c |
| @@ -502,9 +502,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; |
| @@ -524,7 +524,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.29.2 |
| |