Re: [PATCH] KVM: fix the handling of dirty bitmaps to avoid overflows

On 04/13/2010 03:52 AM, Takuya Yoshikawa wrote:
> (2010/04/13 2:39), Marcelo Tosatti wrote:
>> On Mon, Apr 12, 2010 at 07:35:35PM +0900, Takuya Yoshikawa wrote:
>>> This patch fixes a bug found by Avi during the review process
>>> of my dirty bitmap related work.
>>>
>>> To ppc and ia64 people:
>>>    The fix is really simple but touches all architectures using
>>>    dirty bitmaps. So please check this will not suffer your part.
>>>
>>> =>>>
>>> Int is not long enough to store the size of a dirty bitmap.
>>>
>>> This patch fixes this problem with the introduction of a wrapper
>>> function to calculate the sizes of dirty bitmaps.
>>>
>>> Note: in mark_page_dirty(), we have to consider the fact that
>>>    __set_bit() takes the offset as int, not long.
>>>
>>> Signed-off-by: Takuya Yoshikawa<[email protected]>
>>
>> Applied, thanks.
>>
>
> Thanks everyone!
>
> BTW, just from my curiosity, are there any cases in which we use such 
> huge
> number of pages currently?
>
>   ALIGN(memslot->npages, BITS_PER_LONG) / 8;
>
> More than G pages need really big memory!
>   -- We are assuming some special cases like "short" int size?

No, int is 32 bits, but memslot->npages is not our under control.

Note that you don't actually need all those pages to create a large 
memory slot.

>
> If so, we may have to care about a lot of things from now on, because 
> common
> functions like __set_bit() don't support such long buffers.

It's better to limit memory slots to something that can be handled by 
everything, then. 2^31 pages is plenty.  Return -EINVAL if the slot is 
too large.


-- 
Do not meddle in the internals of kernels, for they are subtle and quick to panic.