Re: [Qemu-devel] [PATCH 1/7] 9pfs: restrict open to regular files and directories

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 1545 bytes --]

On 01/10/2017 08:32 AM, Greg Kurz wrote:
> It really does not make sense for the 9P server to open anything else but
> a regular file or a directory.
> 
> Malicious code in a guest could for example create a named pipe, associate
> it to a valid fid and pass it to the server in a RLOPEN message. This would
> cause QEMU to hang in open(), waiting for someone to open the other end of
> the pipe.
> 
> Signed-off-by: Greg Kurz <groug@kaod.org>
> ---
>  hw/9pfs/9p.c |    5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
> index fa58877570f6..edd7b97270e3 100644
> --- a/hw/9pfs/9p.c
> +++ b/hw/9pfs/9p.c
> @@ -1462,7 +1462,7 @@ static void coroutine_fn v9fs_open(void *opaque)
>              goto out;
>          }
>          err += offset;
> -    } else {
> +    } else if (S_ISREG(stbuf.st_mode)) {
>          if (s->proto_version == V9FS_PROTO_2000L) {

TOCTTOU race.  You are checking the stat() results and only then calling
open(), rather than calling open() first and validating fstat().  That
means the guest can STILL cause you to open() a pipe by changing the
file type in between the stat and the open.

I think you need to rework this patch to open() first, then validate
(closing the fd if necessary); the open can be done with O_NONBLOCK to
avoid hanging on a pipe.  Yes, that's more annoying, but that's life
with TOCTTOU races.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]