<div dir="ltr"><br><br><div class="gmail_quote"><div dir="ltr">On Fri, Sep 8, 2017 at 10:06 AM Kostya Serebryany <<a href="mailto:kcc@google.com">kcc@google.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Fri, Sep 8, 2017 at 9:36 AM, David Blaikie <span dir="ltr"><<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>></span> wrote:<br></div></div></div><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><span class="m_-8904025554993892206gmail-"><div dir="ltr">On Thu, Sep 7, 2017 at 6:38 PM Kostya Serebryany <<a href="mailto:kcc@google.com" target="_blank">kcc@google.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Thu, Sep 7, 2017 at 9:14 AM, David Blaikie <span dir="ltr"><<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr">maybe this is unreasonable, but could libFuzzer/asan/something mock out malloc & have it fail for large allocations </div></blockquote><div><br></div></div></div></div><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div>Depending on your definition of 'fail'. </div><div>libFuzzer has a flag -rss_limit_mb (default: 2048) that will case the process to crash (i.e. report a bug) on </div><div> a) any single allocation exceeding the limit (yes, libFuzzer intercepts malloc) or</div><div> b) if the total rss exceeds the limit.</div><div><br></div><div>asan has another flag (allocator_may_return_null, off by default) that will cause malloc to return 0 on large allocations instead of crashing. </div></div></div></div></blockquote></span></div></div></blockquote></div></div></div><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><div><br>That ^ seems like a good thing/approach.</div></div></div></blockquote><div><br></div><div> </div><div>Not sure I understand you. </div><div>Is allocator_may_return_null=0 a good approach or allocator_may_return_null=1? </div></div></div></div></blockquote><div><br>allocator_may_return_null=1, I think. But I could be wrong/no doubt other folks have other ideas.<br> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div><br></div><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><div>(though we're sort of trained that checking for allocation failure is almost pointless because modern systems don't actually allocate at the time & wait to page fault the memory in (I'm sure I'm using the terminology incorrectly, but hopefully makes enough sense) - but seems like a possible necessary/useful tradeoff for fuzzing rather than every program needing to implement/enforce a fixed memory bound)<br> </div><div><div class="m_-8904025554993892206gmail-h5"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div>By default asan crashes on HUGE allocations (e.g. 1Tb) but doesn't fail on anything it can actually allocate. </div><div><br></div><div>--kcc </div></div></div></div><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div><br></div><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr">rather than having applications need to support such a bound themselves?</div><div class="m_-8904025554993892206gmail-m_-7048744295831608324m_-6815502067883720512gmail-HOEnZb"><div class="m_-8904025554993892206gmail-m_-7048744295831608324m_-6815502067883720512gmail-h5"><br><div class="gmail_quote"><div dir="ltr">On Thu, Sep 7, 2017 at 9:10 AM Kostya Serebryany <<a href="mailto:kcc@google.com" target="_blank">kcc@google.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Thu, Sep 7, 2017 at 3:17 AM, George Rimar <span dir="ltr"><<a href="mailto:grimar@accesssoftek.com" target="_blank">grimar@accesssoftek.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div dir="ltr" style="font-size:12pt;color:rgb(0,0,0);background-color:rgb(255,255,255);font-family:Calibri,Arial,Helvetica,sans-serif">
<p>Compressing/decompressing can be used for any sections in theory, </p>
<p>but in practice <span style="font-size:12pt">use</span><span style="font-size:12pt">d for .debug_* ones only I think (not sure though). </span></p>
<p><span style="font-size:12pt"></span><span style="font-size:12pt">Afaik LLVM does not support DWARF64 now.</span></p>
<p><span style="font-size:12pt">Given above we probably can limit decompressed size to 4gb and it be reasonable limit ?</span></p>
<p><span style="font-size:12pt"><br></span></p></div></blockquote></div></div></div><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div>It might be reasonable as the default limit, but for fuzzing 4Gb is a bit too expensive. </div><div>The default total rss limit in libFuzzer is 2Gb, and the same limit is enforced on oss-fuzz. </div><div>So, there should be a way to enforce another limit (1Gb sounds best to me)</div></div></div></div><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr" style="font-size:12pt;color:rgb(0,0,0);background-color:rgb(255,255,255);font-family:Calibri,Arial,Helvetica,sans-serif"><p><span style="font-size:12pt">
</span></p>
<p><span style="font-size:12pt">Another point (already mentioned in different thread) is that</span></p>
<div class="gmail_extra">
<div class="gmail_quote">install_bad_alloc_error_handler API looks completely dead then ? </div>
<div class="gmail_quote">S<span style="font-size:12pt">ince it looks not possible to use it currently without breaking </span><span style="color:rgb(33,33,33);font-family:"Segoe UI","Segoe WP","Segoe UI WPC",Tahoma,Arial,sans-serif;font-size:15px">ASan</span><span style="font-size:12pt">.</span></div>
<div class="gmail_quote"><br>
</div>
</div>
<div id="m_-8904025554993892206gmail-m_-7048744295831608324m_-6815502067883720512gmail-m_-6634002833082946053m_-915388056702310940m_-1457283807461563210Signature">
<div name="divtagdefaultwrapper">
<div class="m_-8904025554993892206gmail-m_-7048744295831608324m_-6815502067883720512gmail-m_-6634002833082946053m_-915388056702310940m_-1457283807461563210BodyFragment"><font size="2">
<div class="m_-8904025554993892206gmail-m_-7048744295831608324m_-6815502067883720512gmail-m_-6634002833082946053m_-915388056702310940m_-1457283807461563210PlainText">Best regards,<br>
George | Developer | <span style="background-color:rgb(255,255,255);color:rgb(33,33,33);font-family:Calibri,sans-serif;font-size:13.3333px">Access Softek, Inc</span></div>
</font></div>
</div>
</div>
<div style="color:rgb(33,33,33)">
<hr style="display:inline-block;width:98%">
<div id="m_-8904025554993892206gmail-m_-7048744295831608324m_-6815502067883720512gmail-m_-6634002833082946053m_-915388056702310940m_-1457283807461563210divRplyFwdMsg" dir="ltr"><font face="Calibri, sans-serif" color="#000000" style="font-size:11pt"><b>От:</b> Kostya Serebryany <<a href="mailto:kcc@google.com" target="_blank">kcc@google.com</a>><br>
<b>Отправлено:</b> 7 сентября 2017 г. 3:26<br>
<b>Кому:</b> Jonas Devlieghere<br>
<b>Копия:</b> Vedant Kumar; LLVM Commits; George Rimar<br>
<b>Тема:</b> Re: [llvm] r312582 - Revert "[Decompression] Fail gracefully when out of memory"</font>
<div> </div>
</div><div><div class="m_-8904025554993892206gmail-m_-7048744295831608324m_-6815502067883720512gmail-m_-6634002833082946053m_-915388056702310940h5">
<div>
<div dir="ltr"><br>
<div class="gmail_extra"><br>
<div class="gmail_quote">On Wed, Sep 6, 2017 at 2:38 PM, Jonas Devlieghere <span dir="ltr">
<<a href="mailto:jdevlieghere@apple.com" target="_blank">jdevlieghere@apple.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div style="word-wrap:break-word">Hi Kostya,
<div><br>
</div>
<div>I completely agree with you that this is far from an optimal solution! However, I don’t really see an alternative here without a complete rewrite of how we currently do decompression. For this particular case, how would you decide whether the allocated
chuck is too big? </div>
</div>
</blockquote>
<div><br>
</div>
<div>It could be an arbitrary limit which can be redefined at compile-time or, better, with a run-time flag (and the default set to e.g. 1Gb?)</div>
<div> </div>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div style="word-wrap:break-word">
<div>Any limit would be arbitrary before doing the actual extraction. This is a pretty common problem with extraction
</div>
</div>
</blockquote>
<div><br>
</div>
<div>Oh yea. zip bombs they call it. </div>
<div> </div>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div style="word-wrap:break-word">
<div>and I *think* libz has some kind of work around for it, though I’d have to check to refresh my memory. We could probably do something similar in the decompressor, but that’d be a pretty big change and this seemed like a reasonable first step as opposed
to just crashing. <br>
</div>
</div>
</blockquote>
<div><br>
</div>
<div>This change doesn't really fix the problem though. </div>
<div>Yes, it might help if the requested allocation is e.g. 42Tb. </div>
<div>But what if the requested allocation is 3Gb? LibFuzzer will still die with OOM (default rss_limit_mb=2048). </div>
<div>What if the requested allocation is 1.99Gb? libFuzzer will dies due to OOM again, just because the process will consume 2Gb</div>
<div> </div>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div style="word-wrap:break-word">
<div>
<div><br>
</div>
<div>Here's the original differential <a href="https://reviews.llvm.org/D37447" target="_blank">
https://reviews.llvm.org/D37447</a></div>
<span class="m_-8904025554993892206gmail-m_-7048744295831608324m_-6815502067883720512gmail-m_-6634002833082946053m_-915388056702310940m_-1457283807461563210gmail-HOEnZb"><font color="#888888">
<div><br>
</div>
</font></span>
<div><span class="m_-8904025554993892206gmail-m_-7048744295831608324m_-6815502067883720512gmail-m_-6634002833082946053m_-915388056702310940m_-1457283807461563210gmail-HOEnZb"><font color="#888888">Jonas</font></span>
<div>
<div class="m_-8904025554993892206gmail-m_-7048744295831608324m_-6815502067883720512gmail-m_-6634002833082946053m_-915388056702310940m_-1457283807461563210gmail-h5"><br>
<div><br>
<blockquote type="cite">
<div>On Sep 6, 2017, at 9:37 PM, Kostya Serebryany <<a href="mailto:kcc@google.com" target="_blank">kcc@google.com</a>> wrote:</div>
<br class="m_-8904025554993892206gmail-m_-7048744295831608324m_-6815502067883720512gmail-m_-6634002833082946053m_-915388056702310940m_-1457283807461563210gmail-m_9117679352079337087Apple-interchange-newline">
<div>
<div dir="ltr">I missed the original commit but I'm glad it's reverted.
<div>I think it's a really bad practice to allocate huge chunks of RAM (petabytes?) and rely on the new handler to bark. </div>
<div>Instead, we should simply not try to allocate large chunks. In most cases if the compiler-related too wants to allocate many Gb of RAM at once it's an indication of a logical bug. </div>
</div>
<div class="gmail_extra"><br>
<div class="gmail_quote">On Tue, Sep 5, 2017 at 3:04 PM, Vedant Kumar via llvm-commits
<span dir="ltr"><<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
Author: vedantk<br>
Date: Tue Sep 5 15:04:00 2017<br>
New Revision: 312582<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=312582&view=rev" rel="noreferrer" target="_blank">
http://llvm.org/viewvc/llvm-project?rev=312582&view=rev</a><br>
Log:<br>
Revert "[Decompression] Fail gracefully when out of memory"<br>
<br>
This reverts commit r312526.<br>
<br>
Revert "Fix test/DebugInfo/dwarfdump-decompression-invalid-size.test"<br>
<br>
This reverts commit r312527.<br>
<br>
It causes an ASan failure:<br>
<a href="http://lab.llvm.org:8080/green/job/clang-stage2-cmake-RgSan_check/4150" rel="noreferrer" target="_blank">http://lab.llvm.org:8080/green/job/clang-stage2-cmake-RgSan_check/4150</a><br>
<br>
Removed:<br>
llvm/trunk/test/DebugInfo/Inputs/dwarfdump-decompression-invalid-size.elf-x86-64<br>
llvm/trunk/test/DebugInfo/dwarfdump-decompression-invalid-size.test<br>
Modified:<br>
llvm/trunk/include/llvm/Object/Decompressor.h<br>
llvm/trunk/lib/Object/Decompressor.cpp<br>
<br>
Modified: llvm/trunk/include/llvm/Object/Decompressor.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Object/Decompressor.h?rev=312582&r1=312581&r2=312582&view=diff" rel="noreferrer" target="_blank">
http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Object/Decompressor.h?rev=312582&r1=312581&r2=312582&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/include/llvm/Object/Decompressor.h (original)<br>
+++ llvm/trunk/include/llvm/Object/Decompressor.h Tue Sep 5 15:04:00 2017<br>
@@ -13,7 +13,6 @@<br>
#include "llvm/ADT/SmallString.h"<br>
#include "llvm/ADT/StringRef.h"<br>
#include "llvm/Object/ObjectFile.h"<br>
-#include "llvm/Support/ErrorHandling.h"<br>
<br>
namespace llvm {<br>
namespace object {<br>
@@ -32,9 +31,7 @@ public:<br>
/// @brief Resize the buffer and uncompress section data into it.<br>
/// @param Out Destination buffer.<br>
template <class T> Error resizeAndDecompress(T &Out) {<br>
- install_bad_alloc_error_handler(outOfMemoryHandler, this);<br>
Out.resize(DecompressedSize);<br>
- remove_bad_alloc_error_handler();<br>
return decompress({Out.data(), (size_t)DecompressedSize});<br>
}<br>
<br>
@@ -55,14 +52,11 @@ public:<br>
static bool isGnuStyle(StringRef Name);<br>
<br>
private:<br>
- static void outOfMemoryHandler(void *Data, const std::string &Message, bool);<br>
-<br>
- Decompressor(StringRef Name, StringRef Data);<br>
+ Decompressor(StringRef Data);<br>
<br>
Error consumeCompressedGnuHeader();<br>
Error consumeCompressedZLibHeader(bool Is64Bit, bool IsLittleEndian);<br>
<br>
- StringRef SectionName;<br>
StringRef SectionData;<br>
uint64_t DecompressedSize;<br>
};<br>
<br>
Modified: llvm/trunk/lib/Object/Decompressor.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Object/Decompressor.cpp?rev=312582&r1=312581&r2=312582&view=diff" rel="noreferrer" target="_blank">
http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Object/Decompressor.cpp?rev=312582&r1=312581&r2=312582&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/Object/Decompressor.cpp (original)<br>
+++ llvm/trunk/lib/Object/Decompressor.cpp Tue Sep 5 15:04:00 2017<br>
@@ -23,7 +23,7 @@ Expected<Decompressor> Decompressor::cre<br>
if (!zlib::isAvailable())<br>
return createError("zlib is not available");<br>
<br>
- Decompressor D(Name, Data);<br>
+ Decompressor D(Data);<br>
Error Err = isGnuStyle(Name) ? D.consumeCompressedGnuHeader()<br>
: D.consumeCompressedZLibHeader(Is64Bit, IsLE);<br>
if (Err)<br>
@@ -31,8 +31,8 @@ Expected<Decompressor> Decompressor::cre<br>
return D;<br>
}<br>
<br>
-Decompressor::Decompressor(StringRef Name, StringRef Data)<br>
- : SectionName(Name), SectionData(Data), DecompressedSize(0) {}<br>
+Decompressor::Decompressor(StringRef Data)<br>
+ : SectionData(Data), DecompressedSize(0) {}<br>
<br>
Error Decompressor::consumeCompressedGnuHeader() {<br>
if (!SectionData.startswith("ZLIB"))<br>
@@ -92,11 +92,3 @@ Error Decompressor::decompress(MutableAr<br>
size_t Size = Buffer.size();<br>
return zlib::uncompress(SectionData, Buffer.data(), Size);<br>
}<br>
-<br>
-void Decompressor::outOfMemoryHandler(void *Data, const std::string &Message,<br>
- bool) {<br>
- const auto *D = static_cast<const Decompressor *>(Data);<br>
- report_fatal_error("decompression of '" + Twine(D->SectionName) +<br>
- "' failed: unable to allocate " +<br>
- Twine(D->DecompressedSize) + " bytes.");<br>
-}<br>
<br>
Removed: llvm/trunk/test/DebugInfo/Inputs/dwarfdump-decompression-invalid-size.elf-x86-64<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/DebugInfo/Inputs/dwarfdump-decompression-invalid-size.elf-x86-64?rev=312581&view=auto" rel="noreferrer" target="_blank">
http://llvm.org/viewvc/llvm-project/llvm/trunk/test/DebugInfo/Inputs/dwarfdump-decompression-invalid-size.elf-x86-64?rev=312581&view=auto</a><br>
==============================================================================<br>
Binary files llvm/trunk/test/DebugInfo/Inputs/dwarfdump-decompression-invalid-size.elf-x86-64 (original) and llvm/trunk/test/DebugInfo/Inputs/dwarfdump-decompression-invalid-size.elf-x86-64 (removed) differ<br>
<br>
Removed: llvm/trunk/test/DebugInfo/dwarfdump-decompression-invalid-size.test<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/DebugInfo/dwarfdump-decompression-invalid-size.test?rev=312581&view=auto" rel="noreferrer" target="_blank">
http://llvm.org/viewvc/llvm-project/llvm/trunk/test/DebugInfo/dwarfdump-decompression-invalid-size.test?rev=312581&view=auto</a><br>
==============================================================================<br>
--- llvm/trunk/test/DebugInfo/dwarfdump-decompression-invalid-size.test (original)<br>
+++ llvm/trunk/test/DebugInfo/dwarfdump-decompression-invalid-size.test (removed)<br>
@@ -1,15 +0,0 @@<br>
-REQUIRES: zlib<br>
-<br>
-// dwarfdump-decompression-invalid-size.elf-x86-64 is prepared using following<br>
-// source code and invocation:<br>
-// test.cpp:<br>
-// int main() { return 0; }<br>
-//<br>
-// gcc test.cpp -o out -g -Wl,--compress-debug-sections,zlib<br>
-//<br>
-// After that result object was modified manually. Decompressed size of<br>
-// .debug_frame section was changed to 0xffffffffffffffff in compression<br>
-// header.<br>
-RUN: not llvm-dwarfdump %p/Inputs/dwarfdump-decompression-invalid-size.elf-x86-64 2>&1 | FileCheck %s<br>
-<br>
-CHECK: decompression of '.debug_frame' failed: unable to allocate 18446744073709551615 bytes.<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
</blockquote>
</div>
<br>
</div>
</div>
</blockquote>
</div>
<br>
</div>
</div>
</div>
</div>
</div>
</blockquote>
</div>
<br>
</div>
</div>
</div>
</div></div></div>
</div>
</blockquote></div></div></div></blockquote></div>
</div></div></blockquote></div></div></div></blockquote></div></div></div></div>
</blockquote></div></div></div></blockquote></div></div>