[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] XSA-283: Post-mortem

  • To: Rich Persaud <persaur@xxxxxxxxx>
  • From: George Dunlap <George.Dunlap@xxxxxxxxxx>
  • Date: Thu, 28 Feb 2019 11:03:16 +0000
  • Accept-language: en-GB, en-US
  • Cc: xen-devel <xen-devel@xxxxxxxxxxxxxxxxxxxx>
  • Delivery-date: Thu, 28 Feb 2019 11:03:25 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
  • Thread-index: AQHUzRzVriDZG7WMQE+YJb8CFJBBoqXxFkWAgAPpJYA=
  • Thread-topic: [Xen-devel] XSA-283: Post-mortem

> On Feb 25, 2019, at 11:20 PM, Rich Persaud <persaur@xxxxxxxxx> wrote:
> On Feb 25, 2019, at 10:14, George Dunlap <george.dunlap@xxxxxxxxxx> wrote:
>> This is an attempt to do a 'post-mortem' on XSA-283, to find out how
>> the error came about, and consider what changes we could make to code
>> / processes to prevent such errors from happening in the future.  The
>> Security Team hopes to make it a habit to perform such analyses in the
>> future.
> ...
>> There was no public review of this patch; the commit contains no R-b
>> or A-b.  Keir's only comment on the patch before committing it was to
>> note that it conflicted with a different patch and ask for a rebase.
> Would it be useful to count/triage commits without R-b or A-b?

It would be easy to do, but I doubt it would turn up anything useful; 
essentially it would turn into “With-Keir” and “After-Keir”, as the discipline 
that nothing get checked in without an [RA]-b was introduced after Keir sort of 
faded from the scene.  I’m not sure that Keir was in the habit of adding [RA]-b 
tags to commits even when they were given.

So a search of the tree for all commits without [RA]-b tags would likely just 
turn up everything for the first 6-8 years of Xen development; not particularly 
useful if looking for further potential issues (which is, I assume, the goal of 
your suggestion).

The reason I pointed it out was that *in general*, I don’t think exhorting 
reviewers to be “more vigilant” is that helpful; but it does seem to me that in 
this particular case, it’s likely that our modern reviewing process would have 
flagged this as at least ugly / unnecessary (even if the reviewer didn’t spot 
the potential security issue).

That said — perhaps something that would be concretely useful would be to ask 
maintainers to regularly look at the commits that introduced security issues, 
so that they can get a feel concretely for what sorts of patterns to look for.

Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.