llvm dev - May 2015 - [LLVMdev] Potential "Buffer Overflow - Array Index Out of Bounds" issue

Hi,

I noticed a possible Buffer Overflow issue in one of the auto-generated files by
AsmWriterEmitter.cpp

The snippet of code generated by the emitter is :

'''''
  uint64_t Bits1 = OpInfo[MI->getOpcode()];
  uint64_t Bits2 = OpInfo2[MI->getOpcode()];
  uint64_t Bits = (Bits2 << 32) | Bits1;
  assert(Bits != 0 && "Cannot print this instruction.");
  O << AsmStrs+(Bits & 4095)-1;
'''''

The risk is that Bits1 and Bits2 could read 0x0 for certain opcodes. If this
happens, "(Bits & 4095)-1" would evaluate to -1, causing an out of
bounds address being put out to raw_ostream O.
There is an assert to check for this very case, but I am wondering if we need to
bail out with an error ( maybe a 'report_fatal_error') in addition to an
assert?

The lines in AsmWritterEmitter generating this snippet (line 450 - 461):

''''
if (BitsLeft < 32) {
    // If we have two tables then we need to perform two lookups and combine
    // the results into a single 64-bit value.
    O << "  uint64_t Bits1 = OpInfo[MI->getOpcode()];\n"
      << "  uint64_t Bits2 = OpInfo2[MI->getOpcode()];\n"
      << "  uint64_t Bits = (Bits2 << 32) | Bits1;\n";
  } else {
    // If only one table is used we just need to perform a single lookup.
    O << "  uint32_t Bits = OpInfo[MI->getOpcode()];\n";
  }
  O << "  assert(Bits != 0 && \"Cannot print this
instruction.\");\n"
    << "  O << AsmStrs+(Bits & " << (1 <<
AsmStrBits)-1 << ")-1;\n\n";
''''
Appreciate comments.

Thanks,
Satyajeet

-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20150514/1f75fcb8/attachment.html>

I think the code in question believes that 'Bits == 0' is a logic bug
somewhere, reporting a fatal error in this case doesn't seem helpful.

On Wed, May 13, 2015 at 5:04 PM, Bhide, Satyajeet B <
satyajeet.b.bhide at intel.com> wrote:
>  Hi,
>
>
>
> I noticed a possible Buffer Overflow issue in one of the auto-generated
> files by AsmWriterEmitter.cpp
>
>
>
> The snippet of code generated by the emitter is :
>
>
>
> ‘’’’’
>
>   uint64_t Bits1 = OpInfo[MI->getOpcode()];
>
>   uint64_t Bits2 = OpInfo2[MI->getOpcode()];
>
>   uint64_t Bits = (Bits2 << 32) | Bits1;
>
>   assert(Bits != 0 && "Cannot print this instruction.");
>
>   O << AsmStrs+(Bits & 4095)-1;
>
> ‘’’’’
>
>
>
> The risk is that Bits1 and Bits2 could read 0x0 for certain opcodes. If
> this happens, “(Bits & 4095)-1” would evaluate to -1, causing an out of
> bounds address being put out to raw_ostream O.
>
> There is an assert to check for this very case, but I am wondering if we
> need to bail out with an error ( maybe a ‘report_fatal_error’) in addition
> to an assert?
>
>
>
> The lines in AsmWritterEmitter generating this snippet (line 450 – 461):
>
>
>
> ’’’’
>
> if (BitsLeft < 32) {
>
>     // If we have two tables then we need to perform two lookups and
> combine
>
>     // the results into a single 64-bit value.
>
>     O << "  uint64_t Bits1 = OpInfo[MI->getOpcode()];\n"
>
>       << "  uint64_t Bits2 =
OpInfo2[MI->getOpcode()];\n"
>
>       << "  uint64_t Bits = (Bits2 << 32) |
Bits1;\n";
>
>   } else {
>
>     // If only one table is used we just need to perform a single lookup.
>
>     O << "  uint32_t Bits = OpInfo[MI->getOpcode()];\n";
>
>   }
>
>   O << "  assert(Bits != 0 && \"Cannot print this
instruction.\");\n"
>
>     << "  O << AsmStrs+(Bits & " << (1
<< AsmStrBits)-1 << ")-1;\n\n";
>
> ’’’’
>
> Appreciate comments.
>
>
>
> Thanks,
>
> Satyajeet
>
>
>
> _______________________________________________
> LLVM Developers mailing list
> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20150513/f7581836/attachment.html>