This is the mail archive of the binutils@sourceware.org mailing list for the binutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] avoid undefined behavior due to oversized shifts


Hi Nickolai,

Looks reasonable to me.  One additional sanity check could be chunksz
!= 0, to ensure that size % chunksz does not divide by zero (and of
course, the !=0 check should go before the %).

Thanks for the suggestion. Here is the patch that I have checked in.


Cheers
  Nick

bfd/ChangeLog
2013-01-03  Nickolai Zeldovich  <nickolai@csail.mit.edu>
	    Nick Clifton  <nickc@redhat.com>

	* elflink.c (get_value): Prevent the use of an undefined shift
	operation.  Add sanity checks.
@@ -7917,31 +7917,49 @@ get_value (bfd_vma size,
           bfd *input_bfd,
           bfd_byte *location)
 {
+  int shift;
   bfd_vma x = 0;

+  /* Sanity checks.  */
+  BFD_ASSERT (chunksz <= sizeof (x)
+             && size >= chunksz
+             && chunksz != 0
+             && (size % chunksz) == 0
+             && input_bfd != NULL
+             && location != NULL);
+
+  if (chunksz == sizeof (x))
+    {
+      BFD_ASSERT (size == chunksz);
+
+      /* Make sure that we do not perform an undefined shift operation.
+        We know that size == chunksz so there will only be one iteration
+        of the loop below.  */
+      shift = 0;
+    }
+  else
+    shift = 8 * chunksz;
+
   for (; size; size -= chunksz, location += chunksz)
     {
       switch (chunksz)
        {
-       default:
-       case 0:
-         abort ();
        case 1:
-         x = (x << (8 * chunksz)) | bfd_get_8 (input_bfd, location);
+         x = (x << shift) | bfd_get_8 (input_bfd, location);
          break;
        case 2:
-         x = (x << (8 * chunksz)) | bfd_get_16 (input_bfd, location);
+         x = (x << shift) | bfd_get_16 (input_bfd, location);
          break;
        case 4:
-         x = (x << (8 * chunksz)) | bfd_get_32 (input_bfd, location);
+         x = (x << shift) | bfd_get_32 (input_bfd, location);
          break;
-       case 8:
 #ifdef BFD64
-         x = (x << (8 * chunksz)) | bfd_get_64 (input_bfd, location);
-#else
-         abort ();
-#endif
+       case 8:
+         x = (x << shift) | bfd_get_64 (input_bfd, location);
          break;
+#endif
+       default:
+         abort ();
        }
     }
   return x;


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]