From d1d3a99795006d81f10c98927b0c2ad270bc78fd Mon Sep 17 00:00:00 2001 From: Vladimir Sementsov-Ogievskiy Date: Tue, 24 Mar 2020 18:36:25 +0300 Subject: [PATCH 1/3] scripts/coccinelle: add error-use-after-free.cocci Add script to find and fix trivial use-after-free of Error objects. How to use: spatch --sp-file scripts/coccinelle/error-use-after-free.cocci \ --macro-file scripts/cocci-macro-file.h --in-place \ --no-show-diff ( FILES... | --use-gitgrep . ) Signed-off-by: Vladimir Sementsov-Ogievskiy Message-Id: <20200324153630.11882-2-vsementsov@virtuozzo.com> Reviewed-by: Richard Henderson Reviewed-by: Markus Armbruster [Pastos in commit message and comment fixed, globbing in MAINTAINERS expanded] Signed-off-by: Markus Armbruster --- MAINTAINERS | 5 ++ scripts/coccinelle/error-use-after-free.cocci | 52 +++++++++++++++++++ 2 files changed, 57 insertions(+) create mode 100644 scripts/coccinelle/error-use-after-free.cocci diff --git a/MAINTAINERS b/MAINTAINERS index 7cb53ec138..9d156d73b3 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -2053,6 +2053,11 @@ F: include/qemu/error-report.h F: qapi/error.json F: util/error.c F: util/qemu-error.c +F: scripts/coccinelle/err-bad-newline.cocci +F: scripts/coccinelle/error-use-after-free.cocci +F: scripts/coccinelle/error_propagate_null.cocci +F: scripts/coccinelle/remove_local_err.cocci +F: scripts/coccinelle/use-error_fatal.cocci GDB stub M: Alex Bennée diff --git a/scripts/coccinelle/error-use-after-free.cocci b/scripts/coccinelle/error-use-after-free.cocci new file mode 100644 index 0000000000..72ae9fdebf --- /dev/null +++ b/scripts/coccinelle/error-use-after-free.cocci @@ -0,0 +1,52 @@ +// Find and fix trivial use-after-free of Error objects +// +// Copyright (c) 2020 Virtuozzo International GmbH. +// +// This program is free software; you can redistribute it and/or +// modify it under the terms of the GNU General Public License as +// published by the Free Software Foundation; either version 2 of the +// License, or (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with this program. If not, see +// . +// +// How to use: +// spatch --sp-file scripts/coccinelle/error-use-after-free.cocci \ +// --macro-file scripts/cocci-macro-file.h --in-place \ +// --no-show-diff ( FILES... | --use-gitgrep . ) + +@ exists@ +identifier fn, fn2; +expression err; +@@ + + fn(...) + { + <... +( + error_free(err); ++ err = NULL; +| + error_report_err(err); ++ err = NULL; +| + error_reportf_err(err, ...); ++ err = NULL; +| + warn_report_err(err); ++ err = NULL; +| + warn_reportf_err(err, ...); ++ err = NULL; +) + ... when != err = NULL + when != exit(...) + fn2(..., err, ...) + ...> + } From b0e709503cee6e30e62b35ef416a53531371c1a7 Mon Sep 17 00:00:00 2001 From: Vladimir Sementsov-Ogievskiy Date: Tue, 24 Mar 2020 18:36:27 +0300 Subject: [PATCH 2/3] dump/win_dump: fix use after free of err It's possible that we'll try to set err twice (or more). It's bad, it will crash. Instead, use warn_report(). Signed-off-by: Vladimir Sementsov-Ogievskiy Message-Id: <20200324153630.11882-4-vsementsov@virtuozzo.com> Reviewed-by: Markus Armbruster Reviewed-by: Richard Henderson Signed-off-by: Markus Armbruster --- dump/win_dump.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/dump/win_dump.c b/dump/win_dump.c index eda2a48974..652c7bad99 100644 --- a/dump/win_dump.c +++ b/dump/win_dump.c @@ -304,13 +304,11 @@ static void restore_context(WinDumpHeader64 *h, struct saved_context *saved_ctx) { int i; - Error *err = NULL; for (i = 0; i < h->NumberProcessors; i++) { if (cpu_memory_rw_debug(first_cpu, saved_ctx[i].addr, (uint8_t *)&saved_ctx[i].ctx, sizeof(WinContext), 1)) { - error_setg(&err, "win-dump: failed to restore CPU #%d context", i); - warn_report_err(err); + warn_report("win-dump: failed to restore CPU #%d context", i); } } } From 6a4a38530e70f3917a58d71d4d08e28bd8146015 Mon Sep 17 00:00:00 2001 From: Vladimir Sementsov-Ogievskiy Date: Tue, 24 Mar 2020 18:36:30 +0300 Subject: [PATCH 3/3] qga/commands-posix: fix use after free of local_err local_err is used several times in guest_suspend(). Setting non-NULL local_err will crash, so let's zero it after freeing. Also fix possible leak of local_err in final if(). Signed-off-by: Vladimir Sementsov-Ogievskiy Message-Id: <20200324153630.11882-7-vsementsov@virtuozzo.com> Reviewed-by: Richard Henderson Signed-off-by: Markus Armbruster --- qga/commands-posix.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/qga/commands-posix.c b/qga/commands-posix.c index 93474ff770..cc69b82704 100644 --- a/qga/commands-posix.c +++ b/qga/commands-posix.c @@ -1773,6 +1773,7 @@ static void guest_suspend(SuspendMode mode, Error **errp) } error_free(local_err); + local_err = NULL; if (pmutils_supports_mode(mode, &local_err)) { mode_supported = true; @@ -1784,6 +1785,7 @@ static void guest_suspend(SuspendMode mode, Error **errp) } error_free(local_err); + local_err = NULL; if (linux_sys_state_supports_mode(mode, &local_err)) { mode_supported = true; @@ -1791,6 +1793,7 @@ static void guest_suspend(SuspendMode mode, Error **errp) } if (!mode_supported) { + error_free(local_err); error_setg(errp, "the requested suspend mode is not supported by the guest"); } else {