Several error use-after-free

[PATCH for-5.0 0/6] Several error use-after-free

Vladimir Sementsov-Ogievskiy posted 6 patches 4 years, 1 month ago

Download series mbox

Test docker-mingw@fedora passed

Test docker-quick@centos7 passed

Test checkpatch passed

Test FreeBSD passed

Test asan passed

Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20200324153630.11882-1-vsementsov@virtuozzo.com

Maintainers: Max Reitz <mreitz@redhat.com>, "Dr. David Alan Gilbert" <dgilbert@redhat.com>, John Snow <jsnow@redhat.com>, Michael Roth <mdroth@linux.vnet.ibm.com>, Kevin Wolf <kwolf@redhat.com>, "Marc-André Lureau" <marcandre.lureau@redhat.com>, Juan Quintela <quintela@redhat.com>, Hailiang Zhang <zhang.zhanghailiang@huawei.com>

scripts/coccinelle/error-use-after-free.cocci | 52 +++++++++++++++++++
block/mirror.c                                |  1 +
dump/win_dump.c                               |  4 +-
migration/colo.c                              |  1 +
migration/ram.c                               |  1 +
qga/commands-posix.c                          |  3 ++
MAINTAINERS                                   |  1 +
7 files changed, 60 insertions(+), 3 deletions(-)
create mode 100644 scripts/coccinelle/error-use-after-free.cocci

Expand all Fold all

[PATCH for-5.0 0/6] Several error use-after-free

Posted by Vladimir Sementsov-Ogievskiy 4 years, 1 month ago

Hi all!

I accidentally found use-after-free of local_err in mirror, and decided
to search for similar cases with help of small coccinelle script
(patch 01). Happily, there no many cases.

Better to fix zero Error* pointer after each freeing everywhere, but
this is too much for 5.0 and most of these cases will be covered by
error-auto-propagation series.

Note also, that there are still a lot of use-after-free cases possible
when error is not local variable but field of some structure, shared by
several functions.

Vladimir Sementsov-Ogievskiy (6):
  scripts/coccinelle: add error-use-after-free.cocci
  block/mirror: fix use after free of local_err
  dump/win_dump: fix use after free of err
  migration/colo: fix use after free of local_err
  migration/ram: fix use after free of local_err
  qga/commands-posix: fix use after free of local_err

 scripts/coccinelle/error-use-after-free.cocci | 52 +++++++++++++++++++
 block/mirror.c                                |  1 +
 dump/win_dump.c                               |  4 +-
 migration/colo.c                              |  1 +
 migration/ram.c                               |  1 +
 qga/commands-posix.c                          |  3 ++
 MAINTAINERS                                   |  1 +
 7 files changed, 60 insertions(+), 3 deletions(-)
 create mode 100644 scripts/coccinelle/error-use-after-free.cocci

-- 
2.21.0

Re: [PATCH for-5.0 0/6] Several error use-after-free

Posted by Richard Henderson 4 years, 1 month ago

On 3/24/20 8:36 AM, Vladimir Sementsov-Ogievskiy wrote:
> Vladimir Sementsov-Ogievskiy (6):
>   scripts/coccinelle: add error-use-after-free.cocci
>   block/mirror: fix use after free of local_err
>   dump/win_dump: fix use after free of err
>   migration/colo: fix use after free of local_err
>   migration/ram: fix use after free of local_err
>   qga/commands-posix: fix use after free of local_err

Whole series:
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

Re: [PATCH for-5.0 0/6] Several error use-after-free

Posted by Markus Armbruster 4 years ago

Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> writes:

> Hi all!
>
> I accidentally found use-after-free of local_err in mirror, and decided
> to search for similar cases with help of small coccinelle script
> (patch 01). Happily, there no many cases.
>
> Better to fix zero Error* pointer after each freeing everywhere, but
> this is too much for 5.0 and most of these cases will be covered by
> error-auto-propagation series.
>
> Note also, that there are still a lot of use-after-free cases possible
> when error is not local variable but field of some structure, shared by
> several functions.

I queued the part that hasn't been merged.  Thanks!