Re: [Xen-devel] [PATCH] x86/badpage: Fix badpage->order overflow

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 12.11.18 at 20:01, <andrew.cooper3@xxxxxxxxxx> wrote:
> On 12/11/18 09:54, Jan Beulich wrote:
>>>>> On 09.11.18 at 15:42, <andrew.cooper3@xxxxxxxxxx> wrote:
>>> For order 32 or more, the shift will truncate.  Spotted by Coverity.
>> I find this pretty absurd. What about order 64 or more? Are you
>> suggesting you expect 16Tb or larger bad page ranges?
>>
>>> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
>> Anyway, as I don't mind the adjustment
>> Acked-by: Jan Beulich <jbeulich@xxxxxxxx>
>>
>> But I'm sure you're aware we have many more examples of
>> 1U (or even plain 1) getting shifted by an order value.
> 
> There is nothing absurd here.
> 
> The complaint is that there are two parts of a single calculation at
> different widths, where C's integer promotion rules results in code
> which reads correctly, but behaves incorrectly.
> 
> If badpage->mfn was uint32_t, there would be no complaint, because all
> parts of the expression would be evaluated at the same width.  This
> complaint is only because you've got a 32bit shift, and a 64bit addition.
> 
> As for likelihood, I really hope we don't need mark a 16Tb range as bad,
> but given the ability to express this, would you really notice that the
> result was subtly wrong?

I think so, yes. It doesn't matter much here, but the change results
in a pointless extra REX.W prefix. Adding a 32-bit value to a 64-bit
one is not per se a bug after all. It is (I believe) impossible for a tool
to tell apart legitimate and problematic cases.

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel