This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
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;