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.ro...@huawei.com>
> Signed-off-by: Gan Qixin <ganqi...@huawei.com>
> ---
> Cc: Thomas Huth <th...@redhat.com>
> Cc: Laurent Vivier <lviv...@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 <lviv...@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
