[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH] x86/p2m: fixed p2m_change_type_range() start / end check

On 04/18/2018 10:48 AM, Razvan Cojocaru wrote:
> On 04/18/2018 10:38 AM, Jan Beulich wrote:
>>>>> On 17.04.18 at 19:16, <rcojocaru@xxxxxxxxxxxxxxx> wrote:
>>> --- a/xen/arch/x86/mm/p2m.c
>>> +++ b/xen/arch/x86/mm/p2m.c
>>> @@ -976,6 +976,13 @@ void p2m_change_type_range(struct domain *d,
>>>      ASSERT(p2m_is_changeable(ot) && p2m_is_changeable(nt));
>>>      p2m_lock(p2m);
>>> +
>>> +    if ( start > p2m->max_mapped_pfn )
>>> +    {
>>> +        p2m_unlock(p2m);
>>> +        return;
>>> +    }
>> I realize this is what George has suggested, but I still wonder if this is 
>> the
>> right thing to do here: Why is this any more important to check than the
>> more general start >= end (in which case the assertion in the rangeset
>> code would also trigger)? Till now the function assumes "sensible" things
>> to be passed in, but specifically also permitting the [start,~0UL] case
>> (just in a more generalizer fashion). The problem you're trying to fix here
>> is something passing in a nonsense range (starting above the valid range).
>> Yet if we want the function to be immune to nonsense being passed in, I
>> think start < end is what needs checking for, and that check would then
>> perhaps better go after end was already adjusted.
>> The obvious alternative is for callers to only pass in sane ranges (in which
>> case adding ASSERT() here may be considered, instead of triggering the
>> one in the rangeset code).
>> In any event you want to add unlikely(), just like the similar end check has.
> FWIW I'll be happy to change the test to unlikely( start < end ), if
> this is OK with George. I'll test the change now.

Sorry, unlikely(end <= start), obviously.


Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.