* [Qemu-devel] [PATCH] spapr: fix buffer-overflow
@ 2017-03-23 10:04 Marc-André Lureau
2017-03-24 2:26 ` David Gibson
0 siblings, 1 reply; 2+ messages in thread
From: Marc-André Lureau @ 2017-03-23 10:04 UTC (permalink / raw)
To: qemu-devel; +Cc: david, agraf, Marc-André Lureau
Running postcopy-test with ASAN produces the following error:
QTEST_QEMU_BINARY=ppc64-softmmu/qemu-system-ppc64 tests/postcopy-test
...
=================================================================
==23641==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x7f1556600000 at pc 0x55b8e9d28208 bp 0x7f1555f4d3c0 sp 0x7f1555f4d3b0
READ of size 8 at 0x7f1556600000 thread T6
#0 0x55b8e9d28207 in htab_save_first_pass /home/elmarco/src/qq/hw/ppc/spapr.c:1528
#1 0x55b8e9d2939c in htab_save_iterate /home/elmarco/src/qq/hw/ppc/spapr.c:1665
#2 0x55b8e9beae3a in qemu_savevm_state_iterate /home/elmarco/src/qq/migration/savevm.c:1044
#3 0x55b8ea677733 in migration_thread /home/elmarco/src/qq/migration/migration.c:1976
#4 0x7f15845f46c9 in start_thread (/lib64/libpthread.so.0+0x76c9)
#5 0x7f157d9d0f7e in clone (/lib64/libc.so.6+0x107f7e)
0x7f1556600000 is located 0 bytes to the right of 2097152-byte region [0x7f1556400000,0x7f1556600000)
allocated by thread T0 here:
#0 0x7f159bb76980 in posix_memalign (/lib64/libasan.so.3+0xc7980)
#1 0x55b8eab185b2 in qemu_try_memalign /home/elmarco/src/qq/util/oslib-posix.c:106
#2 0x55b8eab186c8 in qemu_memalign /home/elmarco/src/qq/util/oslib-posix.c:122
#3 0x55b8e9d268a8 in spapr_reallocate_hpt /home/elmarco/src/qq/hw/ppc/spapr.c:1214
#4 0x55b8e9d26e04 in ppc_spapr_reset /home/elmarco/src/qq/hw/ppc/spapr.c:1261
#5 0x55b8ea12e913 in qemu_system_reset /home/elmarco/src/qq/vl.c:1697
#6 0x55b8ea13fa40 in main /home/elmarco/src/qq/vl.c:4679
#7 0x7f157d8e9400 in __libc_start_main (/lib64/libc.so.6+0x20400)
Thread T6 created by T0 here:
#0 0x7f159bae0488 in __interceptor_pthread_create (/lib64/libasan.so.3+0x31488)
#1 0x55b8eab1d9cb in qemu_thread_create /home/elmarco/src/qq/util/qemu-thread-posix.c:465
#2 0x55b8ea67874c in migrate_fd_connect /home/elmarco/src/qq/migration/migration.c:2096
#3 0x55b8ea66cbb0 in migration_channel_connect /home/elmarco/src/qq/migration/migration.c:500
#4 0x55b8ea678f38 in socket_outgoing_migration /home/elmarco/src/qq/migration/socket.c:87
#5 0x55b8eaa5a03a in qio_task_complete /home/elmarco/src/qq/io/task.c:142
#6 0x55b8eaa599cc in gio_task_thread_result /home/elmarco/src/qq/io/task.c:88
#7 0x7f15823e38e6 (/lib64/libglib-2.0.so.0+0x468e6)
SUMMARY: AddressSanitizer: heap-buffer-overflow /home/elmarco/src/qq/hw/ppc/spapr.c:1528 in htab_save_first_pass
index seems to be wrongly incremented, unless I miss something that
would be worth a comment.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
hw/ppc/spapr.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 6ee566d658..63439811d5 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -1524,16 +1524,16 @@ static void htab_save_first_pass(QEMUFile *f, sPAPRMachineState *spapr,
/* Consume invalid HPTEs */
while ((index < htabslots)
&& !HPTE_VALID(HPTE(spapr->htab, index))) {
- index++;
CLEAN_HPTE(HPTE(spapr->htab, index));
+ index++;
}
/* Consume valid HPTEs */
chunkstart = index;
while ((index < htabslots) && (index - chunkstart < USHRT_MAX)
&& HPTE_VALID(HPTE(spapr->htab, index))) {
- index++;
CLEAN_HPTE(HPTE(spapr->htab, index));
+ index++;
}
if (index > chunkstart) {
--
2.12.0.191.gc5d8de91d
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [Qemu-devel] [PATCH] spapr: fix buffer-overflow
2017-03-23 10:04 [Qemu-devel] [PATCH] spapr: fix buffer-overflow Marc-André Lureau
@ 2017-03-24 2:26 ` David Gibson
0 siblings, 0 replies; 2+ messages in thread
From: David Gibson @ 2017-03-24 2:26 UTC (permalink / raw)
To: Marc-André Lureau; +Cc: qemu-devel, agraf
[-- Attachment #1: Type: text/plain, Size: 4198 bytes --]
On Thu, Mar 23, 2017 at 02:04:55PM +0400, Marc-André Lureau wrote:
> Running postcopy-test with ASAN produces the following error:
>
> QTEST_QEMU_BINARY=ppc64-softmmu/qemu-system-ppc64 tests/postcopy-test
> ...
> =================================================================
> ==23641==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x7f1556600000 at pc 0x55b8e9d28208 bp 0x7f1555f4d3c0 sp 0x7f1555f4d3b0
> READ of size 8 at 0x7f1556600000 thread T6
> #0 0x55b8e9d28207 in htab_save_first_pass /home/elmarco/src/qq/hw/ppc/spapr.c:1528
> #1 0x55b8e9d2939c in htab_save_iterate /home/elmarco/src/qq/hw/ppc/spapr.c:1665
> #2 0x55b8e9beae3a in qemu_savevm_state_iterate /home/elmarco/src/qq/migration/savevm.c:1044
> #3 0x55b8ea677733 in migration_thread /home/elmarco/src/qq/migration/migration.c:1976
> #4 0x7f15845f46c9 in start_thread (/lib64/libpthread.so.0+0x76c9)
> #5 0x7f157d9d0f7e in clone (/lib64/libc.so.6+0x107f7e)
>
> 0x7f1556600000 is located 0 bytes to the right of 2097152-byte region [0x7f1556400000,0x7f1556600000)
> allocated by thread T0 here:
> #0 0x7f159bb76980 in posix_memalign (/lib64/libasan.so.3+0xc7980)
> #1 0x55b8eab185b2 in qemu_try_memalign /home/elmarco/src/qq/util/oslib-posix.c:106
> #2 0x55b8eab186c8 in qemu_memalign /home/elmarco/src/qq/util/oslib-posix.c:122
> #3 0x55b8e9d268a8 in spapr_reallocate_hpt /home/elmarco/src/qq/hw/ppc/spapr.c:1214
> #4 0x55b8e9d26e04 in ppc_spapr_reset /home/elmarco/src/qq/hw/ppc/spapr.c:1261
> #5 0x55b8ea12e913 in qemu_system_reset /home/elmarco/src/qq/vl.c:1697
> #6 0x55b8ea13fa40 in main /home/elmarco/src/qq/vl.c:4679
> #7 0x7f157d8e9400 in __libc_start_main (/lib64/libc.so.6+0x20400)
>
> Thread T6 created by T0 here:
> #0 0x7f159bae0488 in __interceptor_pthread_create (/lib64/libasan.so.3+0x31488)
> #1 0x55b8eab1d9cb in qemu_thread_create /home/elmarco/src/qq/util/qemu-thread-posix.c:465
> #2 0x55b8ea67874c in migrate_fd_connect /home/elmarco/src/qq/migration/migration.c:2096
> #3 0x55b8ea66cbb0 in migration_channel_connect /home/elmarco/src/qq/migration/migration.c:500
> #4 0x55b8ea678f38 in socket_outgoing_migration /home/elmarco/src/qq/migration/socket.c:87
> #5 0x55b8eaa5a03a in qio_task_complete /home/elmarco/src/qq/io/task.c:142
> #6 0x55b8eaa599cc in gio_task_thread_result /home/elmarco/src/qq/io/task.c:88
> #7 0x7f15823e38e6 (/lib64/libglib-2.0.so.0+0x468e6)
> SUMMARY: AddressSanitizer: heap-buffer-overflow /home/elmarco/src/qq/hw/ppc/spapr.c:1528 in htab_save_first_pass
>
> index seems to be wrongly incremented, unless I miss something that
> would be worth a comment.
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Oh my. That's an impressively embarrassing bug. As well as the
overrun itself, we're marking the wrong HPTEs clean. I'm kind of
surprised we haven't seen this bite already. Anyway, I've applied to
ppc-for-2.9, and I'll send a pull request ASAP.
> ---
> hw/ppc/spapr.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 6ee566d658..63439811d5 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -1524,16 +1524,16 @@ static void htab_save_first_pass(QEMUFile *f, sPAPRMachineState *spapr,
> /* Consume invalid HPTEs */
> while ((index < htabslots)
> && !HPTE_VALID(HPTE(spapr->htab, index))) {
> - index++;
> CLEAN_HPTE(HPTE(spapr->htab, index));
> + index++;
> }
>
> /* Consume valid HPTEs */
> chunkstart = index;
> while ((index < htabslots) && (index - chunkstart < USHRT_MAX)
> && HPTE_VALID(HPTE(spapr->htab, index))) {
> - index++;
> CLEAN_HPTE(HPTE(spapr->htab, index));
> + index++;
> }
>
> if (index > chunkstart) {
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2017-03-24 5:53 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-03-23 10:04 [Qemu-devel] [PATCH] spapr: fix buffer-overflow Marc-André Lureau
2017-03-24 2:26 ` David Gibson
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).