[PATCH] D40029: [ELF] Only add contiguous isRelro sections to PT_GNU_RELRO PHDR

[Rui Ueyama via Phabricator via llvm-commits]

ruiu added a comment.

> Hmmm, I think that approach will be difficult for a couple of reasons. The first is that I think it will be difficult to get all the existing dynamic loaders to change unless we could think of an important use-case that can't be solved by just a single segment. I think that if I were a dynamic loader author I'd say "Why do you want multiple non-contiguous RELRO segments in an ELF file designed for an OS with a standard image layout? Why do I need to add complexity for an edge case?". The second is that there is a large lag time between getting a patch accepted in glibc and it getting deployed, for LTS releases we could be looking at several years before loaders that could support multiple RELRO segments.

I don't know if it is actually a problem. In this patch, you are attempting to add a RELRO segment only for the first contiguous regions that should be RELRO. My understanding is that the existing loader works OK with multiple RELRO segments, but only the first RELRO segment takes effect. That means, if you create multiple RELRO segments, that effectively works the same as creating a RELRO for the first contiguous region, no? If that's the case, creating multiple RELRO should fix the crash bug that you are trying to fix.

By doing that, we can buy time to fix dynamic loaders. As a result, the situation will be (1) if you are using an old loader, your binary still work, but the second and further RELRO segments are ignored, and (2) if you are using a newer loader, all RELRO segments are processed.

The above was my plan. What am I missing?


https://reviews.llvm.org/D40029