From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with archive (Exim 4.43) id 1PYDDD-0001YQ-Tb for mharc-grub-devel@gnu.org; Thu, 30 Dec 2010 02:46:15 -0500 Received: from [140.186.70.92] (port=49786 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1PYDDB-0001YD-DQ for grub-devel@gnu.org; Thu, 30 Dec 2010 02:46:14 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1PYDDA-0004dt-0k for grub-devel@gnu.org; Thu, 30 Dec 2010 02:46:13 -0500 Received: from mail-yx0-f169.google.com ([209.85.213.169]:47740) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1PYDD9-0004dp-TL for grub-devel@gnu.org; Thu, 30 Dec 2010 02:46:11 -0500 Received: by yxl31 with SMTP id 31so5092286yxl.0 for ; Wed, 29 Dec 2010 23:46:11 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:received:received:sender:subject:from:to :in-reply-to:references:content-type:date:message-id:mime-version :x-mailer:content-transfer-encoding; bh=ZFgKHjYYlIU0WLE2U0ICANTH2VMuX+hcpbE5oMpHbF4=; b=UxsyH6/h+mEp80j/of/7Ka/88mPIiOcqrhW2/p9LeR/h6ruq7cLCJ/nN9WgckJccKQ yhvUryLEyq3fpCF7isQO63Mp2yjWv2fQpMwqaqlCfUFqmLMcNtiTkdpWdr5zDlzMGOYm HZDtRxO+/ziGEld2EEI0FNMshDtBTr6PJZcVo= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=sender:subject:from:to:in-reply-to:references:content-type:date :message-id:mime-version:x-mailer:content-transfer-encoding; b=rmzI7sXmAipvVRFPLISmHhrNGvpUUwn8FnSp0vNtZYvdhPRSFV3KOhdmpM4mTMxJdx dHJwvJYLaYrjrDgtxJgkbeOPMHD/x2jeQcl/8bBqBnaitLDntfa/TvMMKreqWCA2sb3X odgSMxi/i/G5R05xFgQvN1gbONuIHee3V40MI= Received: by 10.90.6.26 with SMTP id 26mr6890773agf.35.1293695171503; Wed, 29 Dec 2010 23:46:11 -0800 (PST) Received: from [192.168.0.106] ([24.145.78.98]) by mx.google.com with ESMTPS id 7sm8812834yhl.27.2010.12.29.23.46.08 (version=TLSv1/SSLv3 cipher=RC4-MD5); Wed, 29 Dec 2010 23:46:09 -0800 (PST) Sender: Dave Vasilevsky From: Dave Vasilevsky To: grub-devel@gnu.org In-Reply-To: <1293694134.2873.17.camel@Inchon> References: <1293694134.2873.17.camel@Inchon> Content-Type: text/plain; charset="UTF-8" Date: Thu, 30 Dec 2010 02:46:07 -0500 Message-ID: <1293695167.2873.20.camel@Inchon> Mime-Version: 1.0 X-Mailer: Evolution 2.30.3 Content-Transfer-Encoding: 7bit X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6 (newer, 2) Subject: Re: [PATCH] hfsplus: Prevent overflows in comparisons X-BeenThere: grub-devel@gnu.org X-Mailman-Version: 2.1.5 Precedence: list Reply-To: The development of GNU GRUB List-Id: The development of GNU GRUB List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 30 Dec 2010 07:46:14 -0000 Now with a ChangeLog entry. -V === modified file 'ChangeLog' --- ChangeLog 2010-12-27 06:19:51 +0000 +++ ChangeLog 2010-12-30 07:39:45 +0000 @@ -1,3 +1,11 @@ +2010-12-30 Dave Vasilevsky + + * grub-core/fs/hfsplus.c: Parent field of + grub_hfsplus_catkey_internal should be unsigned. + (grub_hfsplus_cmp_catkey): Don't compare using subtraction, it + overflows. + (grub_hfsplus_cmp_extkey): Likewise + 2010-12-27 Vladimir Serbinenko * grub-core/loader/xnu.c (grub_cmd_xnu_kernel) [! GRUB_MACHINE_EFI]: === modified file 'grub-core/fs/hfsplus.c' --- grub-core/fs/hfsplus.c 2010-01-20 08:12:47 +0000 +++ grub-core/fs/hfsplus.c 2010-12-30 06:10:23 +0000 @@ -178,7 +178,7 @@ /* Internal representation of a catalog key. */ struct grub_hfsplus_catkey_internal { - int parent; + grub_uint32_t parent; char *name; }; @@ -520,9 +520,12 @@ int i; int diff; - diff = grub_be_to_cpu32 (catkey_a->parent) - catkey_b->parent; - if (diff) - return diff; + /* Safe unsigned comparison */ + grub_uint32_t aparent = grub_be_to_cpu32 (catkey_a->parent); + if (aparent > catkey_b->parent) + return 1; + if (aparent < catkey_b->parent) + return -1; /* Change the filename in keya so the endianness is correct. */ for (i = 0; i < grub_be_to_cpu16 (catkey_a->namelen); i++) @@ -555,15 +558,21 @@ { struct grub_hfsplus_extkey *extkey_a = &keya->extkey; struct grub_hfsplus_extkey_internal *extkey_b = &keyb->extkey; - int diff; - - diff = grub_be_to_cpu32 (extkey_a->fileid) - extkey_b->fileid; - - if (diff) - return diff; - - diff = grub_be_to_cpu32 (extkey_a->start) - extkey_b->start; - return diff; + grub_uint32_t akey; + + /* Safe unsigned comparison */ + akey = grub_be_to_cpu32 (extkey_a->fileid); + if (akey > extkey_b->fileid) + return 1; + if (akey < extkey_b->fileid) + return -1; + + akey = grub_be_to_cpu32 (extkey_a->start); + if (akey > extkey_b->start) + return 1; + if (akey < extkey_b->start) + return -1; + return 0; } static char *