Re: [Xen-devel] Difference between patch in XSA and patch checked in

[George Dunlap <george.dunlap@xxxxxxxxxx>]

On 08/23/2017 05:39 PM, Andrew Cooper wrote:
> On 23/08/17 17:35, George Dunlap wrote:
>> Dear committers,
>>
>> I spent way to many hours this afternoon trying to rebase the CentOS
>> patchqueue from 4.6.5 to 4.6.6.  In theory "git rebase" should detect
>> when duplicate patches are being merged over and forget the patch
>> automatically.
>>
>> Unfortunately, this didn't work in a large number of cases because there
>> were minor changes between the patch published in the XSA and the patch
>> that ended up getting checked in.  One example of this is
>> xsa218-4.6/0003-gnttab-Avoid-potential-double-put-of-maptrack-entry.patch,
>> which in the advisory has:
>>
>> +        gdprintk(XENLOG_WARNING, "Unstable handle %#x\n", op->handle);
>>
>> But in the checked-in patch for stable-4.6 (c/s 819044abe4) has:
>>
>> +        gdprintk(XENLOG_WARNING, "Unstable handle %u\n", op->handle);
>>
>> So I spent some time writing a script that would automatically look for
>> a patch in the commit history with the same title.  This also didn't
>> work, because the *titles* often changed, in really minor ways.  One
>> example of this is xsa219-4.6.patch, which has the commit title:
>>
>> x86/shadow: Hold references for the duration of emulated writes
>>
>> But when checked in to stable-4.6, had this title (c/s 4d13019cb0):
>>
>> x86/shadow: hold references for the duration of emulated writes
>>
>> Another example is xsa222-1-4.6.patch, which had the following title :
>>
>> xen/memory: Fix return value handing of guest_remove_page()
>>
>> But was checked in to stable-4.6 with this this title (c/s d23eb82c8a):
>>
>> memory: fix return value handing of guest_remove_page()
>>
>> I know Lars has run into similar problems.  Having a computer be able to
>> easily tell whether a patch has been applied or not will save everyone a
>> lot of time in the long run, and is far more important than fixing a
>> printk string or fixing capitalization in a patch title.
>>
>> Can I propose that committers should always check in the exact version
>> of the patch in the publicly-released advisory?  Preferably directly
>> from xsa.git, and with 'git am' (and not rebasing or modifying patches)?
>>
>> "Bugs" in patch titles should be caught during pre-embargo review, and
>> left if they get missed; stylistic issues with the patch should be fixed
>> in follow-up patches.
> 
> I agree.  I only ever use `git am` to apply XSAs to staging branches.
> 
> The one and only change I make is to add the CVE reference in to the
> commit message if it only had an XSA reference in the published patch.

Yes, I think adding tags like CVE numbers, or Reported-by's, is necessary.

 -George

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel