[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 00/28] Series short description
From: |
Stefan Hajnoczi |
Subject: |
Re: [Qemu-devel] [PATCH v2 00/28] Series short description |
Date: |
Mon, 27 Feb 2017 15:33:02 +0000 |
User-agent: |
Mutt/1.7.1 (2016-10-04) |
On Sun, Feb 26, 2017 at 11:41:32PM +0100, Greg Kurz wrote:
> This series tries to fix CVE-2016-9602 reported by Jann Horn of Google
> Project Zero:
>
> https://bugzilla.redhat.com/show_bug.cgi?id=1413929
>
> This vulnerability affects all accesses to the underlying filesystem in
> the "local" backend code.
>
> If QEMU is started with:
>
> -fsdev local,security_model=<passthrough|none>,path=/foo/bar
>
> then the guest can cause QEMU to create symlinks in /foo/bar.
>
> This causes accesses to any path /foo/bar/some/path to be unsafe, since
> untrusted code within the guest (or in another guest sharing the same
> virtfs folder) could change some/path to point to a random path of the
> host filesystem.
>
> The core problem is that the "local" backend relies on path-based syscalls
> to access the underlying filesystem. All path-based syscalls are vulnerable
> to this issue, even open(O_NOFOLLOW) or syscalls that explicitly don't
> dereference symlinks, since the kernel only checks the rightmost element of
> the path. Depending on the privilege level of the QEMU process, a guest can
> end up opening, renaming, changing ACLs, unlinking... files on the host
> filesystem.
>
> The right way to address this is to use "at" variants of all syscalls in
> the "local" backend code. This requires to open directories without
> traversing any symlink in the intermediate path elements. There was a
> tentative to introduce an O_BENEATH flag for openat() that would address
> this:
>
> https://patchwork.kernel.org/patch/7007181/
>
> Unfortunately this never got merged. An alternative is to walk through all
> path elements manually with openat(O_NOFOLLOW).
>
> v2:
> - /proc based implementation for xattr code (fixes metadata perf drop
> observed with v1)
> - some code refactoring
>
> Stefan.
>
> I had to rework some patches you had already reviewed, please consider
> giving your Reviewed-by again if the changes are ok.
>
> Thanks.
>
> --
> Greg
>
> ---
>
> Greg Kurz (28):
> 9pfs: local: move xattr security ops to 9p-xattr.c
> 9pfs: remove side-effects in local_init()
> 9pfs: remove side-effects in local_open() and local_opendir()
> 9pfs: introduce openat_nofollow() helper
> 9pfs: local: keep a file descriptor on the shared folder
> 9pfs: local: open/opendir: don't follow symlinks
> 9pfs: local: lgetxattr: don't follow symlinks
> 9pfs: local: llistxattr: don't follow symlinks
> 9pfs: local: lsetxattr: don't follow symlinks
> 9pfs: local: lremovexattr: don't follow symlinks
> 9pfs: local: unlinkat: don't follow symlinks
> 9pfs: local: remove: don't follow symlinks
> 9pfs: local: utimensat: don't follow symlinks
> 9pfs: local: statfs: don't follow symlinks
> 9pfs: local: truncate: don't follow symlinks
> 9pfs: local: readlink: don't follow symlinks
> 9pfs: local: lstat: don't follow symlinks
> 9pfs: local: renameat: don't follow symlinks
> 9pfs: local: rename: use renameat
> 9pfs: local: improve error handling in link op
> 9pfs: local: link: don't follow symlinks
> 9pfs: local: chmod: don't follow symlinks
> 9pfs: local: chown: don't follow symlinks
> 9pfs: local: symlink: don't follow symlinks
> 9pfs: local: mknod: don't follow symlinks
> 9pfs: local: mkdir: don't follow symlinks
> 9pfs: local: open2: don't follow symlinks
> 9pfs: local: drop unused code
>
>
> hw/9pfs/9p-local.c | 1023
> ++++++++++++++++++++++++++---------------------
> hw/9pfs/9p-local.h | 20 +
> hw/9pfs/9p-posix-acl.c | 44 --
> hw/9pfs/9p-util.c | 65 +++
> hw/9pfs/9p-util.h | 52 ++
> hw/9pfs/9p-xattr-user.c | 24 -
> hw/9pfs/9p-xattr.c | 166 +++++++-
> hw/9pfs/9p-xattr.h | 87 +---
> hw/9pfs/Makefile.objs | 2
> 9 files changed, 887 insertions(+), 596 deletions(-)
> create mode 100644 hw/9pfs/9p-local.h
> create mode 100644 hw/9pfs/9p-util.c
> create mode 100644 hw/9pfs/9p-util.h
>
Reviewed-by: Stefan Hajnoczi <address@hidden>
signature.asc
Description: PGP signature
- [Qemu-devel] [PATCH v2 23/28] 9pfs: local: chown: don't follow symlinks, (continued)
- [Qemu-devel] [PATCH v2 23/28] 9pfs: local: chown: don't follow symlinks, Greg Kurz, 2017/02/26
- [Qemu-devel] [PATCH v2 24/28] 9pfs: local: symlink: don't follow symlinks, Greg Kurz, 2017/02/26
- [Qemu-devel] [PATCH v2 25/28] 9pfs: local: mknod: don't follow symlinks, Greg Kurz, 2017/02/26
- [Qemu-devel] [PATCH v2 26/28] 9pfs: local: mkdir: don't follow symlinks, Greg Kurz, 2017/02/26
- [Qemu-devel] [PATCH v2 27/28] 9pfs: local: open2: don't follow symlinks, Greg Kurz, 2017/02/26
- [Qemu-devel] [PATCH v2 28/28] 9pfs: local: drop unused code, Greg Kurz, 2017/02/26
- Re: [Qemu-devel] [PATCH v2 00/28] 9pfs: local: fix vulnerability to symlink attacks, Greg Kurz, 2017/02/26
- Re: [Qemu-devel] [PATCH v2 00/28] Series short description, Stefan Hajnoczi, 2017/02/27
- Re: [Qemu-devel] [PATCH v2 00/28] Series short description,
Stefan Hajnoczi <=