Re: [Qemu-devel] [PATCH 1/2] 9pfs: fix information leak in xattr read

[Greg Kurz <groug@kaod.org>]

On Sat,  8 Oct 2016 22:26:51 -0700
Li Qiang <liq3ea@gmail.com> wrote:

> From: Li Qiang <liqiang6-s@360.cn>
> 
> 9pfs uses g_malloc() to allocate the xattr memory space, if the guest
> reads this memory before writing to it, this will leak host heap memory
> to the guest. This patch avoid this.
> 
> Signed-off-by: Li Qiang <liqiang6-s@360.cn>
> ---

I've looked again and we could theorically defer allocation until
v9fs_xattr_write() is called, and only allow v9fs_xattr_read() to
return bytes previously written by the client. But this would
result in rather complex code to handle partial writes and reads.

So this patch looks like the way to go.

Reviewed-by: Greg Kurz <groug@kaod.org>

>  hw/9pfs/9p.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
> index 119ee58..8751c19 100644
> --- a/hw/9pfs/9p.c
> +++ b/hw/9pfs/9p.c
> @@ -3282,7 +3282,7 @@ static void v9fs_xattrcreate(void *opaque)
>      xattr_fidp->fs.xattr.flags = flags;
>      v9fs_string_init(&xattr_fidp->fs.xattr.name);
>      v9fs_string_copy(&xattr_fidp->fs.xattr.name, &name);
> -    xattr_fidp->fs.xattr.value = g_malloc(size);
> +    xattr_fidp->fs.xattr.value = g_malloc0(size);
>      err = offset;
>      put_fid(pdu, file_fidp);
>  out_nofid: