* [PATCH 1/2] qemu-img: Fix amend option parse error handling
2025-10-23 8:10 [PATCH 0/2] iotests: Run iotests with sanitizers Akihiko Odaki
@ 2025-10-23 8:10 ` Akihiko Odaki
2025-10-23 8:10 ` [PATCH 2/2] iotests: Run iotests with sanitizers Akihiko Odaki
2025-11-04 16:36 ` [PATCH 0/2] " Kevin Wolf
2 siblings, 0 replies; 5+ messages in thread
From: Akihiko Odaki @ 2025-10-23 8:10 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Hanna Reitz, qemu-block, Akihiko Odaki
qemu_opts_del(opts) dereferences opts->list, which is the old amend_opts
pointer that can be dangling after executing
qemu_opts_append(amend_opts, bs->drv->create_opts) and cause
use-after-free.
Fix the potential use-after-free by moving the qemu_opts_del() call
before the qemu_opts_append() call.
Signed-off-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
---
qemu-img.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/qemu-img.c b/qemu-img.c
index 7a162fdc08d3..63961e2b76f0 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -4571,9 +4571,9 @@ static int img_amend(const img_cmd_t *ccmd, int argc, char **argv)
amend_opts = qemu_opts_append(amend_opts, bs->drv->amend_opts);
opts = qemu_opts_create(amend_opts, NULL, 0, &error_abort);
if (!qemu_opts_do_parse(opts, options, NULL, &err)) {
+ qemu_opts_del(opts);
/* Try to parse options using the create options */
amend_opts = qemu_opts_append(amend_opts, bs->drv->create_opts);
- qemu_opts_del(opts);
opts = qemu_opts_create(amend_opts, NULL, 0, &error_abort);
if (qemu_opts_do_parse(opts, options, NULL, NULL)) {
error_append_hint(&err,
--
2.51.0
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH 2/2] iotests: Run iotests with sanitizers
2025-10-23 8:10 [PATCH 0/2] iotests: Run iotests with sanitizers Akihiko Odaki
2025-10-23 8:10 ` [PATCH 1/2] qemu-img: Fix amend option parse error handling Akihiko Odaki
@ 2025-10-23 8:10 ` Akihiko Odaki
2025-10-23 8:16 ` Philippe Mathieu-Daudé
2025-11-04 16:36 ` [PATCH 0/2] " Kevin Wolf
2 siblings, 1 reply; 5+ messages in thread
From: Akihiko Odaki @ 2025-10-23 8:10 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Hanna Reitz, qemu-block, Akihiko Odaki
Commit 2cc4d1c5eab1 ("tests/check-block: Skip iotests when sanitizers
are enabled") changed iotests to skip when sanitizers are enabled.
The rationale is that AddressSanitizer emits warnings and reports leaks,
which results in test breakage. Later, sanitizers that are enabled for
production environments (safe-stack and cfi-icall) were exempted.
However, this approach has a few problems.
- It requires rebuild to disable sanitizers if the existing build has
them enabled.
- It disables other useful non-production sanitizers.
- The exemption of safe-stack and cfi-icall is not correctly
implemented, so qemu-iotests are incorrectly enabled whenever either
safe-stack or cfi-icall is enabled *and*, even if there is another
sanitizer like AddressSanitizer.
To solve these problems, direct AddressSanitizer warnings to separate
files to avoid changing the test results, and selectively disable
leak detection at runtime instead of requiring to disable all
sanitizers at buildtime.
Signed-off-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
---
tests/qemu-iotests/meson.build | 8 --------
tests/qemu-iotests/testrunner.py | 12 ++++++++++++
2 files changed, 12 insertions(+), 8 deletions(-)
diff --git a/tests/qemu-iotests/meson.build b/tests/qemu-iotests/meson.build
index fad340ad5957..56b04468274a 100644
--- a/tests/qemu-iotests/meson.build
+++ b/tests/qemu-iotests/meson.build
@@ -2,14 +2,6 @@ if not have_tools or host_os == 'windows'
subdir_done()
endif
-foreach cflag: qemu_ldflags
- if cflag.startswith('-fsanitize') and \
- not cflag.contains('safe-stack') and not cflag.contains('cfi-icall')
- message('Sanitizers are enabled ==> Disabled the qemu-iotests.')
- subdir_done()
- endif
-endforeach
-
bash = find_program('bash', required: false, version: '>= 4.0')
if not bash.found()
message('bash >= v4.0 not available ==> Disabled the qemu-iotests.')
diff --git a/tests/qemu-iotests/testrunner.py b/tests/qemu-iotests/testrunner.py
index 14cc8492f9fb..e2a365899414 100644
--- a/tests/qemu-iotests/testrunner.py
+++ b/tests/qemu-iotests/testrunner.py
@@ -263,10 +263,21 @@ def do_run_test(self, test: str) -> TestResult:
Path(env[d]).mkdir(parents=True, exist_ok=True)
test_dir = env['TEST_DIR']
+ f_asan = Path(test_dir, f_test.name + '.out.asan')
f_bad = Path(test_dir, f_test.name + '.out.bad')
f_notrun = Path(test_dir, f_test.name + '.notrun')
f_casenotrun = Path(test_dir, f_test.name + '.casenotrun')
+ env['ASAN_OPTIONS'] = f'detect_leaks=0:log_path={f_asan}'
+
+ def unlink_asan():
+ with os.scandir(test_dir) as it:
+ for entry in it:
+ if entry.name.startswith(f_asan.name):
+ os.unlink(entry)
+
+ unlink_asan()
+
for p in (f_notrun, f_casenotrun):
silent_unlink(p)
@@ -312,6 +323,7 @@ def do_run_test(self, test: str) -> TestResult:
description=f'output mismatch (see {f_bad})',
diff=diff, casenotrun=casenotrun)
else:
+ unlink_asan()
f_bad.unlink()
return TestResult(status='pass', elapsed=elapsed,
casenotrun=casenotrun)
--
2.51.0
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH 2/2] iotests: Run iotests with sanitizers
2025-10-23 8:10 ` [PATCH 2/2] iotests: Run iotests with sanitizers Akihiko Odaki
@ 2025-10-23 8:16 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 5+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-10-23 8:16 UTC (permalink / raw)
To: Akihiko Odaki, qemu-devel; +Cc: Kevin Wolf, Hanna Reitz, qemu-block
On 23/10/25 10:10, Akihiko Odaki wrote:
> Commit 2cc4d1c5eab1 ("tests/check-block: Skip iotests when sanitizers
> are enabled") changed iotests to skip when sanitizers are enabled.
> The rationale is that AddressSanitizer emits warnings and reports leaks,
> which results in test breakage. Later, sanitizers that are enabled for
> production environments (safe-stack and cfi-icall) were exempted.
>
> However, this approach has a few problems.
>
> - It requires rebuild to disable sanitizers if the existing build has
> them enabled.
> - It disables other useful non-production sanitizers.
> - The exemption of safe-stack and cfi-icall is not correctly
> implemented, so qemu-iotests are incorrectly enabled whenever either
> safe-stack or cfi-icall is enabled *and*, even if there is another
> sanitizer like AddressSanitizer.
>
> To solve these problems, direct AddressSanitizer warnings to separate
> files to avoid changing the test results, and selectively disable
> leak detection at runtime instead of requiring to disable all
> sanitizers at buildtime.
>
> Signed-off-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
> ---
> tests/qemu-iotests/meson.build | 8 --------
> tests/qemu-iotests/testrunner.py | 12 ++++++++++++
> 2 files changed, 12 insertions(+), 8 deletions(-)
Nice.
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 0/2] iotests: Run iotests with sanitizers
2025-10-23 8:10 [PATCH 0/2] iotests: Run iotests with sanitizers Akihiko Odaki
2025-10-23 8:10 ` [PATCH 1/2] qemu-img: Fix amend option parse error handling Akihiko Odaki
2025-10-23 8:10 ` [PATCH 2/2] iotests: Run iotests with sanitizers Akihiko Odaki
@ 2025-11-04 16:36 ` Kevin Wolf
2 siblings, 0 replies; 5+ messages in thread
From: Kevin Wolf @ 2025-11-04 16:36 UTC (permalink / raw)
To: Akihiko Odaki; +Cc: qemu-devel, Hanna Reitz, qemu-block
Am 23.10.2025 um 10:10 hat Akihiko Odaki geschrieben:
> Commit 2cc4d1c5eab1 ("tests/check-block: Skip iotests when sanitizers
> are enabled") changed iotests to skip when sanitizers are enabled.
> The rationale is that AddressSanitizer emits warnings and reports leaks,
> which results in test breakage. Later, sanitizers that are enabled for
> production environments (safe-stack and cfi-icall) were exempted.
>
> However, this approach has a few problems.
>
> - It requires rebuild to disable sanitizers if the existing build has
> them enabled.
> - It disables other useful non-production sanitizers.
> - The exemption of safe-stack and cfi-icall is not correctly
> implemented, so qemu-iotests are incorrectly enabled whenever either
> safe-stack or cfi-icall is enabled *and*, even if there is another
> sanitizer like AddressSanitizer.
>
> To solve these problems, direct AddressSanitizer warnings to separate
> files to avoid changing the test results, and selectively disable
> leak detection at runtime instead of requiring to disable all
> sanitizers at buildtime.
>
> Enabling AddressSanitizer actually revealed a use-after-free so a patch
> to fix it is placed before one that enables iotests with sanitizers.
>
> Signed-off-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
Thanks, applied to the block branch.
Kevin
^ permalink raw reply [flat|nested] 5+ messages in thread