Re: [Qemu-devel] [PATCH v2 00/28] Series short description

[Stefan Hajnoczi]

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.

I have reviewed patches that didn't have R-b from me.  Please see
comments on individual patches.

Stefan

signature.asc
Description: PGP signature