Re: [Xen-devel] [PATCH] timers: limit heap size

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 09.04.19 at 15:38, <andrew.cooper3@xxxxxxxxxx> wrote:
> On 09/04/2019 14:01, Jan Beulich wrote:
>> @@ -463,9 +463,14 @@ static void timer_softirq_action(void)
>>      if ( unlikely(ts->list != NULL) )
>>      {
>>          /* old_limit == (2^n)-1; new_limit == (2^(n+4))-1 */
>> -        int old_limit = heap_metadata(heap)->limit;
>> -        int new_limit = ((old_limit + 1) << 4) - 1;
>> -        struct timer **newheap = xmalloc_array(struct timer *, new_limit + 
> 1);
>> +        unsigned int old_limit = heap_metadata(heap)->limit;
>> +        unsigned int new_limit = ((old_limit + 1) << 4) - 1;
>> +        struct timer **newheap = NULL;
>> +
>> +        /* Don't grow the heap beyond what is representable in its 
>> metadata. */
>> +        if ( new_limit == (typeof(heap_metadata(heap)->limit))new_limit &&
>> +             new_limit + 1 )
>> +            newheap = xmalloc_array(struct timer *, new_limit + 1);
> 
> It would probably be helpful to have a warn_once/print_once in the case
> that we do hit the metadata limit

I can do this, albeit the lack of the constructs you suggest will
make this a little ugly.

> What is the new_limit + 1 for?  Is it an overflow check?

Kind of: It avoids the second argument to xmalloc_array() to
degenerate.

> Given a) that we're not moving from uint16_t while we have 32bit
> hypervisor builds in use, and b) the calculation of new_limit will
> truncate before getting into a position where this overflow check would
> trip, I don't think it is helpful to retain.

But that's what I'm (trying to) state(ing) in the description:
Making the size half a pointer's width is surely an option, which
would make it 32 bits on 64-bit builds (and result in better code
to be generated on x86). From a size/limit field width's
perspective the changes done here would be sufficient. The left
shifting in down_heap() would still need taking care of if we really
were to go that route.

Jan



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