Re: [Xen-devel] GPF in mcheck_init() when booting xen-unstable on VMware ESX 5.1

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 31.05.13 at 21:32, Andrew Cooper <andrew.cooper3@xxxxxxxxxx> wrote:
> On 31/05/13 20:19, Aravindh Puthiyaparambil (aravindp) wrote:
>> I have narrowed it down to line 631 in set_poll_bankmask():
>>      bitmap_copy(mb->bank_map, mca_allbanks->bank_map, nr_mce_banks);
>>
>> What is happening is that in mca_cap_init(), nr_mce_banks is being set to 0. 
> This causes the allocation of bank_map to be set to ZERO_BLOCK_PTR which is 
> the return value for zero-size allocation by xzalloc_array()/_xmalloc(). This 
> results in the bitmap_copy() to fail disastrously. Is it correct to disable 
> MCE if nr_mce_banks is 0? Or say this is a quirk of the VMware virtual 
> platform and run with mce=0? Linux is to be able to handle this gracefully.
>>
>> Another question I have is that callers of xzalloc_array() and friends only 
> check for a NULL return as an error. So what about cases like the one above 
> which fell through the cracks because the return value is ZERO_BLOCK_PTR? 
> Should they all be checking for ZERO_BLOCK_PTR too or ensuring that no calls 
> are made with zero size allocations?
> 
> ZERO_BLOCK_PTR is specifically distinguished from NULL (As the comment
> beside it says).
> 
> The real bug is calling **alloc() with 0 as a parameter.

That's not really a bug - there are cases (hence the returning of
ZERO_BLOCK_PTR in the first place) where this is okay (e.g. in
order to simplify code). memcpy(), memset(), etc are all well
capable of dealing with that situation. The fact that bitmap_copy()
isn't is the root bug from my pov (and similarly other bitmap
operations - they should all bail on non-positive nbits input to be
generically usable, but perhaps we should make nbits of unsigned
type in the first place, as negative values are senseless anyway).

> I would say that nr_mce_banks of 0 should result in an implicit mce=0. 
> You certainly cant sensibly use MCEs with 0 banks to play with.

Yes, this is sensible to be fixed regardless of the above.

Jan


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