From patchwork Wed Apr 30 18:23:53 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kevin Wolf X-Patchwork-Id: 344273 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id D7B0314009B for ; Thu, 1 May 2014 04:30:31 +1000 (EST) Received: from localhost ([::1]:58461 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WfZH3-0000OP-JM for incoming@patchwork.ozlabs.org; Wed, 30 Apr 2014 14:30:29 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:45722) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WfZBS-0007Pz-QW for qemu-devel@nongnu.org; Wed, 30 Apr 2014 14:24:50 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WfZBM-0003Ir-Gu for qemu-devel@nongnu.org; Wed, 30 Apr 2014 14:24:42 -0400 Received: from mx1.redhat.com ([209.132.183.28]:30358) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WfZBM-0003Ie-8F for qemu-devel@nongnu.org; Wed, 30 Apr 2014 14:24:36 -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 (8.14.4/8.14.4) with ESMTP id s3UIOZt4019742 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK) for ; Wed, 30 Apr 2014 14:24:35 -0400 Received: from noname.redhat.com (ovpn-116-113.ams2.redhat.com [10.36.116.113]) by int-mx11.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id s3UIO4it011903; Wed, 30 Apr 2014 14:24:34 -0400 From: Kevin Wolf To: qemu-devel@nongnu.org Date: Wed, 30 Apr 2014 20:23:53 +0200 Message-Id: <1398882243-14783-22-git-send-email-kwolf@redhat.com> In-Reply-To: <1398882243-14783-1-git-send-email-kwolf@redhat.com> References: <1398882243-14783-1-git-send-email-kwolf@redhat.com> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.24 X-detected-operating-system: by eggs.gnu.org: GNU/Linux 3.x X-Received-From: 209.132.183.28 Cc: kwolf@redhat.com Subject: [Qemu-devel] [PULL 21/31] qcow2: Check min_size in qcow2_grow_l1_table() X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org From: Max Reitz First, new_l1_size is an int64_t, whereas min_size is a uint64_t. Therefore, during the loop which adjusts new_l1_size until it equals or exceeds min_size, new_l1_size might overflow and become negative. The comparison in the loop condition however will take it as an unsigned value (because min_size is unsigned) and therefore recognize it as exceeding min_size. Therefore, the loop is left with a negative new_l1_size, which is not correct. This could be fixed by making new_l1_size uint64_t. On the other hand, however, by doing this, the while loop may take forever. If min_size is e.g. UINT64_MAX, it will take new_l1_size probably multiple overflows to reach the exact same value (if it reaches it at all). Then, right after the loop, new_l1_size will be recognized as being too big anyway. Both problems require a ridiculously high min_size value, which is very unlikely to occur; but both problems are also simply avoided by checking whether min_size is sane before calculating new_l1_size (which should still be checked separately, though). Signed-off-by: Max Reitz Signed-off-by: Kevin Wolf --- block/qcow2-cluster.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c index b746429..76d2bcf 100644 --- a/block/qcow2-cluster.c +++ b/block/qcow2-cluster.c @@ -42,6 +42,13 @@ int qcow2_grow_l1_table(BlockDriverState *bs, uint64_t min_size, if (min_size <= s->l1_size) return 0; + /* Do a sanity check on min_size before trying to calculate new_l1_size + * (this prevents overflows during the while loop for the calculation of + * new_l1_size) */ + if (min_size > INT_MAX / sizeof(uint64_t)) { + return -EFBIG; + } + if (exact_size) { new_l1_size = min_size; } else {