[v5,0/3] virtiofsd: prevent opening of special files (CVE-2020-35517)

Message

Stefan Hajnoczi Feb. 4, 2021, 3:02 p.m. UTC

v4:
 * Patch 1: Return positive errno if openat(2) fails in lo_do_open() [Greg]
 * Patch 3: Return -fd instead or -errno after lo_inode_open() in lo_do_open() [Greg]
 * Patch 3: Use De Morgan's Law to simplify the boolean expression in lo_create() [Vivek]
 * Patch 3: Add missing errno = -truncfd after lo_inode_open() call in lo_setattr
v3:
 * Restructure lo_create() to handle externally-created files (we need
   to allocate an inode for them) [Greg]
 * Patch 1 & 2 refactor the code so that Patch 3 can implement the CVE fix
v3:
 * Protect lo_create() [Greg]
v2:
 * Add doc comment clarifying that symlinks are traversed client-side
   [Daniel]

A well-behaved FUSE client does not attempt to open special files with
FUSE_OPEN because they are handled on the client side (e.g. device nodes
are handled by client-side device drivers).

The check to prevent virtiofsd from opening special files is missing in
a few cases, most notably FUSE_OPEN. A malicious client can cause
virtiofsd to open a device node, potentially allowing the guest to
escape. This can be exploited by a modified guest device driver. It is
not exploitable from guest userspace since the guest kernel will handle
special files inside the guest instead of sending FUSE requests.

This patch series fixes this issue by introducing the lo_inode_open() function
to check the file type before opening it. This is a short-term solution because
it does not prevent a compromised virtiofsd process from opening device nodes
on the host.

This issue was diagnosed on public IRC and is therefore already known
and not embargoed.

Reported-by: Alex Xu <[e-mail]>
Fixes: CVE-2020-35517

Stefan Hajnoczi (3):
  virtiofsd: extract lo_do_open() from lo_open()
  virtiofsd: optionally return inode pointer from lo_do_lookup()
  virtiofsd: prevent opening of special files (CVE-2020-35517)

 tools/virtiofsd/passthrough_ll.c | 224 ++++++++++++++++++++-----------
 1 file changed, 148 insertions(+), 76 deletions(-)

Comments

[e-mail] Feb. 4, 2021, 4:15 p.m. UTC | #1

Patchew URL: https://patchew.org/QEMU/[e-mail]/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: [e-mail]
Subject: [PATCH v5 0/3] virtiofsd: prevent opening of special files (CVE-2020-35517)

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
   db754f8..1ba089f  master     -> master
 - [tag update]      patchew/[e-mail] -> patchew/[e-mail]
 - [tag update]      patchew/[e-mail] -> patchew/[e-mail]
 * [new tag]         patchew/[e-mail] -> patchew/[e-mail]
 * [new tag]         patchew/[e-mail] -> patchew/[e-mail]
Switched to a new branch 'test'
b5bb803 virtiofsd: prevent opening of special files (CVE-2020-35517)
0d88a79 virtiofsd: optionally return inode pointer from lo_do_lookup()
be6aa23 virtiofsd: extract lo_do_open() from lo_open()

=== OUTPUT BEGIN ===
1/3 Checking commit be6aa2319875 (virtiofsd: extract lo_do_open() from lo_open())
ERROR: return of an errno should typically be -ve (return -ENOMEM)
#70: FILE: tools/virtiofsd/passthrough_ll.c:1674:
+        return ENOMEM;

total: 1 errors, 0 warnings, 114 lines checked

Patch 1/3 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

2/3 Checking commit 0d88a7925a83 (virtiofsd: optionally return inode pointer from lo_do_lookup())
3/3 Checking commit b5bb8039eb3c (virtiofsd: prevent opening of special files (CVE-2020-35517))
=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/[e-mail]/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to [e-mail]

Dr. David Alan Gilbert Feb. 4, 2021, 5:35 p.m. UTC | #2

* [e-mail] ([e-mail]) wrote:
> Patchew URL: https://patchew.org/QEMU/[e-mail]/
> 
> 
> 
> Hi,
> 
> This series seems to have some coding style problems. See output below for
> more information:
> 
> Type: series
> Message-id: [e-mail]
> Subject: [PATCH v5 0/3] virtiofsd: prevent opening of special files (CVE-2020-35517)
> 
> === TEST SCRIPT BEGIN ===
> #!/bin/bash
> git rev-parse base > /dev/null || exit 0
> git config --local diff.renamelimit 0
> git config --local diff.renames True
> git config --local diff.algorithm histogram
> ./scripts/checkpatch.pl --mailback base..
> === TEST SCRIPT END ===
> 
> Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
> From https://github.com/patchew-project/qemu
>    db754f8..1ba089f  master     -> master
>  - [tag update]      patchew/[e-mail] -> patchew/[e-mail]
>  - [tag update]      patchew/[e-mail] -> patchew/[e-mail]
>  * [new tag]         patchew/[e-mail] -> patchew/[e-mail]
>  * [new tag]         patchew/[e-mail] -> patchew/[e-mail]
> Switched to a new branch 'test'
> b5bb803 virtiofsd: prevent opening of special files (CVE-2020-35517)
> 0d88a79 virtiofsd: optionally return inode pointer from lo_do_lookup()
> be6aa23 virtiofsd: extract lo_do_open() from lo_open()
> 
> === OUTPUT BEGIN ===
> 1/3 Checking commit be6aa2319875 (virtiofsd: extract lo_do_open() from lo_open())
> ERROR: return of an errno should typically be -ve (return -ENOMEM)
> #70: FILE: tools/virtiofsd/passthrough_ll.c:1674:
> +        return ENOMEM;

This is intended

> total: 1 errors, 0 warnings, 114 lines checked
> 
> Patch 1/3 has style problems, please review.  If any of these errors
> are false positives report them to the maintainer, see
> CHECKPATCH in MAINTAINERS.
> 
> 2/3 Checking commit 0d88a7925a83 (virtiofsd: optionally return inode pointer from lo_do_lookup())
> 3/3 Checking commit b5bb8039eb3c (virtiofsd: prevent opening of special files (CVE-2020-35517))
> === OUTPUT END ===
> 
> Test command exited with code: 1
> 
> 
> The full log is available at
> http://patchew.org/logs/[e-mail]/testing.checkpatch/?type=message.
> ---
> Email generated automatically by Patchew [https://patchew.org/].
> Please send your feedback to [e-mail]

Dr. David Alan Gilbert Feb. 4, 2021, 6:14 p.m. UTC | #3

* Stefan Hajnoczi ([e-mail]) wrote:
> v4:
>  * Patch 1: Return positive errno if openat(2) fails in lo_do_open() [Greg]
>  * Patch 3: Return -fd instead or -errno after lo_inode_open() in lo_do_open() [Greg]
>  * Patch 3: Use De Morgan's Law to simplify the boolean expression in lo_create() [Vivek]
>  * Patch 3: Add missing errno = -truncfd after lo_inode_open() call in lo_setattr
> v3:
>  * Restructure lo_create() to handle externally-created files (we need
>    to allocate an inode for them) [Greg]
>  * Patch 1 & 2 refactor the code so that Patch 3 can implement the CVE fix
> v3:
>  * Protect lo_create() [Greg]
> v2:
>  * Add doc comment clarifying that symlinks are traversed client-side
>    [Daniel]
> 
> A well-behaved FUSE client does not attempt to open special files with
> FUSE_OPEN because they are handled on the client side (e.g. device nodes
> are handled by client-side device drivers).
> 
> The check to prevent virtiofsd from opening special files is missing in
> a few cases, most notably FUSE_OPEN. A malicious client can cause
> virtiofsd to open a device node, potentially allowing the guest to
> escape. This can be exploited by a modified guest device driver. It is
> not exploitable from guest userspace since the guest kernel will handle
> special files inside the guest instead of sending FUSE requests.
> 
> This patch series fixes this issue by introducing the lo_inode_open() function
> to check the file type before opening it. This is a short-term solution because
> it does not prevent a compromised virtiofsd process from opening device nodes
> on the host.
> 
> This issue was diagnosed on public IRC and is therefore already known
> and not embargoed.
> 
> Reported-by: Alex Xu <[e-mail]>
> Fixes: CVE-2020-35517

Queued

> Stefan Hajnoczi (3):
>   virtiofsd: extract lo_do_open() from lo_open()
>   virtiofsd: optionally return inode pointer from lo_do_lookup()
>   virtiofsd: prevent opening of special files (CVE-2020-35517)
> 
>  tools/virtiofsd/passthrough_ll.c | 224 ++++++++++++++++++++-----------
>  1 file changed, 148 insertions(+), 76 deletions(-)
> 
> -- 
> 2.29.2
>