From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:55764) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ai9qH-0005k7-Tw for qemu-devel@nongnu.org; Mon, 21 Mar 2016 20:06:39 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ai9qE-0006JU-Ng for qemu-devel@nongnu.org; Mon, 21 Mar 2016 20:06:37 -0400 Received: from mx1.redhat.com ([209.132.183.28]:56728) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ai9qE-0006JQ-HN for qemu-devel@nongnu.org; Mon, 21 Mar 2016 20:06:34 -0400 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) by mx1.redhat.com (Postfix) with ESMTPS id F236780E47 for ; Tue, 22 Mar 2016 00:06:33 +0000 (UTC) From: Bandan Das References: <20160321163441.29684cbf@t450s.home> Date: Mon, 21 Mar 2016 20:06:32 -0400 In-Reply-To: <20160321163441.29684cbf@t450s.home> (Alex Williamson's message of "Mon, 21 Mar 2016 16:34:41 -0600") Message-ID: MIME-Version: 1.0 Content-Type: text/plain Subject: Re: [Qemu-devel] [PATCH] vfio: add check for memory region overflow condition List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alex Williamson Cc: qemu-devel Alex Williamson writes: > On Mon, 21 Mar 2016 18:00:50 -0400 > Bandan Das wrote: > >> vfio_listener_region_add for a iommu mr results in >> an overflow assert since emulated iommu memory region is initialized >> with UINT64_MAX. Add a check just like memory_region_size() >> does. >> >> Signed-off-by: Bandan Das >> --- >> hw/vfio/common.c | 7 ++++++- >> 1 file changed, 6 insertions(+), 1 deletion(-) >> >> diff --git a/hw/vfio/common.c b/hw/vfio/common.c >> index fb588d8..269244b 100644 >> --- a/hw/vfio/common.c >> +++ b/hw/vfio/common.c >> @@ -349,7 +349,12 @@ static void vfio_listener_region_add(MemoryListener *listener, >> if (int128_ge(int128_make64(iova), llend)) { >> return; >> } >> - end = int128_get64(llend); >> + >> + if (int128_eq(llend, int128_2_64())) { >> + end = UINT64_MAX; >> + } else { >> + end = int128_get64(llend); >> + } >> >> if ((iova < container->min_iova) || ((end - 1) > container->max_iova)) { >> error_report("vfio: IOMMU container %p can't map guest IOVA region" > > But now all the calculations where we use end-1 are wrong. See the > discussion with Pierre Morel in the January qemu-devel archives. > There's a solution in there, but I never saw a follow-up from Pierre > with a revised patch. Thanks, I am missing something. When end < UIN64_MAX, end - 1 calculations are valid because the patch doesn't change that behavior. When end is UINT64_MAX, int128_get64() doesn't know how to calculate this value and we are just feeding it manually. The patch is just the opposite of what memory_region_init() did to init the mem region in the first place: mr->size = int128_make64(size); if (size == UINT64_MAX) { mr->size = int128_2_64(); } So, end - 1 is still valid for end = UINT64_MAX, no ? > Alex