From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:38460) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1eEvUf-0003G1-Ar for qemu-devel@nongnu.org; Wed, 15 Nov 2017 06:04:34 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1eEvUe-0007DE-Gg for qemu-devel@nongnu.org; Wed, 15 Nov 2017 06:04:33 -0500 Date: Wed, 15 Nov 2017 11:04:19 +0000 From: Darren Kenny Message-ID: <20171115110419.k5vs4fghfzbu3f2c@starbug-vm.ie.oracle.com> References: <20171114184127.24238-1-mreitz@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8; format=flowed Content-Disposition: inline In-Reply-To: <20171114184127.24238-1-mreitz@redhat.com> Subject: Re: [Qemu-devel] [Qemu-block] [PATCH for-2.11] qcow2: Fix overly broad madvise() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Max Reitz Cc: qemu-block@nongnu.org, Kevin Wolf , qemu-devel@nongnu.org FWIW, Reviewed-by: Darren Kenny Thanks, Darren. On Tue, Nov 14, 2017 at 07:41:27PM +0100, Max Reitz wrote: >@mem_size and @offset are both size_t, thus subtracting them from one >another will just return a big size_t if mem_size < offset -- even more >obvious here because the result is stored in another size_t. > >Checking that result to be positive is therefore not sufficient to >excluse the case that offset > mem_size. Thus, we currently sometimes >issue an madvise() over a very large address range. > >This is triggered by iotest 163, but with -m64, this does not result in >tangible problems. But with -m32, this test produces three segfaults, >all of which are fixed by this patch. > >Signed-off-by: Max Reitz >--- > block/qcow2-cache.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > >diff --git a/block/qcow2-cache.c b/block/qcow2-cache.c >index 75746a7f43..5222a7b94d 100644 >--- a/block/qcow2-cache.c >+++ b/block/qcow2-cache.c >@@ -73,7 +73,7 @@ static void qcow2_cache_table_release(BlockDriverState *bs, Qcow2Cache *c, > size_t mem_size = (size_t) s->cluster_size * num_tables; > size_t offset = QEMU_ALIGN_UP((uintptr_t) t, align) - (uintptr_t) t; > size_t length = QEMU_ALIGN_DOWN(mem_size - offset, align); >- if (length > 0) { >+ if (mem_size > offset && length > 0) { > madvise((uint8_t *) t + offset, length, MADV_DONTNEED); > } > #endif >-- >2.13.6 > >