[Qemu-devel] [PULL 0/1] 9p fixes for v3.1.0-rc2

[Qemu-devel] [PULL 0/1] 9p fixes for v3.1.0-rc2

[Greg Kurz]

The following changes since commit 3c035a41dca808f096a128fe2b62d849fe638a25:

  Merge remote-tracking branch 'remotes/huth-gitlab/tags/pull-request-2018-11-20' into staging (2018-11-20 10:56:57 +0000)

are available in the Git repository at:

  https://github.com/gkurz/qemu.git tags/for-upstream

for you to fetch changes up to 5b3c77aa581ebb215125c84b0742119483571e55:

  9p: take write lock on fid path updates (CVE-2018-19364) (2018-11-20 13:00:35 +0100)

----------------------------------------------------------------
Fixes yet another use-after-free issue that could be triggered by a
misbehaving guest. This is a follow-up to commit:

commit 5b76ef50f62079a2389ba28cacaf6cce68b1a0ed
Author: Greg Kurz <groug@kaod.org>
Date:   Wed Nov 7 01:00:04 2018 +0100

    9p: write lock path in v9fs_co_open2()

----------------------------------------------------------------
Greg Kurz (1):
      9p: take write lock on fid path updates (CVE-2018-19364)

 hw/9pfs/9p.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)
-- 
2.17.2

[Qemu-devel] [PULL 1/1] 9p: take write lock on fid path updates (CVE-2018-19364)

[Greg Kurz]

Recent commit 5b76ef50f62079a fixed a race where v9fs_co_open2() could
possibly overwrite a fid path with v9fs_path_copy() while it is being
accessed by some other thread, ie, use-after-free that can be detected
by ASAN with a custom 9p client.

It turns out that the same can happen at several locations where
v9fs_path_copy() is used to set the fid path. The fix is again to
take the write lock.

Fixes CVE-2018-19364.

Cc: P J P <ppandit@redhat.com>
Reported-by: zhibin hu <noirfate@gmail.com>
Reviewed-by: Prasad J Pandit <pjp@fedoraproject.org>
Signed-off-by: Greg Kurz <groug@kaod.org>
---
 hw/9pfs/9p.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
index eef289e394d4..267a25533b77 100644
--- a/hw/9pfs/9p.c
+++ b/hw/9pfs/9p.c
@@ -1391,7 +1391,9 @@ static void coroutine_fn v9fs_walk(void *opaque)
             err = -EINVAL;
             goto out;
         }
+        v9fs_path_write_lock(s);
         v9fs_path_copy(&fidp->path, &path);
+        v9fs_path_unlock(s);
     } else {
         newfidp = alloc_fid(s, newfid);
         if (newfidp == NULL) {
@@ -2160,6 +2162,7 @@ static void coroutine_fn v9fs_create(void *opaque)
     V9fsString extension;
     int iounit;
     V9fsPDU *pdu = opaque;
+    V9fsState *s = pdu->s;
 
     v9fs_path_init(&path);
     v9fs_string_init(&name);
@@ -2200,7 +2203,9 @@ static void coroutine_fn v9fs_create(void *opaque)
         if (err < 0) {
             goto out;
         }
+        v9fs_path_write_lock(s);
         v9fs_path_copy(&fidp->path, &path);
+        v9fs_path_unlock(s);
         err = v9fs_co_opendir(pdu, fidp);
         if (err < 0) {
             goto out;
@@ -2216,7 +2221,9 @@ static void coroutine_fn v9fs_create(void *opaque)
         if (err < 0) {
             goto out;
         }
+        v9fs_path_write_lock(s);
         v9fs_path_copy(&fidp->path, &path);
+        v9fs_path_unlock(s);
     } else if (perm & P9_STAT_MODE_LINK) {
         int32_t ofid = atoi(extension.data);
         V9fsFidState *ofidp = get_fid(pdu, ofid);
@@ -2234,7 +2241,9 @@ static void coroutine_fn v9fs_create(void *opaque)
             fidp->fid_type = P9_FID_NONE;
             goto out;
         }
+        v9fs_path_write_lock(s);
         v9fs_path_copy(&fidp->path, &path);
+        v9fs_path_unlock(s);
         err = v9fs_co_lstat(pdu, &fidp->path, &stbuf);
         if (err < 0) {
             fidp->fid_type = P9_FID_NONE;
@@ -2272,7 +2281,9 @@ static void coroutine_fn v9fs_create(void *opaque)
         if (err < 0) {
             goto out;
         }
+        v9fs_path_write_lock(s);
         v9fs_path_copy(&fidp->path, &path);
+        v9fs_path_unlock(s);
     } else if (perm & P9_STAT_MODE_NAMED_PIPE) {
         err = v9fs_co_mknod(pdu, fidp, &name, fidp->uid, -1,
                             0, S_IFIFO | (perm & 0777), &stbuf);
@@ -2283,7 +2294,9 @@ static void coroutine_fn v9fs_create(void *opaque)
         if (err < 0) {
             goto out;
         }
+        v9fs_path_write_lock(s);
         v9fs_path_copy(&fidp->path, &path);
+        v9fs_path_unlock(s);
     } else if (perm & P9_STAT_MODE_SOCKET) {
         err = v9fs_co_mknod(pdu, fidp, &name, fidp->uid, -1,
                             0, S_IFSOCK | (perm & 0777), &stbuf);
@@ -2294,7 +2307,9 @@ static void coroutine_fn v9fs_create(void *opaque)
         if (err < 0) {
             goto out;
         }
+        v9fs_path_write_lock(s);
         v9fs_path_copy(&fidp->path, &path);
+        v9fs_path_unlock(s);
     } else {
         err = v9fs_co_open2(pdu, fidp, &name, -1,
                             omode_to_uflags(mode)|O_CREAT, perm, &stbuf);
-- 
2.17.2

Re: [Qemu-devel] [PULL 0/1] 9p fixes for v3.1.0-rc2

[Peter Maydell]

On 20 November 2018 at 12:03, Greg Kurz <groug@kaod.org> wrote:
> The following changes since commit 3c035a41dca808f096a128fe2b62d849fe638a25:
>
>   Merge remote-tracking branch 'remotes/huth-gitlab/tags/pull-request-2018-11-20' into staging (2018-11-20 10:56:57 +0000)
>
> are available in the Git repository at:
>
>   https://github.com/gkurz/qemu.git tags/for-upstream
>
> for you to fetch changes up to 5b3c77aa581ebb215125c84b0742119483571e55:
>
>   9p: take write lock on fid path updates (CVE-2018-19364) (2018-11-20 13:00:35 +0100)
>
> ----------------------------------------------------------------
> Fixes yet another use-after-free issue that could be triggered by a
> misbehaving guest. This is a follow-up to commit:
>
> commit 5b76ef50f62079a2389ba28cacaf6cce68b1a0ed
> Author: Greg Kurz <groug@kaod.org>
> Date:   Wed Nov 7 01:00:04 2018 +0100
>
>     9p: write lock path in v9fs_co_open2()
>
> ----------------------------------------------------------------
> Greg Kurz (1):
>       9p: take write lock on fid path updates (CVE-2018-19364)
>
Applied, thanks.

-- PMM