[PULL 0/1] 9p security fix 2021-01-15

[PULL 0/1] 9p security fix 2021-01-15

[Greg Kurz]

The following changes since commit 7c79721606be11b5bc556449e5bcbc331ef6867d:

  Merge remote-tracking branch 'remotes/rth-gitlab/tags/pull-tcg-20210113' into staging (2021-01-14 09:54:29 +0000)

are available in the Git repository at:

  https://gitlab.com/gkurz/qemu.git tags/9p-next-2021-01-15

for you to fetch changes up to 89fbea8737e8f7b954745a1ffc4238d377055305:

  9pfs: Fully restart unreclaim loop (CVE-2021-20181) (2021-01-15 08:44:28 +0100)

----------------------------------------------------------------
Fix for CVE-2021-20181

----------------------------------------------------------------
Greg Kurz (1):
      9pfs: Fully restart unreclaim loop (CVE-2021-20181)

 hw/9pfs/9p.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
-- 
2.26.2

[PULL 1/1] 9pfs: Fully restart unreclaim loop (CVE-2021-20181)

[Greg Kurz]

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>
---
 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 94df440fc740..6026b51a1c04 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.26.2

Re: [PULL 0/1] 9p security fix 2021-01-15

[Peter Maydell]

On Fri, 15 Jan 2021 at 09:05, Greg Kurz <groug@kaod.org> wrote:
>
> The following changes since commit 7c79721606be11b5bc556449e5bcbc331ef6867d:
>
>   Merge remote-tracking branch 'remotes/rth-gitlab/tags/pull-tcg-20210113' into staging (2021-01-14 09:54:29 +0000)
>
> are available in the Git repository at:
>
>   https://gitlab.com/gkurz/qemu.git tags/9p-next-2021-01-15
>
> for you to fetch changes up to 89fbea8737e8f7b954745a1ffc4238d377055305:
>
>   9pfs: Fully restart unreclaim loop (CVE-2021-20181) (2021-01-15 08:44:28 +0100)
>
> ----------------------------------------------------------------
> Fix for CVE-2021-20181

Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/6.0
for any user-visible changes.

-- PMM