[llvm] r312582 - Revert "[Decompression] Fail gracefully when out of memory"
Kostya Serebryany via llvm-commits
llvm-commits at lists.llvm.org
Fri Sep 8 10:06:32 PDT 2017
On Fri, Sep 8, 2017 at 9:36 AM, David Blaikie <dblaikie at gmail.com> wrote:
>
>
> On Thu, Sep 7, 2017 at 6:38 PM Kostya Serebryany <kcc at google.com> wrote:
>
>> On Thu, Sep 7, 2017 at 9:14 AM, David Blaikie <dblaikie at gmail.com> wrote:
>>
>>> maybe this is unreasonable, but could libFuzzer/asan/something mock out
>>> malloc & have it fail for large allocations
>>>
>>
>> Depending on your definition of 'fail'.
>> libFuzzer has a flag -rss_limit_mb (default: 2048) that will case the
>> process to crash (i.e. report a bug) on
>> a) any single allocation exceeding the limit (yes, libFuzzer intercepts
>> malloc) or
>> b) if the total rss exceeds the limit.
>>
>> asan has another flag (allocator_may_return_null, off by default) that
>> will cause malloc to return 0 on large allocations instead of crashing.
>>
>
> That ^ seems like a good thing/approach.
>
Not sure I understand you.
Is allocator_may_return_null=0 a good approach or
allocator_may_return_null=1?
> (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)
>
>
>> By default asan crashes on HUGE allocations (e.g. 1Tb) but doesn't fail
>> on anything it can actually allocate.
>>
>> --kcc
>>
>>
>>
>>
>>> rather than having applications need to support such a bound themselves?
>>>
>>> On Thu, Sep 7, 2017 at 9:10 AM Kostya Serebryany <kcc at google.com> wrote:
>>>
>>>> On Thu, Sep 7, 2017 at 3:17 AM, George Rimar <grimar at accesssoftek.com>
>>>> wrote:
>>>>
>>>>> Compressing/decompressing can be used for any sections in theory,
>>>>>
>>>>> but in practice used for .debug_* ones only I think (not sure
>>>>> though).
>>>>>
>>>>> Afaik LLVM does not support DWARF64 now.
>>>>>
>>>>> Given above we probably can limit decompressed size to 4gb and it be
>>>>> reasonable limit ?
>>>>>
>>>>>
>>>>> It might be reasonable as the default limit, but for fuzzing 4Gb is a
>>>> bit too expensive.
>>>> The default total rss limit in libFuzzer is 2Gb, and the same limit is
>>>> enforced on oss-fuzz.
>>>> So, there should be a way to enforce another limit (1Gb sounds best to
>>>> me)
>>>>
>>>>
>>>>> Another point (already mentioned in different thread) is that
>>>>> install_bad_alloc_error_handler API looks completely dead then ?
>>>>> Since it looks not possible to use it currently without breaking ASan
>>>>> .
>>>>>
>>>>> Best regards,
>>>>> George | Developer | Access Softek, Inc
>>>>> ------------------------------
>>>>> *От:* Kostya Serebryany <kcc at google.com>
>>>>> *Отправлено:* 7 сентября 2017 г. 3:26
>>>>> *Кому:* Jonas Devlieghere
>>>>> *Копия:* Vedant Kumar; LLVM Commits; George Rimar
>>>>> *Тема:* Re: [llvm] r312582 - Revert "[Decompression] Fail gracefully
>>>>> when out of memory"
>>>>>
>>>>>
>>>>>
>>>>> On Wed, Sep 6, 2017 at 2:38 PM, Jonas Devlieghere <
>>>>> jdevlieghere at apple.com> wrote:
>>>>>
>>>>>> Hi Kostya,
>>>>>>
>>>>>> 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?
>>>>>>
>>>>>
>>>>> 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?)
>>>>>
>>>>>
>>>>>> Any limit would be arbitrary before doing the actual extraction. This
>>>>>> is a pretty common problem with extraction
>>>>>>
>>>>>
>>>>> Oh yea. zip bombs they call it.
>>>>>
>>>>>
>>>>>> 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.
>>>>>>
>>>>>
>>>>> This change doesn't really fix the problem though.
>>>>> Yes, it might help if the requested allocation is e.g. 42Tb.
>>>>> But what if the requested allocation is 3Gb? LibFuzzer will still die
>>>>> with OOM (default rss_limit_mb=2048).
>>>>> What if the requested allocation is 1.99Gb? libFuzzer will dies due to
>>>>> OOM again, just because the process will consume 2Gb
>>>>>
>>>>>
>>>>>>
>>>>>> Here's the original differential https://reviews.llvm.org/D37447
>>>>>>
>>>>>> Jonas
>>>>>>
>>>>>>
>>>>>> On Sep 6, 2017, at 9:37 PM, Kostya Serebryany <kcc at google.com> wrote:
>>>>>>
>>>>>> I missed the original commit but I'm glad it's reverted.
>>>>>> I think it's a really bad practice to allocate huge chunks of RAM
>>>>>> (petabytes?) and rely on the new handler to bark.
>>>>>> 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.
>>>>>>
>>>>>> On Tue, Sep 5, 2017 at 3:04 PM, Vedant Kumar via llvm-commits <
>>>>>> llvm-commits at lists.llvm.org> wrote:
>>>>>>
>>>>>>> Author: vedantk
>>>>>>> Date: Tue Sep 5 15:04:00 2017
>>>>>>> New Revision: 312582
>>>>>>>
>>>>>>> URL: http://llvm.org/viewvc/llvm-project?rev=312582&view=rev
>>>>>>> Log:
>>>>>>> Revert "[Decompression] Fail gracefully when out of memory"
>>>>>>>
>>>>>>> This reverts commit r312526.
>>>>>>>
>>>>>>> Revert "Fix test/DebugInfo/dwarfdump-decompression-invalid-size.
>>>>>>> test"
>>>>>>>
>>>>>>> This reverts commit r312527.
>>>>>>>
>>>>>>> It causes an ASan failure:
>>>>>>> http://lab.llvm.org:8080/green/job/clang-stage2-cmake-
>>>>>>> RgSan_check/4150
>>>>>>>
>>>>>>> Removed:
>>>>>>> llvm/trunk/test/DebugInfo/Inputs/dwarfdump-
>>>>>>> decompression-invalid-size.elf-x86-64
>>>>>>> llvm/trunk/test/DebugInfo/dwarfdump-decompression-
>>>>>>> invalid-size.test
>>>>>>> Modified:
>>>>>>> llvm/trunk/include/llvm/Object/Decompressor.h
>>>>>>> llvm/trunk/lib/Object/Decompressor.cpp
>>>>>>>
>>>>>>> Modified: llvm/trunk/include/llvm/Object/Decompressor.h
>>>>>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/
>>>>>>> llvm/Object/Decompressor.h?rev=312582&r1=312581&r2=312582&view=diff
>>>>>>> ============================================================
>>>>>>> ==================
>>>>>>> --- llvm/trunk/include/llvm/Object/Decompressor.h (original)
>>>>>>> +++ llvm/trunk/include/llvm/Object/Decompressor.h Tue Sep 5
>>>>>>> 15:04:00 2017
>>>>>>> @@ -13,7 +13,6 @@
>>>>>>> #include "llvm/ADT/SmallString.h"
>>>>>>> #include "llvm/ADT/StringRef.h"
>>>>>>> #include "llvm/Object/ObjectFile.h"
>>>>>>> -#include "llvm/Support/ErrorHandling.h"
>>>>>>>
>>>>>>> namespace llvm {
>>>>>>> namespace object {
>>>>>>> @@ -32,9 +31,7 @@ public:
>>>>>>> /// @brief Resize the buffer and uncompress section data into it.
>>>>>>> /// @param Out Destination buffer.
>>>>>>> template <class T> Error resizeAndDecompress(T &Out) {
>>>>>>> - install_bad_alloc_error_handler(outOfMemoryHandler, this);
>>>>>>> Out.resize(DecompressedSize);
>>>>>>> - remove_bad_alloc_error_handler();
>>>>>>> return decompress({Out.data(), (size_t)DecompressedSize});
>>>>>>> }
>>>>>>>
>>>>>>> @@ -55,14 +52,11 @@ public:
>>>>>>> static bool isGnuStyle(StringRef Name);
>>>>>>>
>>>>>>> private:
>>>>>>> - static void outOfMemoryHandler(void *Data, const std::string
>>>>>>> &Message, bool);
>>>>>>> -
>>>>>>> - Decompressor(StringRef Name, StringRef Data);
>>>>>>> + Decompressor(StringRef Data);
>>>>>>>
>>>>>>> Error consumeCompressedGnuHeader();
>>>>>>> Error consumeCompressedZLibHeader(bool Is64Bit, bool
>>>>>>> IsLittleEndian);
>>>>>>>
>>>>>>> - StringRef SectionName;
>>>>>>> StringRef SectionData;
>>>>>>> uint64_t DecompressedSize;
>>>>>>> };
>>>>>>>
>>>>>>> Modified: llvm/trunk/lib/Object/Decompressor.cpp
>>>>>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Object/
>>>>>>> Decompressor.cpp?rev=312582&r1=312581&r2=312582&view=diff
>>>>>>> ============================================================
>>>>>>> ==================
>>>>>>> --- llvm/trunk/lib/Object/Decompressor.cpp (original)
>>>>>>> +++ llvm/trunk/lib/Object/Decompressor.cpp Tue Sep 5 15:04:00 2017
>>>>>>> @@ -23,7 +23,7 @@ Expected<Decompressor> Decompressor::cre
>>>>>>> if (!zlib::isAvailable())
>>>>>>> return createError("zlib is not available");
>>>>>>>
>>>>>>> - Decompressor D(Name, Data);
>>>>>>> + Decompressor D(Data);
>>>>>>> Error Err = isGnuStyle(Name) ? D.consumeCompressedGnuHeader()
>>>>>>> : D.consumeCompressedZLibHeader(Is64Bit,
>>>>>>> IsLE);
>>>>>>> if (Err)
>>>>>>> @@ -31,8 +31,8 @@ Expected<Decompressor> Decompressor::cre
>>>>>>> return D;
>>>>>>> }
>>>>>>>
>>>>>>> -Decompressor::Decompressor(StringRef Name, StringRef Data)
>>>>>>> - : SectionName(Name), SectionData(Data), DecompressedSize(0) {}
>>>>>>> +Decompressor::Decompressor(StringRef Data)
>>>>>>> + : SectionData(Data), DecompressedSize(0) {}
>>>>>>>
>>>>>>> Error Decompressor::consumeCompressedGnuHeader() {
>>>>>>> if (!SectionData.startswith("ZLIB"))
>>>>>>> @@ -92,11 +92,3 @@ Error Decompressor::decompress(MutableAr
>>>>>>> size_t Size = Buffer.size();
>>>>>>> return zlib::uncompress(SectionData, Buffer.data(), Size);
>>>>>>> }
>>>>>>> -
>>>>>>> -void Decompressor::outOfMemoryHandler(void *Data, const
>>>>>>> std::string &Message,
>>>>>>> - bool) {
>>>>>>> - const auto *D = static_cast<const Decompressor *>(Data);
>>>>>>> - report_fatal_error("decompression of '" + Twine(D->SectionName) +
>>>>>>> - "' failed: unable to allocate " +
>>>>>>> - Twine(D->DecompressedSize) + " bytes.");
>>>>>>> -}
>>>>>>>
>>>>>>> Removed: llvm/trunk/test/DebugInfo/Inputs/dwarfdump-
>>>>>>> decompression-invalid-size.elf-x86-64
>>>>>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/
>>>>>>> DebugInfo/Inputs/dwarfdump-decompression-invalid-size.
>>>>>>> elf-x86-64?rev=312581&view=auto
>>>>>>> ============================================================
>>>>>>> ==================
>>>>>>> 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
>>>>>>>
>>>>>>> Removed: llvm/trunk/test/DebugInfo/dwarfdump-decompression-
>>>>>>> invalid-size.test
>>>>>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/
>>>>>>> DebugInfo/dwarfdump-decompression-invalid-size.
>>>>>>> test?rev=312581&view=auto
>>>>>>> ============================================================
>>>>>>> ==================
>>>>>>> --- llvm/trunk/test/DebugInfo/dwarfdump-decompression-invalid-size.test
>>>>>>> (original)
>>>>>>> +++ llvm/trunk/test/DebugInfo/dwarfdump-decompression-invalid-size.test
>>>>>>> (removed)
>>>>>>> @@ -1,15 +0,0 @@
>>>>>>> -REQUIRES: zlib
>>>>>>> -
>>>>>>> -// dwarfdump-decompression-invalid-size.elf-x86-64 is prepared
>>>>>>> using following
>>>>>>> -// source code and invocation:
>>>>>>> -// test.cpp:
>>>>>>> -// int main() { return 0; }
>>>>>>> -//
>>>>>>> -// gcc test.cpp -o out -g -Wl,--compress-debug-sections,zlib
>>>>>>> -//
>>>>>>> -// After that result object was modified manually. Decompressed
>>>>>>> size of
>>>>>>> -// .debug_frame section was changed to 0xffffffffffffffff in
>>>>>>> compression
>>>>>>> -// header.
>>>>>>> -RUN: not llvm-dwarfdump %p/Inputs/dwarfdump-
>>>>>>> decompression-invalid-size.elf-x86-64 2>&1 | FileCheck %s
>>>>>>> -
>>>>>>> -CHECK: decompression of '.debug_frame' failed: unable to allocate
>>>>>>> 18446744073709551615 bytes.
>>>>>>>
>>>>>>>
>>>>>>> _______________________________________________
>>>>>>> llvm-commits mailing list
>>>>>>> llvm-commits at lists.llvm.org
>>>>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170908/95102599/attachment.html>
More information about the llvm-commits
mailing list