Re: [Qemu-devel] [PATCH] hw/9pfs: use O_NOFOLLOW for mapped readlink operation

["Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>]

"Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com> writes:

> From: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>
>
> With mapped security models like mapped-xattr and mapped-file, we save the
> symlink target as file contents. Now if we ever expose a normal directory
> with mapped security model and find real symlinks in export path, never
> follow them and return proper error.
>
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
> ---
>  hw/9pfs/virtio-9p-local.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/9pfs/virtio-9p-local.c b/hw/9pfs/virtio-9p-local.c
> index 6ece6f7..87aa75d 100644
> --- a/hw/9pfs/virtio-9p-local.c
> +++ b/hw/9pfs/virtio-9p-local.c
> @@ -284,7 +284,7 @@ static ssize_t local_readlink(FsContext *fs_ctx, V9fsPath *fs_path,
>      if ((fs_ctx->export_flags & V9FS_SM_MAPPED) ||
>          (fs_ctx->export_flags & V9FS_SM_MAPPED_FILE)) {
>          int fd;
> -        fd = open(rpath(fs_ctx, path, buffer), O_RDONLY);
> +        fd = open(rpath(fs_ctx, path, buffer), O_RDONLY | O_NOFOLLOW);
>          if (fd == -1) {
>              return -1;
>          }

We may want to apply this to stable, considering that the existing
code can be used to show contents of file outside export path. So if we
use the security model pass-through and create a symlink in guest
pointing to some file like /etc/file-not-allowed-to-read, with
pass-through, the /etc/file-not-allowed-to-read will resolve within
guest. Now if we expose the same export path via mapped-file security
model, we will consider the content of the link as link target. But
when we open link in mapped-file model, we didn't use O_NOFOLLOW, so
we will follow the link in the host and consider the content of
/etc/file-not-allowed-to-read as the link target, there by making the
content visible to guest.

I have another patch that add O_NOFOLLOW to all open(2) calls. But that
would require wider testing before posting.

-aneesh