Re: [PATCH v2 4/5] virtiofsd: Open lo->source while setting up root in sandbox=NONE mode

[Stefan Hajnoczi <stefanha@redhat.com>]

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

On Thu, Jul 30, 2020 at 03:47:35PM -0400, Vivek Goyal wrote:
> In sandbox=NONE mode, lo->source points to the directory which is being
> exported. We have not done any chroot()/pivot_root(). So open lo->source.
> 
> Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
> ---
>  tools/virtiofsd/passthrough_ll.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/virtiofsd/passthrough_ll.c b/tools/virtiofsd/passthrough_ll.c
> index 76ef891105..a6fa816b6c 100644
> --- a/tools/virtiofsd/passthrough_ll.c
> +++ b/tools/virtiofsd/passthrough_ll.c
> @@ -3209,7 +3209,10 @@ static void setup_root(struct lo_data *lo, struct lo_inode *root)
>      int fd, res;
>      struct stat stat;
>  
> -    fd = open("/", O_PATH);
> +    if (lo->sandbox == SANDBOX_NONE)
> +        fd = open(lo->source, O_PATH);
> +    else
> +        fd = open("/", O_PATH);

Up until now virtiofsd has been able to assume that path traversal has
the shared directory as "/".

Now this is no longer true and it is necessary to audit all syscalls
that take path arguments. They must ensure that:
1. Path components are safe (no ".." or "/" allowed)
2. Symlinks are not followed.

Did you audit all syscalls made by passthrough_ll.c?

virtiofsd still needs to restrict the client to the shared directory for
two reasons:
1. The guest may not be trusted. An unprivileged sandbox=none mount can
   be used with a malicious guest.
2. If accidental escapes are possible then the guest could accidentally
   corrupt or delete files outside the shared directory.

Stefan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]