Re: [Xen-devel] [PATCH V2] xen/hvm: fix hypervisor crash with hvm_save_one()

[Razvan Cojocaru <rcojocaru@xxxxxxxxxxxxxxx>]

On 05/03/17 12:30, Jan Beulich wrote:
>>>> On 03.05.17 at 11:21, <tim@xxxxxxx> wrote:
>> At 10:15 +0100 on 03 May (1493806508), Tim Deegan wrote:
>>> At 00:31 -0600 on 03 May (1493771502), Jan Beulich wrote:
>>>> +    else if ( ctxt.cur > sizeof(*desc) )
>>>>      {
>>>>          uint32_t off;
>>>> -        const struct hvm_save_descriptor *desc;
>>>>  
>>>> -        rv = -ENOENT;
>>>>          for ( off = 0; off < (ctxt.cur - sizeof(*desc)); off += 
>>>> desc->length )
>>
>> It occurs to me that as well as underflowing, this test is off by one.
>> It ought to be "off + sizeof(*desc) <= ctxt.cur" to allow for a
>> zero-length record.  AFAIK we don't actually have any of those, so
>> it's academic, but we might want to represent the presence of some
>> feature without having any feature-specific state to save.
> 
> Good point; I already have two follow-up patches, one of which I
> think this adjustment would easily fit into.

Should I re-send the original patch with the updated comment then (thus
also being able to keep Andrew's Signed-off-by), and if so, is it
alright to keep Julien's Release-Acked-by?

Or will you use this later patch (presumably with your Signed-off-by),
in which case I should test it?

Things got rather confusing (apologies for my own part in the confusion).


Thanks,
Razvan

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