This is an archive of the discontinued LLVM Phabricator instance.

[ELF] - Do not crash when unable to parse ELF object file.
ClosedPublic

Authored by grimar on Oct 4 2016, 5:30 AM.

Details

Summary

createELFObj() may call error(...), for example when file is too short.
In that case header is not set and following line lead to crash:

EMachine = ELFObj.getHeader()->e_machine;

Patch fixes the issue.

Diff Detail

Repository
rL LLVM

Event Timeline

grimar updated this revision to Diff 73464.Oct 4 2016, 5:30 AM
grimar retitled this revision from to [ELF] - Do not crash when unable to parse ELF object file..
grimar updated this object.
grimar added reviewers: ruiu, rafael, davide.
grimar added subscribers: llvm-commits, grimar, evgeny777.
grimar added inline comments.
ELF/InputFiles.cpp
60 ↗(On Diff #73464)

I wonder may be we can just make this fatal() ?
It is not called when file not found, but only when there is something wrong with it.

ruiu added inline comments.Oct 4 2016, 9:25 AM
ELF/InputFiles.cpp
73–74 ↗(On Diff #73464)

This seems very weird. HasError should be used at major checkpoints, and we shouldn't have so many checkpoints. It is on the other hand called for every instantiation of the class.

grimar added inline comments.Oct 5 2016, 1:05 AM
ELF/InputFiles.cpp
73–74 ↗(On Diff #73464)

What about my comment above about switching error -> fatal in createELFObj() then ?
This should be consistent with latest ELF parsing hardening as we use fatal for broken objects and
createELFObj() now call error() even if ELF is broken, what I think is the only possible reason for fail.

ruiu added inline comments.Oct 5 2016, 1:52 PM
ELF/InputFiles.cpp
73–74 ↗(On Diff #73464)

That's probably better. We shouldn't do too much to try to recover from unrecoverable errors.

grimar updated this revision to Diff 73752.Oct 6 2016, 3:28 AM
  • Addressed review comments.
ruiu accepted this revision.Oct 6 2016, 1:15 PM
ruiu edited edge metadata.

LGTM

This revision is now accepted and ready to land.Oct 6 2016, 1:15 PM
rafael accepted this revision.Oct 6 2016, 2:34 PM
rafael edited edge metadata.

LGTM

This revision was automatically updated to reflect the committed changes.