9pfs: fix information leak in xattr read
PATCH 1/1 9pfs: fix information leak in xattr read
From: Li Qiang <[email protected]>
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.
--- hw/9pfs/9p.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
hw/9pfs/9p.c +1 -1
@@ -3291,7 +3291,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:
Signature · 1 line
1.8.3.1
Hi Greg,
What about this patch?
It seems you have forget it.
2016-09-27 12:44 GMT+08:00 Li Qiang <[email protected]>:
Quoted 28 lines
From: Li Qiang <[email protected]>
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 <[email protected]>--- hw/9pfs/9p.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)hw/9pfs/9p.c +1
-2@@ -3291,7 +3291,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: --1.8.3.1
Quoted 6 lines · On Wed, 28 Sep 2016 14:42:05 +0800 Li Qiang <[email protected]> wrote:
Hi Greg,
What about this patch?
It seems you have forget it.
Heh not exactly... I'm just wondering why we allocate xattr_fidp->fs.xattr.value here if we don't use it. Can you please investigate if it is possible to defer allocation until it is really needed ?
Thanks.
--
Greg
Quoted 30 lines
2016-09-27 12:44 GMT+08:00 Li Qiang <[email protected]>:
Quoted 28 lines
From: Li Qiang <[email protected]>
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 <[email protected]>--- hw/9pfs/9p.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)hw/9pfs/9p.c +1
-2@@ -3291,7 +3291,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: --1.8.3.1
Yes, allocation here is also causing a memory leak issue. I will try to look more at it.
Quoted 45 lines · On 2016-09-28 16:47 GMT+08:00 Greg Kurz <[email protected]> wrote:
Heh not exactly... I'm just wondering why we allocate
xattr_fidp->fs.xattr.value
here if we don't use it. Can you please investigate if it is possible to defer
allocation until it is really needed ?Thanks.
--
GregQuoted 30 lines
2016-09-27 12:44 GMT+08:00 Li Qiang <[email protected]>:
Quoted 28 lines
From: Li Qiang <[email protected]>
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 <[email protected]>--- hw/9pfs/9p.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)hw/9pfs/9p.c +1
-2@@ -3291,7 +3291,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: --1.8.3.1