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

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

[Greg Kurz]

The following changes since commit a7ce790a029bd94eb320d8c69f38900f5233997e:

  tcg/tcg-op.h: Add multiple include guard (2018-11-08 15:15:32 +0000)

are available in the Git repository at:

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

for you to fetch changes up to 5b76ef50f62079a2389ba28cacaf6cce68b1a0ed:

  9p: write lock path in v9fs_co_open2() (2018-11-08 21:19:05 +0100)

----------------------------------------------------------------
Fixes a potential use-after-free issue that could be triggered by a
misbehaving guest.

----------------------------------------------------------------
Greg Kurz (1):
      9p: write lock path in v9fs_co_open2()

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

[Qemu-devel] [PULL 1/1] 9p: write lock path in v9fs_co_open2()

[Greg Kurz]

The assumption that the fid cannot be used by any other operation is
wrong. At least, nothing prevents a misbehaving client to create a
file with a given fid, and to pass this fid to some other operation
at the same time (ie, without waiting for the response to the creation
request). The call to v9fs_path_copy() performed by the worker thread
after the file was created can race with any access to the fid path
performed by some other thread. This causes use-after-free issues that
can be detected by ASAN with a custom 9p client.

Unlike other operations that only read the fid path, v9fs_co_open2()
does modify it. It should hence take the write lock.

Cc: P J P <ppandit@redhat.com>
Reported-by: zhibin hu <noirfate@gmail.com>
Signed-off-by: Greg Kurz <groug@kaod.org>
---
 hw/9pfs/cofile.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/hw/9pfs/cofile.c b/hw/9pfs/cofile.c
index 88791bc327ac..9c22837cda32 100644
--- a/hw/9pfs/cofile.c
+++ b/hw/9pfs/cofile.c
@@ -140,10 +140,10 @@ int coroutine_fn v9fs_co_open2(V9fsPDU *pdu, V9fsFidState *fidp,
     cred.fc_gid = gid;
     /*
      * Hold the directory fid lock so that directory path name
-     * don't change. Read lock is fine because this fid cannot
-     * be used by any other operation.
+     * don't change. Take the write lock to be sure this fid
+     * cannot be used by another operation.
      */
-    v9fs_path_read_lock(s);
+    v9fs_path_write_lock(s);
     v9fs_co_run_in_worker(
         {
             err = s->ops->open2(&s->ctx, &fidp->path,
-- 
2.17.2

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

[Peter Maydell]

On 8 November 2018 at 20:41, Greg Kurz <groug@kaod.org> wrote:
> The following changes since commit a7ce790a029bd94eb320d8c69f38900f5233997e:
>
>   tcg/tcg-op.h: Add multiple include guard (2018-11-08 15:15:32 +0000)
>
> are available in the Git repository at:
>
>   https://github.com/gkurz/qemu.git tags/for-upstream
>
> for you to fetch changes up to 5b76ef50f62079a2389ba28cacaf6cce68b1a0ed:
>
>   9p: write lock path in v9fs_co_open2() (2018-11-08 21:19:05 +0100)
>
> ----------------------------------------------------------------
> Fixes a potential use-after-free issue that could be triggered by a
> misbehaving guest.
>
> ----------------------------------------------------------------
> Greg Kurz (1):
>       9p: write lock path in v9fs_co_open2()
>
>  hw/9pfs/cofile.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)

Applied, thanks.

-- PMM