[Virtio-fs] 回复：add no-pivot option to virtiofsd process

[Stefan Hajnoczi]

On Tue, Oct 22, 2019 at 03:00:01PM +0800, 白宇 wrote:
> From 278d81f5a00c35e431a0a450b6c6eb7b7edca787 Mon Sep 17 00:00:00 2001
> From: bekars <baiyu10 at baidu.com>
> Date: Sun, 20 Oct 2019 17:41:11 +0800
> Subject: [PATCH] add no pivot_root to virtiofsd.

Please write a commit description that explains the reason for this
patch.

I have a general concern about this patch:

virtiofsd uses pivot_root for sandboxing.  If the virtiofsd process is
compromised through a security bug there must be no way of accessing
files outside the shared directory.  A simple chroot(2) call is not
equivalent to the pivot_root(2) and namespace setup that is normally
done during startup.  Therefore, I'm not confident that this change
results in the same level of security as with pivot_root(2).

The following more complicated sequence of steps is described in
Documentation/filesystems/ramfs-rootfs-initramfs.txt:

  - When switching another root device, initrd would pivot_root and then
    umount the ramdisk.  But initramfs is rootfs: you can neither pivot_root
    rootfs, nor unmount it.  Instead delete everything out of rootfs to
    free up the space (find -xdev / -exec rm '{}' ';'), overmount rootfs
    with the new root (cd /newmount; mount --move . /; chroot .), attach
    stdin/stdout/stderr to the new /dev/console, and exec the new init.

    Since this is a remarkably persnickety process (and involves deleting
    commands before you can run them), the klibc package introduced a helper
    program (utils/run_init.c) to do all this for you.  Most other packages
    (such as busybox) have named this command "switch_root".

At least the "cd /newmount; mount --move . /" steps look worthwhile.
The mount namespace should contain only the shared directory and there
must be no way to get back to files outside the shared directory.

> diff --git a/contrib/virtiofsd/passthrough_ll.c b/contrib/virtiofsd/passthrough_ll.c
> index 5bfd650..f84dcca 100644
> --- a/contrib/virtiofsd/passthrough_ll.c
> +++ b/contrib/virtiofsd/passthrough_ll.c
> @@ -2061,7 +2061,11 @@ static void setup_pivot_root(const char *source)
>  		err(1, "fchdir(newroot)");
>  	}
>  
> +#ifdef CONFIG_NO_PIVOT
> +	if (chroot(source) < 0){
> +#else
>  	if (syscall(__NR_pivot_root, ".", ".") < 0){
> +#endif

There is no way of knowing if virtiofsd will be deployed on rootfs at
compile-time for virtiofsd binaries shipped by a project like Kata
Containers or as part of a distro package.  If you compile a custom
binary for yourself then it's not an issue but upstream virtio-fs
requires more generic code.

Please change this to a run-time fallback.  The rootfs code should be
execute when pivot_root(2) fails.

Stefan
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/virtio-fs/attachments/20191023/4a8b6e1b/attachment.sig>