Bhide, Satyajeet B
2015-May-14 00:04 UTC
[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: <lists.llvm.org/pipermail/llvm-dev/attachments/20150514/1f75fcb8/attachment.html>
David Majnemer
2015-May-14 00:23 UTC
[LLVMdev] Potential "Buffer Overflow - Array Index Out of Bounds" issue
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 llvm.cs.uiuc.edu > lists.cs.uiuc.edu/mailman/listinfo/llvmdev > >-------------- next part -------------- An HTML attachment was scrubbed... URL: <lists.llvm.org/pipermail/llvm-dev/attachments/20150513/f7581836/attachment.html>