qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Eric Blake
Subject: Re: [Qemu-devel] [PATCH 1/7] 9pfs: restrict open to regular files and directories
Date: Tue, 10 Jan 2017 08:38:27 -0600
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.6.0

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 <address@hidden>
> ---
>  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: signature.asc
Description: OpenPGP digital signature


reply via email to

[Prev in Thread] Current Thread [Next in Thread]