Re: [Xen-devel] [PATCH 4/6] x86/xstate: Fix latent bugs in expand_xsave_states()

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 12.09.16 at 15:57, <andrew.cooper3@xxxxxxxxxx> wrote:
> On 12/09/16 13:43, Jan Beulich wrote:
>>>>> On 12.09.16 at 14:29, <andrew.cooper3@xxxxxxxxxx> wrote:
>>> On 12/09/16 12:41, Jan Beulich wrote:
>>>>>>> On 12.09.16 at 11:51, <andrew.cooper3@xxxxxxxxxx> wrote:
>>>>> @@ -205,11 +222,9 @@ void expand_xsave_states(struct vcpu *v, void *dest, 
> unsigned int size)
>>>>>  
>>>>>          if ( src )
>>>>>          {
>>>>> -            ASSERT((xstate_offsets[index] + xstate_sizes[index]) <= 
>>>>> size);
>>>>> +            BUG_ON((xstate_offsets[index] + xstate_sizes[index]) <= 
>>>>> size);
>>>> Surely converting an ASSERT() to BUG_ON() means inverting the
>>>> relational operator used?
>>> Very true.  It is unfortunate that all of this is dead code, and
>>> impossible to test.  I also had half a mind to explicitly #if 0 it out
>>> to leave people in no illusion that it ever might have been tested.
>> So with this correct, the patch is then
>> Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
> 
> The discussion on this patch has shown that the "if ( src )" is
> unconditionally true, and as such, I would like to remove it.  Would
> your R-b stand with this hunk altered similarly to the final hunk in
> patch 6 (with the BUG_ON() logic adjustment, and updated wording) ?

Yes.

Jan


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