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 <gr...@kaod.org>
> ---
> 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 <ebl...@redhat.com> -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
signature.asc
Description: OpenPGP digital signature
