qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Eric Blake <eblake@redhat.com>
To: Max Reitz <mreitz@redhat.com>,
	Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>,
	qemu-devel@nongnu.org
Cc: kwolf@redhat.com, zhang.zhanghailiang@huawei.com,
	qemu-block@nongnu.org, quintela@redhat.com, armbru@redhat.com,
	dgilbert@redhat.com, marcandre.lureau@redhat.com, den@openvz.org,
	mdroth@linux.vnet.ibm.com
Subject: Re: [PATCH 2/6] block/mirror: fix use after free of local_err
Date: Wed, 25 Mar 2020 08:01:47 -0500	[thread overview]
Message-ID: <6a231ab1-869c-d9ba-5ccf-9d8c213e5cf1@redhat.com> (raw)
In-Reply-To: <8cb2bda7-55f5-2646-3c35-d901089ccde5@redhat.com>

On 3/25/20 6:11 AM, Max Reitz wrote:
> On 24.03.20 16:36, Vladimir Sementsov-Ogievskiy wrote:
>> local_err is used again in mirror_exit_common() after
>> bdrv_set_backing_hd(), so we must zero it. Otherwise try to set
>> non-NULL local_err will crash.
> 
> OK, but wouldn’t it be better hygiene to set it to NULL every time it is
> freed?

If we change the signature to error_report_err(&local_err), where 
error_report_err both reports the error (if any) AND sets local_err to 
NULL, then we fix the problem for all callers.  It's a global 
search-and-replace job (Coccinelle is great for that) to update all 
callers to the new signature.

>  (There is a second instance of error_report_err() in this
> function.  I’m a bit worried we might introduce another local_err use
> after that one at some point in the future, and forget to run the cocci
> script then.)
> 
> Are the cocci scripts run regularly by someone?  E.g. as part of a pull
> to master?

I'm not aware of any automated procedure for it at the moment; rather, 
it is still ad hoc as someone notices something needs to be re-run.  But 
there was another thread about someone considering automating Cocci 
scripts as part of the Euler robot...

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org



  parent reply	other threads:[~2020-03-25 13:03 UTC|newest]

Thread overview: 39+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-03-24 15:36 [PATCH for-5.0 0/6] Several error use-after-free Vladimir Sementsov-Ogievskiy
2020-03-24 15:36 ` [PATCH 1/6] scripts/coccinelle: add error-use-after-free.cocci Vladimir Sementsov-Ogievskiy
2020-03-31  9:00   ` Markus Armbruster
2020-03-31  9:12     ` Vladimir Sementsov-Ogievskiy
2020-03-31 13:14       ` Markus Armbruster
2020-03-31 13:49         ` Vladimir Sementsov-Ogievskiy
2020-03-31 18:38           ` Markus Armbruster
2020-03-31 18:56     ` Peter Maydell
2020-04-01  5:07       ` Markus Armbruster
2020-04-01 11:04         ` Peter Maydell
2020-04-01 14:44           ` Markus Armbruster
2020-04-01 16:12             ` Peter Maydell
2020-04-02  6:55               ` Markus Armbruster
2020-04-02  8:19                 ` Peter Maydell
2020-04-02  8:36                   ` Markus Armbruster
2020-03-24 15:36 ` [PATCH 2/6] block/mirror: fix use after free of local_err Vladimir Sementsov-Ogievskiy
2020-03-24 15:57   ` Eric Blake
2020-03-24 17:10   ` John Snow
2020-03-25 11:11   ` Max Reitz
2020-03-25 11:29     ` Max Reitz
2020-03-25 11:47     ` Vladimir Sementsov-Ogievskiy
2020-03-25 12:00       ` Max Reitz
2020-03-31  9:12         ` Markus Armbruster
2020-03-25 13:01     ` Eric Blake [this message]
2020-03-25 12:01   ` Max Reitz
2020-03-24 15:36 ` [PATCH 3/6] dump/win_dump: fix use after free of err Vladimir Sementsov-Ogievskiy
2020-03-30 15:54   ` Markus Armbruster
2020-03-24 15:36 ` [PATCH 4/6] migration/colo: fix use after free of local_err Vladimir Sementsov-Ogievskiy
2020-03-24 19:40   ` Dr. David Alan Gilbert
2020-03-24 15:36 ` [PATCH 5/6] migration/ram: " Vladimir Sementsov-Ogievskiy
2020-03-24 19:41   ` Dr. David Alan Gilbert
2020-03-24 15:36 ` [PATCH 6/6] qga/commands-posix: " Vladimir Sementsov-Ogievskiy
2020-03-24 20:03   ` Eric Blake
2020-03-25  4:28     ` Vladimir Sementsov-Ogievskiy
2020-03-31 11:46       ` Markus Armbruster
2020-03-31 12:04         ` Vladimir Sementsov-Ogievskiy
2020-03-31  8:13     ` Markus Armbruster
2020-03-24 15:50 ` [PATCH for-5.0 0/6] Several error use-after-free Richard Henderson
2020-04-04 12:18 ` Markus Armbruster

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=6a231ab1-869c-d9ba-5ccf-9d8c213e5cf1@redhat.com \
    --to=eblake@redhat.com \
    --cc=armbru@redhat.com \
    --cc=den@openvz.org \
    --cc=dgilbert@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=marcandre.lureau@redhat.com \
    --cc=mdroth@linux.vnet.ibm.com \
    --cc=mreitz@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=quintela@redhat.com \
    --cc=vsementsov@virtuozzo.com \
    --cc=zhang.zhanghailiang@huawei.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).