[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 1/6] scripts/coccinelle: add error-use-after-free.cocci
From: |
Markus Armbruster |
Subject: |
Re: [PATCH 1/6] scripts/coccinelle: add error-use-after-free.cocci |
Date: |
Tue, 31 Mar 2020 11:00:12 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/26.3 (gnu/linux) |
Vladimir Sementsov-Ogievskiy <address@hidden> writes:
> Add script to find and fix trivial use-after-free of Error objects.
> How to use:
> spatch --sp-file scripts/coccinelle/auto-propagated-errp.cocci \
> --macro-file scripts/cocci-macro-file.h --in-place \
> --no-show-diff ( FILES... | --use-gitgrep . )
Pasto: you mean scripts/coccinelle/error-use-after-free.cocci.
--use-gitgrep is just one of several methods. Any particular reason for
recommending it over the others?
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <address@hidden>
> ---
> scripts/coccinelle/error-use-after-free.cocci | 52 +++++++++++++++++++
> MAINTAINERS | 1 +
> 2 files changed, 53 insertions(+)
> create mode 100644 scripts/coccinelle/error-use-after-free.cocci
>
> diff --git a/scripts/coccinelle/error-use-after-free.cocci
> b/scripts/coccinelle/error-use-after-free.cocci
> new file mode 100644
> index 0000000000..7cfa42355b
> --- /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
> +// <http://www.gnu.org/licenses/>.
> +//
> +// How to use:
> +// spatch --sp-file scripts/coccinelle/auto-propagated-errp.cocci \
> +// --macro-file scripts/cocci-macro-file.h --in-place \
> +// --no-show-diff ( FILES... | --use-gitgrep . )
Same pasto.
I doubt including basic spatch instructions with every script is a good
idea. Better explain it in one place, with proper maintenance.
scripts/coccinelle/README? We could be a bit more verbose there,
e.g. to clarify required vs. suggested options.
> +
> +@ 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, ...)
> + ...>
> + }
This inserts err = NULL after error_free() if there is a path to a
certain kind of use of @err without such an assignment.
The "when != exit()" part excludes certain "phony" paths. It's not a
tight check; there are other ways to unconditionally terminate the
process or jump elsewhere behind Coccinelle's back. Not a problem, the
script is meant to have its output reviewed manually.
Should we mention the need to review the script's output?
> diff --git a/MAINTAINERS b/MAINTAINERS
> index b5c86ec494..ba97cc43fc 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -2037,6 +2037,7 @@ F: include/qemu/error-report.h
> F: qapi/error.json
> F: util/error.c
> F: util/qemu-error.c
> +F: scripts/coccinelle/*err*.cocci
Silently captures existing scripts in addition to this new one.
Tolerable. The globbing looks rather brittle, though.
>
> GDB stub
> M: Alex Bennée <address@hidden>
Re: [PATCH 2/6] block/mirror: fix use after free of local_err, Max Reitz, 2020/03/25
[PATCH 1/6] scripts/coccinelle: add error-use-after-free.cocci, Vladimir Sementsov-Ogievskiy, 2020/03/24
- Re: [PATCH 1/6] scripts/coccinelle: add error-use-after-free.cocci,
Markus Armbruster <=
Re: [PATCH 1/6] scripts/coccinelle: add error-use-after-free.cocci, Peter Maydell, 2020/03/31
[PATCH 4/6] migration/colo: fix use after free of local_err, Vladimir Sementsov-Ogievskiy, 2020/03/24
[PATCH 3/6] dump/win_dump: fix use after free of err, Vladimir Sementsov-Ogievskiy, 2020/03/24
[PATCH 5/6] migration/ram: fix use after free of local_err, Vladimir Sementsov-Ogievskiy, 2020/03/24