Re: [PATCH] hfsplus: Prevent overflows in comparisons

[Dave Vasilevsky]

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 <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 *