[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] fsdev: fix virtfs-proxy-helper cwd
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH] fsdev: fix virtfs-proxy-helper cwd |
Date: |
Fri, 19 May 2017 17:19:10 -0500 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.1.0 |
On 05/19/2017 09:30 AM, Greg Kurz wrote:
> Since chroot() doesn't change the current directory, it is indeed a good
> practice to chdir() to the target directory and then then chroot(), or
> to chroot() to the target directory and then chdir("/").
>
> The current code does neither of them actually. Let's go for the latter.
>
> This doesn't fix any security issue since all of this takes place before
> the helper begins to process requests.
>
> Signed-off-by: Greg Kurz <address@hidden>
> ---
> fsdev/virtfs-proxy-helper.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
You are correct that failing to sanitize the current working directory
alongside a chroot() can lead to escaped access outside of the new
smaller root.
Aside: chdir() is annoying in multi-threaded apps - it is global state,
rather than thread-local. A multi-threaded app should therefore either
never change the current working directory, or else never rely on the
current working directory. But if I'm not mistaken, virtfs-proxy-helper
is an independent helper app, not qemu proper, so the use of
chdir/chroot is not affecting other threads.
Reviewed-by: Eric Blake <address@hidden>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
signature.asc
Description: OpenPGP digital signature