From mboxrd@z Thu Jan 1 00:00:00 1970 From: Avi Kivity Subject: Re: [PATCH] KVM: fix the handling of dirty bitmaps to avoid overflows Date: Tue, 13 Apr 2010 09:50:20 +0300 Message-ID: <4BC4142C.8050400@redhat.com> References: <20100412193535.6c502695.yoshikawa.takuya@oss.ntt.co.jp> <20100412173951.GA5614@amt.cnet> <4BC3C048.5030704@oss.ntt.co.jp> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Cc: Marcelo Tosatti , kvm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, kvm-ia64-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, kvm-ppc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org To: Takuya Yoshikawa Return-path: In-Reply-To: <4BC3C048.5030704-gVGce1chcLdL9jVzuh4AOg@public.gmane.org> Sender: kvm-ppc-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org List-Id: kvm.vger.kernel.org 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 >> >> 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.