Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[mupdf]: malloc() does not return NULL upon out of memory... #1830

Closed
sebras opened this issue Sep 20, 2018 · 4 comments
Closed

[mupdf]: malloc() does not return NULL upon out of memory... #1830

sebras opened this issue Sep 20, 2018 · 4 comments

Comments

@sebras
Copy link
Contributor

sebras commented Sep 20, 2018

In bug #1817 @Dor1s suggested that I take a look at oss-fuzz #7803
Just like in oss-fuzz #5679 mupdf runs out of memory.

error: syntax error in content stream
error: syntax error in content stream
warning: cannot render glyph
==8== ERROR: libFuzzer: out-of-memory (used: 2976Mb; limit: 2048Mb)
   To change the out-of-memory limit use -rss_limit_mb=<N>

SUMMARY: libFuzzer: out-of-memory

I know from the faq that the fuzzer is restricted to 2Gbyte of memory. As we will always run into some fuzzed file that may cause us to attempt large allocations mupdf handles this by attempting to call malloc() and handle failures where it returns NULL and error out. What seems counterintuitive though is that malloc() in the fuzzer does not return NULL at this point, instead the fuzzer aborts like in the log above, thereby not allowing mupdf to do its error recovery.

I saw that in infra/base-images/base-runner/Dockerfile you set ASAN_OPTIONS="allocator_may_return_null=1", however this seems does not seem to have any effect as it aborts before malloc() is allowed to return NULL. How can I change my project so that malloc() does return NULL when it runs out of memory?

@Dor1s
Copy link
Contributor

Dor1s commented Sep 20, 2018

The root cause is that a fuzzing VM has more than 2GB and therefore malloc() doesn't fail to allocate more. I'm not sure if we can and should emulate memory constraints on the fuzzing process, but at the same time I see your point and it makes sense to me.

//cc @oliverchang @kcc @morehouse, I feel like we've had a discussion about another way to handle OOMs some time ago.

@oliverchang
Copy link
Collaborator

We may try to use cgroups to restrict the memory available to a process, but that adds complexity to our infrastructure and I'm not sure how well this will actually work in practice. It's not clear that malloc will always return NULL on oom anyway.

This may also mask legitimate OOMs that could be fixed, so I'm not sure this is ideal as a general solution for handlings OOMs either. Perhaps there are cases where we will silently miss a lot of coverage because of failed allocations.

I think our guidance thus far for handling large allocations is to set artificial limits in the library code before allocations (possibly under the "FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION" define which we set in our builders).

@sebras
Copy link
Contributor Author

sebras commented Sep 21, 2018

While looking for FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION I found this which got me thinking... Since MuPDF supports having a custom allocator (most of our dependencies use it too), perhaps the best option is for me to implement a custom allocator that keeps track of how much memory we've allocated, and if the total allocation surpasses some limit the custom allocator's malloc() will return NULL. I would need to keep track of every pointer and that memory blocks' size so I can deduct that size upon free() or realloc(0). However this doesn't resolve the issues that might arise from dependent libraries that do not allow for a custom allocator. I'm not sure how to fix those.

I did toy with disabling memory overcommit in my local docker container, but as @Dor1s confirms this wouldn't be a workable solution.

sebras added a commit to sebras/oss-fuzz that referenced this issue Sep 25, 2018
oliverchang pushed a commit that referenced this issue Sep 25, 2018
…) (#1832)

This fixes oss-fuzz #5679 and oss-fuzz #7803 for the mupdf project.
tmatth pushed a commit to tmatth/oss-fuzz that referenced this issue Oct 22, 2018
@sebras
Copy link
Contributor Author

sebras commented Apr 5, 2019

Commit 02c1436 resolved this issue for mupdf since it implemented a custom allocator for mupdf. I'm not sure why commit 02c1436 mentions two issue numbers, perhaps there was a mix up. Anyway, this means that this issue is now resolved, and I will close it.

Other projects probably would need to likewise implement their own custom allocators if they hit the same problem.

@sebras sebras closed this as completed Apr 5, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants