* [Qemu-devel] [PATCH] xen/pt: Set is_express to avoid out-of-bounds write
@ 2017-10-28 2:53 ` Simon Gaiser
0 siblings, 0 replies; 4+ messages in thread
From: Simon Gaiser @ 2017-10-28 2:53 UTC (permalink / raw)
To: qemu-devel; +Cc: Simon Gaiser, Stefano Stabellini, Anthony Perard, xen-devel
The passed-through device might be an express device. In this case the
old code allocated a too small emulated config space in
pci_config_alloc() since pci_config_size() returned the size for a
non-express device. This leads to an out-of-bound write in
xen_pt_config_reg_init(), which sometimes results in crashes. So set
is_express as already done for KVM in vfio-pci.
Shortened ASan report:
==17512==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x611000041648 at pc 0x55e0fdac51ff bp 0x7ffe4af07410 sp 0x7ffe4af07408
WRITE of size 2 at 0x611000041648 thread T0
#0 0x55e0fdac51fe in memcpy /usr/include/x86_64-linux-gnu/bits/string3.h:53
#1 0x55e0fdac51fe in stw_he_p include/qemu/bswap.h:330
#2 0x55e0fdac51fe in stw_le_p include/qemu/bswap.h:379
#3 0x55e0fdac51fe in pci_set_word include/hw/pci/pci.h:490
#4 0x55e0fdac51fe in xen_pt_config_reg_init hw/xen/xen_pt_config_init.c:1991
#5 0x55e0fdac51fe in xen_pt_config_init hw/xen/xen_pt_config_init.c:2067
#6 0x55e0fdabcf4d in xen_pt_realize hw/xen/xen_pt.c:830
#7 0x55e0fdf59666 in pci_qdev_realize hw/pci/pci.c:2034
#8 0x55e0fdda7d3d in device_set_realized hw/core/qdev.c:914
[...]
0x611000041648 is located 8 bytes to the right of 256-byte region [0x611000041540,0x611000041640)
allocated by thread T0 here:
#0 0x7ff596a94bb8 in __interceptor_calloc (/usr/lib/x86_64-linux-gnu/libasan.so.4+0xd9bb8)
#1 0x7ff57da66580 in g_malloc0 (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x50580)
#2 0x55e0fdda7d3d in device_set_realized hw/core/qdev.c:914
[...]
Signed-off-by: Simon Gaiser <hw42@ipsumj.de>
---
I found this by debugging crashes and I'm not familiar with this code,
so I'm not sure if this has no unintended side effects.
hw/xen/xen_pt.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/hw/xen/xen_pt.c b/hw/xen/xen_pt.c
index b6d71bb52a..90ffd45e7d 100644
--- a/hw/xen/xen_pt.c
+++ b/hw/xen/xen_pt.c
@@ -946,6 +946,7 @@ static void xen_pci_passthrough_class_init(ObjectClass *klass, void *data)
k->exit = xen_pt_unregister_device;
k->config_read = xen_pt_pci_read_config;
k->config_write = xen_pt_pci_write_config;
+ k->is_express = 1; /* We might be */
set_bit(DEVICE_CATEGORY_MISC, dc->categories);
dc->desc = "Assign an host PCI device with Xen";
dc->props = xen_pci_passthrough_properties;
--
2.15.0.rc1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH] xen/pt: Set is_express to avoid out-of-bounds write
@ 2017-10-28 2:53 ` Simon Gaiser
0 siblings, 0 replies; 4+ messages in thread
From: Simon Gaiser @ 2017-10-28 2:53 UTC (permalink / raw)
To: qemu-devel; +Cc: Anthony Perard, xen-devel, Simon Gaiser, Stefano Stabellini
The passed-through device might be an express device. In this case the
old code allocated a too small emulated config space in
pci_config_alloc() since pci_config_size() returned the size for a
non-express device. This leads to an out-of-bound write in
xen_pt_config_reg_init(), which sometimes results in crashes. So set
is_express as already done for KVM in vfio-pci.
Shortened ASan report:
==17512==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x611000041648 at pc 0x55e0fdac51ff bp 0x7ffe4af07410 sp 0x7ffe4af07408
WRITE of size 2 at 0x611000041648 thread T0
#0 0x55e0fdac51fe in memcpy /usr/include/x86_64-linux-gnu/bits/string3.h:53
#1 0x55e0fdac51fe in stw_he_p include/qemu/bswap.h:330
#2 0x55e0fdac51fe in stw_le_p include/qemu/bswap.h:379
#3 0x55e0fdac51fe in pci_set_word include/hw/pci/pci.h:490
#4 0x55e0fdac51fe in xen_pt_config_reg_init hw/xen/xen_pt_config_init.c:1991
#5 0x55e0fdac51fe in xen_pt_config_init hw/xen/xen_pt_config_init.c:2067
#6 0x55e0fdabcf4d in xen_pt_realize hw/xen/xen_pt.c:830
#7 0x55e0fdf59666 in pci_qdev_realize hw/pci/pci.c:2034
#8 0x55e0fdda7d3d in device_set_realized hw/core/qdev.c:914
[...]
0x611000041648 is located 8 bytes to the right of 256-byte region [0x611000041540,0x611000041640)
allocated by thread T0 here:
#0 0x7ff596a94bb8 in __interceptor_calloc (/usr/lib/x86_64-linux-gnu/libasan.so.4+0xd9bb8)
#1 0x7ff57da66580 in g_malloc0 (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x50580)
#2 0x55e0fdda7d3d in device_set_realized hw/core/qdev.c:914
[...]
Signed-off-by: Simon Gaiser <hw42@ipsumj.de>
---
I found this by debugging crashes and I'm not familiar with this code,
so I'm not sure if this has no unintended side effects.
hw/xen/xen_pt.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/hw/xen/xen_pt.c b/hw/xen/xen_pt.c
index b6d71bb52a..90ffd45e7d 100644
--- a/hw/xen/xen_pt.c
+++ b/hw/xen/xen_pt.c
@@ -946,6 +946,7 @@ static void xen_pci_passthrough_class_init(ObjectClass *klass, void *data)
k->exit = xen_pt_unregister_device;
k->config_read = xen_pt_pci_read_config;
k->config_write = xen_pt_pci_write_config;
+ k->is_express = 1; /* We might be */
set_bit(DEVICE_CATEGORY_MISC, dc->categories);
dc->desc = "Assign an host PCI device with Xen";
dc->props = xen_pci_passthrough_properties;
--
2.15.0.rc1
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [PATCH] xen/pt: Set is_express to avoid out-of-bounds write
2017-10-28 2:53 ` Simon Gaiser
@ 2017-12-08 22:53 ` Stefano Stabellini
-1 siblings, 0 replies; 4+ messages in thread
From: Stefano Stabellini @ 2017-12-08 22:53 UTC (permalink / raw)
To: Simon Gaiser; +Cc: qemu-devel, Anthony Perard, xen-devel, Stefano Stabellini
On Sat, 28 Oct 2017, Simon Gaiser wrote:
> The passed-through device might be an express device. In this case the
> old code allocated a too small emulated config space in
> pci_config_alloc() since pci_config_size() returned the size for a
> non-express device. This leads to an out-of-bound write in
> xen_pt_config_reg_init(), which sometimes results in crashes. So set
> is_express as already done for KVM in vfio-pci.
>
> Shortened ASan report:
>
> ==17512==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x611000041648 at pc 0x55e0fdac51ff bp 0x7ffe4af07410 sp 0x7ffe4af07408
> WRITE of size 2 at 0x611000041648 thread T0
> #0 0x55e0fdac51fe in memcpy /usr/include/x86_64-linux-gnu/bits/string3.h:53
> #1 0x55e0fdac51fe in stw_he_p include/qemu/bswap.h:330
> #2 0x55e0fdac51fe in stw_le_p include/qemu/bswap.h:379
> #3 0x55e0fdac51fe in pci_set_word include/hw/pci/pci.h:490
> #4 0x55e0fdac51fe in xen_pt_config_reg_init hw/xen/xen_pt_config_init.c:1991
> #5 0x55e0fdac51fe in xen_pt_config_init hw/xen/xen_pt_config_init.c:2067
> #6 0x55e0fdabcf4d in xen_pt_realize hw/xen/xen_pt.c:830
> #7 0x55e0fdf59666 in pci_qdev_realize hw/pci/pci.c:2034
> #8 0x55e0fdda7d3d in device_set_realized hw/core/qdev.c:914
> [...]
>
> 0x611000041648 is located 8 bytes to the right of 256-byte region [0x611000041540,0x611000041640)
> allocated by thread T0 here:
> #0 0x7ff596a94bb8 in __interceptor_calloc (/usr/lib/x86_64-linux-gnu/libasan.so.4+0xd9bb8)
> #1 0x7ff57da66580 in g_malloc0 (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x50580)
> #2 0x55e0fdda7d3d in device_set_realized hw/core/qdev.c:914
> [...]
>
> Signed-off-by: Simon Gaiser <hw42@ipsumj.de>
Acked-by: Stefano Stabellini <sstabellini@kernel.org>
> ---
>
> I found this by debugging crashes and I'm not familiar with this code,
> so I'm not sure if this has no unintended side effects.
>
> hw/xen/xen_pt.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/hw/xen/xen_pt.c b/hw/xen/xen_pt.c
> index b6d71bb52a..90ffd45e7d 100644
> --- a/hw/xen/xen_pt.c
> +++ b/hw/xen/xen_pt.c
> @@ -946,6 +946,7 @@ static void xen_pci_passthrough_class_init(ObjectClass *klass, void *data)
> k->exit = xen_pt_unregister_device;
> k->config_read = xen_pt_pci_read_config;
> k->config_write = xen_pt_pci_write_config;
> + k->is_express = 1; /* We might be */
> set_bit(DEVICE_CATEGORY_MISC, dc->categories);
> dc->desc = "Assign an host PCI device with Xen";
> dc->props = xen_pci_passthrough_properties;
> --
> 2.15.0.rc1
>
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [PATCH] xen/pt: Set is_express to avoid out-of-bounds write
@ 2017-12-08 22:53 ` Stefano Stabellini
0 siblings, 0 replies; 4+ messages in thread
From: Stefano Stabellini @ 2017-12-08 22:53 UTC (permalink / raw)
To: Simon Gaiser; +Cc: Anthony Perard, xen-devel, Stefano Stabellini, qemu-devel
On Sat, 28 Oct 2017, Simon Gaiser wrote:
> The passed-through device might be an express device. In this case the
> old code allocated a too small emulated config space in
> pci_config_alloc() since pci_config_size() returned the size for a
> non-express device. This leads to an out-of-bound write in
> xen_pt_config_reg_init(), which sometimes results in crashes. So set
> is_express as already done for KVM in vfio-pci.
>
> Shortened ASan report:
>
> ==17512==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x611000041648 at pc 0x55e0fdac51ff bp 0x7ffe4af07410 sp 0x7ffe4af07408
> WRITE of size 2 at 0x611000041648 thread T0
> #0 0x55e0fdac51fe in memcpy /usr/include/x86_64-linux-gnu/bits/string3.h:53
> #1 0x55e0fdac51fe in stw_he_p include/qemu/bswap.h:330
> #2 0x55e0fdac51fe in stw_le_p include/qemu/bswap.h:379
> #3 0x55e0fdac51fe in pci_set_word include/hw/pci/pci.h:490
> #4 0x55e0fdac51fe in xen_pt_config_reg_init hw/xen/xen_pt_config_init.c:1991
> #5 0x55e0fdac51fe in xen_pt_config_init hw/xen/xen_pt_config_init.c:2067
> #6 0x55e0fdabcf4d in xen_pt_realize hw/xen/xen_pt.c:830
> #7 0x55e0fdf59666 in pci_qdev_realize hw/pci/pci.c:2034
> #8 0x55e0fdda7d3d in device_set_realized hw/core/qdev.c:914
> [...]
>
> 0x611000041648 is located 8 bytes to the right of 256-byte region [0x611000041540,0x611000041640)
> allocated by thread T0 here:
> #0 0x7ff596a94bb8 in __interceptor_calloc (/usr/lib/x86_64-linux-gnu/libasan.so.4+0xd9bb8)
> #1 0x7ff57da66580 in g_malloc0 (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x50580)
> #2 0x55e0fdda7d3d in device_set_realized hw/core/qdev.c:914
> [...]
>
> Signed-off-by: Simon Gaiser <hw42@ipsumj.de>
Acked-by: Stefano Stabellini <sstabellini@kernel.org>
> ---
>
> I found this by debugging crashes and I'm not familiar with this code,
> so I'm not sure if this has no unintended side effects.
>
> hw/xen/xen_pt.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/hw/xen/xen_pt.c b/hw/xen/xen_pt.c
> index b6d71bb52a..90ffd45e7d 100644
> --- a/hw/xen/xen_pt.c
> +++ b/hw/xen/xen_pt.c
> @@ -946,6 +946,7 @@ static void xen_pci_passthrough_class_init(ObjectClass *klass, void *data)
> k->exit = xen_pt_unregister_device;
> k->config_read = xen_pt_pci_read_config;
> k->config_write = xen_pt_pci_write_config;
> + k->is_express = 1; /* We might be */
> set_bit(DEVICE_CATEGORY_MISC, dc->categories);
> dc->desc = "Assign an host PCI device with Xen";
> dc->props = xen_pci_passthrough_properties;
> --
> 2.15.0.rc1
>
>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2017-12-08 22:53 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-10-28 2:53 [Qemu-devel] [PATCH] xen/pt: Set is_express to avoid out-of-bounds write Simon Gaiser
2017-10-28 2:53 ` Simon Gaiser
2017-12-08 22:53 ` [Qemu-devel] " Stefano Stabellini
2017-12-08 22:53 ` Stefano Stabellini
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.