Re: PATCH: PR ld/5149: Set PF_X for PT_GNU_RELRO segment only when needed

[Roland McGrath <roland at redhat dot com>]

Sorry for the slow reply.

> On Wed, Oct 24, 2007 at 03:25:59PM +0930, Alan Modra wrote:
> > On Mon, Oct 22, 2007 at 03:31:09PM -0700, H.J. Lu wrote:
> > > 	PR ld/5149
> > > 	* elf.c (assign_file_positions_for_non_load_sections): Set
> > > 	PF_X for PT_GNU_RELRO segment only if a section has
> > > 	SHF_EXECINSTR.
> > 
> > I took a look at what glibc ld.so does with PT_GNU_RELRO and it
> > appears that p_flags are completely ignored.  The mprotect call
> > always uses PROT_READ.  So I don't really see the point of this
> > patch.
> > 
> 
> I don't think we should use glibc as spec here. If it is case,
> why not just set it to PROT_READ?
> 
> The question is if PT_GNU_RELRO can have the PF_X bit set and
> what it means. Roland, do you have comments?

My understanding from talking with Jakub is that it can make sense to have
SHF_EXECINSTR sections fall inside the PT_GNU_RELRO range in some cases.
Those are when PLT has to be writable for reloc, but using -z relro -z now.
That can arise on maybe sparc, alpha, and ppc32 -mbss-plt, that I know of.
(For those targets, with -z relro but without -z now, the relro range must
not cover the executable sections in question, since they are changed
dynamically after startup.)

For the targets with no read-only PLT ABI available, it really is
legitimate and unavoidable (for now) to have the executable-in-relro case
under -z now.  To support this case, ld.so should change to honor the
p_flags of the PT_GNU_RELRO phdr, at least its PF_X bit (and I'm inclined
to make it follow exactly the combination of PF_R|PF_X given, in case there
is some all-PLT PF_X-only case in theory).  

The constraint I think should be required of the linker to call its phdrs
correct is that p_flags of the PT_GNU_RELRO is a subset of the p_flags of
the PT_LOAD into which it falls (so that an ld.so without PT_GNU_RELRO
processing will always provide sufficient access), that it does not include
PF_W, and that its PT_LOAD does include PF_W (so that the PT_GNU_RELRO has
any actual purpose at all).

If I'm correct about all this, and that there is no other legitimate case
for bits other than PF_R in PT_GNU_RELRO, then I will do the glibc part
(and fix eu-elflint not to complain for the case).


Thanks,
Roland