Re: [PATCH] hfsplus: Prevent overflows in comparisons

[Vladimir 'φ-coder/phcoder' Serbinenko]

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 <address@hidden>
> +
> +     * 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  <address@hidden>
>  
>       * 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

signature.asc
Description: OpenPGP digital signature