[PATCH] D43819: [ELF] - Restrict section offsets that exceeds file size.

George Rimar via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Mar 2 04:11:15 PST 2018


grimar added a comment.

In https://reviews.llvm.org/D43819#1025104, @jhenderson wrote:

> In https://reviews.llvm.org/D43819#1025064, @grimar wrote:
>
> > I am not sure I understand the use case, sorry. We normally can not have an empty section at the end of the file, because we place section header table there.
> >  If somehow section offset is damaged during calculation because of overflows, then `((Sec->Offset >= FileSize) || (Sec->Offset + Sec->Size >= FileSize)` check will catch it and report an error to prevent
> >  mapped buffer overflow and crash during writing. That is what this code supposed to do. What am I missing?
>
>
> It's a correctness thing again. Ultimately, you are correct, we //currently// can't generate sections at the end of the file, because the section header table is written there. However, this may not always be the case, and then we'd have a bug. Two potential future use cases might be a) writing the section header table earlier in the file (although admittedly I don't know why we would, but it is valid), or b) not writing the section header table at all (since linked ELFs don't require section headers in order to be run).
>  Sec->Offset == FileSize (or Sec->Offset + Sec->Size == FileSize - note that this latter case actually applies to non-empty sections too).
>
> We don't need to add any additional code to get this. Simply change the '>=' to '>' in the comparisons.


I see now. Thank you for explanation. I'll change the comparsions as suggested, but I think
probably will be unable to add test for that. Can not imagine reasonable test since we have section headers currently.


https://reviews.llvm.org/D43819





More information about the llvm-commits mailing list