Re: [Qemu-devel] [PATCH 0/4] migration: fix CVE-2014-7840

[Amit Shah]

On (Mon) 17 Nov 2014 [13:48:58], Michael S. Tsirkin wrote:
> On Mon, Nov 17, 2014 at 04:37:50PM +0530, Amit Shah wrote:
> > On (Mon) 17 Nov 2014 [12:52:59], Michael S. Tsirkin wrote:
> > > On Mon, Nov 17, 2014 at 04:08:58PM +0530, Amit Shah wrote:
> > > > On (Mon) 17 Nov 2014 [12:32:57], Michael S. Tsirkin wrote:
> > > > > On Mon, Nov 17, 2014 at 12:06:38PM +0530, Amit Shah wrote:
> > > > > > On (Wed) 12 Nov 2014 [11:44:35], Michael S. Tsirkin wrote:
> > > > > > > This patchset fixes CVE-2014-7840: invalid
> > > > > > > migration stream can cause arbitrary qemu memory
> > > > > > > overwrite.
> > > > > > > First patch includes the minimal fix for the issue.
> > > > > > > Follow-up patches on top add extra checking to reduce the
> > > > > > > chance this kind of bug recurs.
> > > > > > > 
> > > > > > > Note: these are already (tentatively-pending review)
> > > > > > > queued in my tree, so only review/ack
> > > > > > > is necessary.
> > > > > > 
> > > > > > Why not let this go in via the migration tree?
> > > > > 
> > > > > Well I Cc'd Juan and David, so if they had a problem with this, I 
> > > > > expect
> > > > > they'd complain.  David acked so I assume it's ok.  Since I wasted 
> > > > > time
> > > > > testing this and have it on my tree already, might as well just merge.
> > > > 
> > > > IMO asking as a courtesy would've been better than just stating it.
> > > 
> > > Right, thanks for reminding me.
> > > 
> > > BTW, there is actually a good reason to special-case it: it's a CVE fix,
> > > which I handle.  So they stay on my private queue and are passed
> > > to vendors so vendors can fix downstreams, until making fix public is
> > > cleared with all reporters and vendors.
> > > After reporting is cleared, I try to collect acks but don't normally route
> > > patches through separate queues - that would make it harder to
> > > track the status which we need for CVEs.
> > 
> > Patch is public, so all of this doesn't really matter.
> > 
> > But: involving maintainers in their areas, even if the patch is
> > embargoed, should be a pre-requisite.  I'm not sure if we're doing
> > that, but please do that so patches get a proper review from the
> > maintainers.
> 
> Involving more people means more back and forth with reporters which
> must approve any disclosure.  If the issue isn't clear, I do involve
> maintainers.  I send patches on list and try to merge them only after
> they get ack from relevant people. I'm sorry, but this is as far as I
> have the time to go.

The other aspect of the thing is sub-optimal, or patches with bugs,
get pushed in, because the maintainers didn't get involved.  Also,
maintainers don't like code being changed behind their backs...

But if you're overloaded with handling security issues, we can help
identify a co-maintainer as well.

> > > I guess this specific one actually is well contained, so it could go in
> > > through a specific tree if it had to.  In fact, it is still possible if
> > > Juan says he prefers it so: I only expect to send pull request around
> > > tomorrow or the day after that.
> > 
> > I'm sure we prefer migration patches go through the migration tree.
> 
> I'm sorry - I disagree. maintainer boundaries in qemu are quite flexible
> because code is somewhat monolithic.  We prefer that patches are
> reviewed by people that are familiar with it, that is for sure.  Which
> tree is definitely secondary.  If there's a conflict, we can resolve it.
> I doubt it will happen here.

For well-maintained areas, I'm not sure I agree with the flexibility
argument.  Juan has been maintaining the migration tree for a long
while now.

> > Also, this week I'm looking at the migration queue -- it's an
> > unofficial split of maintenance duties between Juan and me while we're
> > still trying to find out what works best.
> > 
> 
> I don't know how am I supposed to know that.

Right now you don't need to -- I just pick patches up and pass them on
to Juan, who does the pull req.

> Send patch to MAINTAINERS or something?

I'm going to add myself to migration maintainers, yes.  But that's
secondary; the pull reqs still go via Juan.


                Amit