Re: [PATCH for-4.2? v3 0/8] block: Fix resize (extending) of short overlays

From: Eric Blake <eblake@redhat.com>
To: Peter Maydell <peter.maydell@linaro.org>, Kevin Wolf <kwolf@redhat.com>
Cc: Qemu-block <qemu-block@nongnu.org>,
	Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>,
	QEMU Developers <qemu-devel@nongnu.org>,
	Stefan Hajnoczi <stefanha@redhat.com>,
	Max Reitz <mreitz@redhat.com>
Subject: Re: [PATCH for-4.2? v3 0/8] block: Fix resize (extending) of short overlays
Date: Fri, 22 Nov 2019 10:41:13 -0600	[thread overview]
Message-ID: <713adac7-b997-6723-6b07-f527d0b804ac@redhat.com> (raw)
In-Reply-To: <CAFEAcA_JU9GfNYDuD2K0MKmAD7GZ7vBUHsXU+WS-6mvPCvf8ig@mail.gmail.com>

On 11/22/19 10:17 AM, Peter Maydell wrote:
> On Fri, 22 Nov 2019 at 16:08, Kevin Wolf <kwolf@redhat.com> wrote:
>>
>> See patch 4 for the description of the bug fixed.
> 
> I guess my questions for trying to answer the "for-4.2?"
> question in the subject are:
>   1) is this a security (leaking data into the guest) bug ?
>   2) is this a regression?
>   3) is this something a lot of people are likely to run into?

My thoughts (although Kevin's may be more definitive):

1) yes, there is a security aspect: certain resize or commit actions can 
result in the guest seeing a revival of stale data that the guest may 
have thought that it previously scrubbed.  Similarly, the tail end of 
the series proves via iotests that we have an actual case of data 
corruption after a block commit without this patch

2) no, this is a long-standing bug, we've only recently noticed it

3) no, it is uncommon to have an overlay with a size shorter than its 
backing file (it's not even all that common to have an overlay longer 
than the backing file), so this is a corner case not many people will 
hit.  It's even less common to have the difference in overlay sizes also 
coincide with formats that introduce the speed penalty of a longer 
blocking due to the added zeroing.

> 
> Eyeballing of the diffstat plus the fact we're on v4 of
> the patchset already makes me a little uneasy about
> putting it into rc3, but if the bug we're fixing matters
> enough we can do it.

In terms of diffstat, the v3 series was much smaller in impact.  Both 
versions add robustness, where the difference between v3 and v4 is 
whether we introduce a speed penalty on an unlikely setup (v3) or reject 
any operation where it would require a speed penalty to avoid data 
problems (v4).  I think all the patches in v3 were reviewed, but I'll go 
ahead and review v4 as well.

Because of point 1, I am leaning towards some version of this patch 
series (whether 3 or 4) making -rc3; but point 2 (it is not a 4.2 
regression) also seems to be a reasonable justification for slipping 
this to 5.0.

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