Re: [PATCH] hfsplus: Prevent overflows in comparisons

["Vladimir 'φ-coder/phcoder' Serbinenko" <phcoder@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 2402 bytes --]

Applied, thanks.
On 12/30/2010 08:46 AM, Dave Vasilevsky wrote:
> === 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 <dave@vasilevsky.ca>
> +
> +	* 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  <phcoder@gmail.com>
>  
>  	* 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 *
>
>
>
>   


-- 
Regards
Vladimir 'φ-coder/phcoder' Serbinenko



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 294 bytes --]