[Qemu-commits] [qemu/qemu] 506f32: virtio-9p/xen-9p: move 9p specific bits to core 9p...

[GitHub]

  Branch: refs/heads/master
  Home:   https://github.com/qemu/qemu
  Commit: 506f3275829258fdf193bbf0f98f3f9e8c0f3c45
      
https://github.com/qemu/qemu/commit/506f3275829258fdf193bbf0f98f3f9e8c0f3c45
  Author: Greg Kurz <address@hidden>
  Date:   2017-05-25 (Thu, 25 May 2017)

  Changed paths:
    M hw/9pfs/9p.c
    M hw/9pfs/9p.h
    M hw/9pfs/virtio-9p-device.c
    M hw/9pfs/xen-9p-backend.c

  Log Message:
  -----------
  virtio-9p/xen-9p: move 9p specific bits to core 9p code

These bits aren't related to the transport so let's move them to the core
code.

Signed-off-by: Greg Kurz <address@hidden>
Reviewed-by: Stefano Stabellini <address@hidden>


  Commit: 57a0aa6b507050734e302f1ff6da452518bd74e8
      
https://github.com/qemu/qemu/commit/57a0aa6b507050734e302f1ff6da452518bd74e8
  Author: Greg Kurz <address@hidden>
  Date:   2017-05-25 (Thu, 25 May 2017)

  Changed paths:
    M fsdev/9p-iov-marshal.c

  Log Message:
  -----------
  fsdev: don't allow unknown format in marshal/unmarshal

The code only uses well known format strings. An unknown format token is a
bug.

Signed-off-by: Greg Kurz <address@hidden>
Reviewed-by: Stefano Stabellini <address@hidden>


  Commit: a17d8659c466aa2470fdf5b05c88e9e68d40d6ee
      
https://github.com/qemu/qemu/commit/a17d8659c466aa2470fdf5b05c88e9e68d40d6ee
  Author: Greg Kurz <address@hidden>
  Date:   2017-05-25 (Thu, 25 May 2017)

  Changed paths:
    M hw/9pfs/9p.c

  Log Message:
  -----------
  9pfs: drop pdu_push_and_notify()

Only pdu_complete() needs to notify the client that a request has completed.

Signed-off-by: Greg Kurz <address@hidden>
Reviewed-by: Stefano Stabellini <address@hidden>


  Commit: 6a87e7929f97b86c5823d4616fa1aa7636b2f116
      
https://github.com/qemu/qemu/commit/6a87e7929f97b86c5823d4616fa1aa7636b2f116
  Author: Greg Kurz <address@hidden>
  Date:   2017-05-25 (Thu, 25 May 2017)

  Changed paths:
    M hw/9pfs/9p-local.c

  Log Message:
  -----------
  9pfs: local: fix unlink of alien files in mapped-file mode

When trying to remove a file from a directory, both created in non-mapped
mode, the file remains and EBADF is returned to the guest.

This is a regression introduced by commit "df4938a6651b 9pfs: local:
unlinkat: don't follow symlinks" when fixing CVE-2016-9602. It changed the
way we unlink the metadata file from

    ret = remove("$dir/.virtfs_metadata/$name");
    if (ret < 0 && errno != ENOENT) {
   /* Error out */
    }
    /* Ignore absence of metadata */

to

    fd = openat("$dir/.virtfs_metadata")
    unlinkat(fd, "$name")
    if (ret < 0 && errno != ENOENT) {
   /* Error out */
    }
    /* Ignore absence of metadata */

If $dir was created in non-mapped mode, openat() fails with ENOENT and
we pass -1 to unlinkat(), which fails in turn with EBADF.

We just need to check the return of openat() and ignore ENOENT, in order
to restore the behaviour we had with remove().

Signed-off-by: Greg Kurz <address@hidden>
Reviewed-by: Eric Blake <address@hidden>
[groug: rewrote the comments as suggested by Eric]


  Commit: 4be56c19593c15bc8d16abcb63bfdd4697a5754d
      
https://github.com/qemu/qemu/commit/4be56c19593c15bc8d16abcb63bfdd4697a5754d
  Author: Greg Kurz <address@hidden>
  Date:   2017-05-25 (Thu, 25 May 2017)

  Changed paths:
    M fsdev/virtfs-proxy-helper.c

  Log Message:
  -----------
  fsdev: fix virtfs-proxy-helper cwd

Since chroot() doesn't change the current directory, it is indeed a good
practice to chdir() to the target directory and then then chroot(), or
to chroot() to the target directory and then chdir("/").

The current code does neither of them actually. Let's go for the latter.

This doesn't fix any security issue since all of this takes place before
the helper begins to process requests.

Signed-off-by: Greg Kurz <address@hidden>
Reviewed-by: Eric Blake <address@hidden>


  Commit: 24df3371d97a7516605aef8abbc253a8c162b211
      
https://github.com/qemu/qemu/commit/24df3371d97a7516605aef8abbc253a8c162b211
  Author: Greg Kurz <address@hidden>
  Date:   2017-05-25 (Thu, 25 May 2017)

  Changed paths:
    M fsdev/virtfs-proxy-helper.c
    M hw/9pfs/9p-handle.c

  Log Message:
  -----------
  9pfs: assume utimensat() and futimens() are present

The utimensat() and futimens() syscalls have been around for ages (ie,
glibc 2.6 and linux 2.6.22), and the decision was already taken to
switch to utimensat() anyway when fixing CVE-2016-9602 in 2.9.

Signed-off-by: Greg Kurz <address@hidden>
Reviewed-by: Eric Blake <address@hidden>


  Commit: fcdcf1eed2fd26bfe836080755ba4322d3c1f2cc
      
https://github.com/qemu/qemu/commit/fcdcf1eed2fd26bfe836080755ba4322d3c1f2cc
  Author: Greg Kurz <address@hidden>
  Date:   2017-05-25 (Thu, 25 May 2017)

  Changed paths:
    M configure
    M include/sysemu/os-posix.h
    M util/oslib-posix.c

  Log Message:
  -----------
  util: drop old utimensat() compat code

Now that 9pfs and virtfs-proxy-helper have been converted to utimensat(),
we don't need to keep qemu_utimens() anymore.

Signed-off-by: Greg Kurz <address@hidden>
Reviewed-by: Eric Blake <address@hidden>


  Commit: 4fa62005d0a85df9bb14ea9f53109d7c5f66eeb9
      
https://github.com/qemu/qemu/commit/4fa62005d0a85df9bb14ea9f53109d7c5f66eeb9
  Author: Greg Kurz <address@hidden>
  Date:   2017-05-25 (Thu, 25 May 2017)

  Changed paths:
    M hw/9pfs/9p.c

  Log Message:
  -----------
  9pfs: check return value of v9fs_co_name_to_path()

These v9fs_co_name_to_path() call sites have always been around. I guess
no care was taken to check the return value because the name_to_path
operation could never fail at the time. This is no longer true: the
handle and synth backends can already fail this operation, and so will the
local backend soon.

Signed-off-by: Greg Kurz <address@hidden>
Reviewed-by: Eric Blake <address@hidden>


  Commit: f57f5878578af19f72344439154234c6d6ba8ccc
      
https://github.com/qemu/qemu/commit/f57f5878578af19f72344439154234c6d6ba8ccc
  Author: Greg Kurz <address@hidden>
  Date:   2017-05-25 (Thu, 25 May 2017)

  Changed paths:
    M hw/9pfs/9p-local.c

  Log Message:
  -----------
  9pfs: local: resolve special directories in paths

When using the mapped-file security mode, the creds of a path /foo/bar
are stored in the /foo/.virtfs_metadata/bar file. This is okay for all
paths unless they end with '.' or '..', because we cannot create the
corresponding file in the metadata directory.

This patch ensures that '.' and '..' are resolved in all paths.

The core code only passes path elements (no '/') to the backend, with
the notable exception of the '/' path, which refers to the virtfs root.
This patch preserves the current behavior of converting it to '.' so
that it can be passed to "*at()" syscalls ('/' would mean the host root).

Signed-off-by: Greg Kurz <address@hidden>
Reviewed-by: Eric Blake <address@hidden>


  Commit: 3dbcf27334b6c41e74a476b55d76f60df1c4007b
      
https://github.com/qemu/qemu/commit/3dbcf27334b6c41e74a476b55d76f60df1c4007b
  Author: Greg Kurz <address@hidden>
  Date:   2017-05-25 (Thu, 25 May 2017)

  Changed paths:
    M hw/9pfs/9p-local.c
    M hw/9pfs/9p-util.c
    M hw/9pfs/9p-util.h

  Log Message:
  -----------
  9pfs: local: simplify file opening

The logic to open a path currently sits between local_open_nofollow() and
the relative_openat_nofollow() helper, which has no other user.

For the sake of clarity, this patch moves all the code of the helper into
its unique caller. While here we also:
- drop the code to skip leading "/" because the backend isn't supposed to
  pass anything but relative paths without consecutive slashes. The assert()
  is kept because we really don't want a buggy backend to pass an absolute
  path to openat().
- use strchrnul() to get a simpler code. This is ok since virtfs is for
  linux+glibc hosts only.
- don't dup() the initial directory and add an assert() to ensure we don't
  return the global mountfd to the caller. BTW, this would mean that the
  caller passed an empty path, which isn't supposed to happen either.

Signed-off-by: Greg Kurz <address@hidden>
Reviewed-by: Eric Blake <address@hidden>
[groug: fixed typos in changelog]


  Commit: 81ffbf5ab1458e357a761f1272105a55829b351e
      
https://github.com/qemu/qemu/commit/81ffbf5ab1458e357a761f1272105a55829b351e
  Author: Greg Kurz <address@hidden>
  Date:   2017-05-25 (Thu, 25 May 2017)

  Changed paths:
    M hw/9pfs/9p-local.c

  Log Message:
  -----------
  9pfs: local: metadata file for the VirtFS root

When using the mapped-file security, credentials are stored in a metadata
directory located in the parent directory. This is okay for all paths with
the notable exception of the root path, since we don't want and probably
can't create a metadata directory above the virtfs directory on the host.

This patch introduces a dedicated metadata file, sitting in the virtfs root
for this purpose. It relies on the fact that the "." name necessarily refers
to the virtfs root.

As for the metadata directory, we don't want the client to see this file.
The current code only cares for readdir() but there are many other places
to fix actually. The filtering logic is hence put in a separate function.

Before:

# ls -ld
drwxr-xr-x. 3 greg greg 4096 May  5 12:49 .
# chown root.root .
chown: changing ownership of '.': Is a directory
# ls -ld
drwxr-xr-x. 3 greg greg 4096 May  5 12:49 .

After:

# ls -ld
drwxr-xr-x. 3 greg greg 4096 May  5 12:49 .
# chown root.root .
# ls -ld
drwxr-xr-x. 3 root root 4096 May  5 12:50 .

and from the host:

ls -al .virtfs_metadata_root
-rwx------. 1 greg greg 26 May  5 12:50 .virtfs_metadata_root
$ cat .virtfs_metadata_root
virtfs.uid=0
virtfs.gid=0

Reported-by: Leo Gaspard <address@hidden>
Signed-off-by: Greg Kurz <address@hidden>
Reviewed-by: Eric Blake <address@hidden>
Tested-by: Leo Gaspard <address@hidden>
[groug: work around a patchew false positive in
  local_set_mapped_file_attrat()]


  Commit: 066ae4f829bcc6b8c98994a7c22fe570d500d548
      
https://github.com/qemu/qemu/commit/066ae4f829bcc6b8c98994a7c22fe570d500d548
  Author: Peter Maydell <address@hidden>
  Date:   2017-06-01 (Thu, 01 Jun 2017)

  Changed paths:
    M configure
    M fsdev/9p-iov-marshal.c
    M fsdev/virtfs-proxy-helper.c
    M hw/9pfs/9p-handle.c
    M hw/9pfs/9p-local.c
    M hw/9pfs/9p-util.c
    M hw/9pfs/9p-util.h
    M hw/9pfs/9p.c
    M hw/9pfs/9p.h
    M hw/9pfs/virtio-9p-device.c
    M hw/9pfs/xen-9p-backend.c
    M include/sysemu/os-posix.h
    M util/oslib-posix.c

  Log Message:
  -----------
  Merge remote-tracking branch 'remotes/gkurz/tags/for-upstream' into staging

Various bugfixes and code cleanups. Most notably, it fixes metadata handling in
mapped-file security mode (especially for the virtfs root).

# gpg: Signature made Tue 30 May 2017 14:36:22 BST
# gpg:                using DSA key 0x02FC3AEB0101DBC2
# gpg: Good signature from "Greg Kurz <address@hidden>"
# gpg:                 aka "Greg Kurz <address@hidden>"
# gpg:                 aka "Greg Kurz <address@hidden>"
# gpg:                 aka "Gregory Kurz (Groug) <address@hidden>"
# gpg:                 aka "[jpeg image of size 3330]"
# gpg: WARNING: This key is not certified with a trusted signature!
# gpg:          There is no indication that the signature belongs to the owner.
# Primary key fingerprint: 2BD4 3B44 535E C0A7 9894  DBA2 02FC 3AEB 0101 DBC2

* remotes/gkurz/tags/for-upstream:
  9pfs: local: metadata file for the VirtFS root
  9pfs: local: simplify file opening
  9pfs: local: resolve special directories in paths
  9pfs: check return value of v9fs_co_name_to_path()
  util: drop old utimensat() compat code
  9pfs: assume utimensat() and futimens() are present
  fsdev: fix virtfs-proxy-helper cwd
  9pfs: local: fix unlink of alien files in mapped-file mode
  9pfs: drop pdu_push_and_notify()
  fsdev: don't allow unknown format in marshal/unmarshal
  virtio-9p/xen-9p: move 9p specific bits to core 9p code

Signed-off-by: Peter Maydell <address@hidden>


Compare: https://github.com/qemu/qemu/compare/70f31414e712...066ae4f829bc