| Message ID | 20210106050625.518041-1-ganqixin@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] qtest/libqtest: fix heap-buffer-overflow in qtest_cb_for_every_machine() | expand |
On 06/01/2021 06:06, Gan Qixin wrote: > When the length of mname is less than 5, memcpy("xenfv", mname, 5) will cause > heap buffer overflow. Therefore, use strncmp to avoid this problem. > > The asan showed stack: > > ERROR: AddressSanitizer: heap-buffer-overflow on address 0x60200000f2f4 at > pc 0x7f65d8cc2225 bp 0x7ffe93cc5a60 sp 0x7ffe93cc5208 READ of size 5 at > 0x60200000f2f4 thread T0 > #0 0x7f65d8cc2224 in memcmp (/lib64/libasan.so.5+0xdf224) > #1 0x5632c20be95b in qtest_cb_for_every_machine tests/qtest/libqtest.c:1282 > #2 0x5632c20b7995 in main tests/qtest/test-hmp.c:160 > #3 0x7f65d88fed42 in __libc_start_main (/lib64/libc.so.6+0x26d42) > #4 0x5632c20b72cd in _start (build/tests/qtest/test-hmp+0x542cd) > > Reported-by: Euler Robot <euler.robot@huawei.com> > Signed-off-by: Gan Qixin <ganqixin@huawei.com> > --- > Cc: Thomas Huth <thuth@redhat.com> > Cc: Laurent Vivier <lvivier@redhat.com> > > v2: > Changes suggested by Thomas Huth: > Replace memcmp(..., 5) with strncmp(..., 5). > --- > tests/qtest/libqtest.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c > index e49f3a1e45..e93424ffdd 100644 > --- a/tests/qtest/libqtest.c > +++ b/tests/qtest/libqtest.c > @@ -1281,7 +1281,7 @@ void qtest_cb_for_every_machine(void (*cb)(const char *machine), > g_assert(qstr); > mname = qstring_get_str(qstr); > /* Ignore machines that cannot be used for qtests */ > - if (!memcmp("xenfv", mname, 5) || g_str_equal("xenpv", mname)) { > + if (!strncmp("xenfv", mname, 5) || g_str_equal("xenpv", mname)) { > continue; > } > if (!skip_old_versioned || !qtest_is_old_versioned_machine(mname)) { > Reviewed-by: Laurent Vivier <lvivier@redhat.com> Problem seem to have been introduced originally by 9a709f06c870 ("piix: fix xenfv regression, add compat machine xenfv-4.2") Moved here by: 51b3ca975929 ("tests/qtest: Unify the test for the xenfv and xenpv machines" Thanks, Laurent
On 06/01/2021 06.06, Gan Qixin wrote: > When the length of mname is less than 5, memcpy("xenfv", mname, 5) will cause > heap buffer overflow. Therefore, use strncmp to avoid this problem. > > The asan showed stack: > > ERROR: AddressSanitizer: heap-buffer-overflow on address 0x60200000f2f4 at > pc 0x7f65d8cc2225 bp 0x7ffe93cc5a60 sp 0x7ffe93cc5208 READ of size 5 at > 0x60200000f2f4 thread T0 > #0 0x7f65d8cc2224 in memcmp (/lib64/libasan.so.5+0xdf224) > #1 0x5632c20be95b in qtest_cb_for_every_machine tests/qtest/libqtest.c:1282 > #2 0x5632c20b7995 in main tests/qtest/test-hmp.c:160 > #3 0x7f65d88fed42 in __libc_start_main (/lib64/libc.so.6+0x26d42) > #4 0x5632c20b72cd in _start (build/tests/qtest/test-hmp+0x542cd) > > Reported-by: Euler Robot <euler.robot@huawei.com> > Signed-off-by: Gan Qixin <ganqixin@huawei.com> > --- > Cc: Thomas Huth <thuth@redhat.com> > Cc: Laurent Vivier <lvivier@redhat.com> > > v2: > Changes suggested by Thomas Huth: > Replace memcmp(..., 5) with strncmp(..., 5). > --- > tests/qtest/libqtest.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Thanks, queued now to my qtest-next branch: https://gitlab.com/huth/qemu/-/commits/qtest-next/ Thomas
diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c index e49f3a1e45..e93424ffdd 100644 --- a/tests/qtest/libqtest.c +++ b/tests/qtest/libqtest.c @@ -1281,7 +1281,7 @@ void qtest_cb_for_every_machine(void (*cb)(const char *machine), g_assert(qstr); mname = qstring_get_str(qstr); /* Ignore machines that cannot be used for qtests */ - if (!memcmp("xenfv", mname, 5) || g_str_equal("xenpv", mname)) { + if (!strncmp("xenfv", mname, 5) || g_str_equal("xenpv", mname)) { continue; } if (!skip_old_versioned || !qtest_is_old_versioned_machine(mname)) {
When the length of mname is less than 5, memcpy("xenfv", mname, 5) will cause heap buffer overflow. Therefore, use strncmp to avoid this problem. The asan showed stack: ERROR: AddressSanitizer: heap-buffer-overflow on address 0x60200000f2f4 at pc 0x7f65d8cc2225 bp 0x7ffe93cc5a60 sp 0x7ffe93cc5208 READ of size 5 at 0x60200000f2f4 thread T0 #0 0x7f65d8cc2224 in memcmp (/lib64/libasan.so.5+0xdf224) #1 0x5632c20be95b in qtest_cb_for_every_machine tests/qtest/libqtest.c:1282 #2 0x5632c20b7995 in main tests/qtest/test-hmp.c:160 #3 0x7f65d88fed42 in __libc_start_main (/lib64/libc.so.6+0x26d42) #4 0x5632c20b72cd in _start (build/tests/qtest/test-hmp+0x542cd) Reported-by: Euler Robot <euler.robot@huawei.com> Signed-off-by: Gan Qixin <ganqixin@huawei.com> --- Cc: Thomas Huth <thuth@redhat.com> Cc: Laurent Vivier <lvivier@redhat.com> v2: Changes suggested by Thomas Huth: Replace memcmp(..., 5) with strncmp(..., 5). --- tests/qtest/libqtest.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)