[musl] RELRO vs deferred binding

[musl] RELRO vs deferred binding

[Alexander Monakov]

Hi,

my understanding is that deferred binding currently does not work if any of the
deferred relocations fall into the RELRO segment.

I am raising this because Alpine may try enabling -fno-plt globally, and
with -fno-plt non-PLT GOT relocations will be in RELRO, and affected Xorg
modules will fail to load, again, but now with a segfault in the dynamic
linker, presumably (I have not tested this).

Alexander

Re: [musl] RELRO vs deferred binding

[Rich Felker]

On Wed, Mar 31, 2021 at 05:27:09PM +0300, Alexander Monakov wrote:
> Hi,
> 
> my understanding is that deferred binding currently does not work if any of the
> deferred relocations fall into the RELRO segment.
> 
> I am raising this because Alpine may try enabling -fno-plt globally, and
> with -fno-plt non-PLT GOT relocations will be in RELRO, and affected Xorg
> modules will fail to load, again, but now with a segfault in the dynamic
> linker, presumably (I have not tested this).

Thanks for raising this. I think deferred binding needs to be updated
either to ignore RELRO if there are outstanding relocations (possibly
deferring it until they are all resolved) or to unprotect and
reprotect on every incremental link. (This could be optimized out and
preserve some further safety by scanning the outstanding relocation
table and skipping the unprotect/reprotect if none of them lie in the
RELRO range.)

Rich

Re: [musl] RELRO vs deferred binding

[Alexander Monakov]

On Wed, 31 Mar 2021, Rich Felker wrote:

> Thanks for raising this. I think deferred binding needs to be updated
> either to ignore RELRO if there are outstanding relocations (possibly
> deferring it until they are all resolved)

This seems undesirable as it leaves GOT unprotected for the rest of
run time if unresolved relocations remain.

> or to unprotect and
> reprotect on every incremental link. (This could be optimized out and
> preserve some further safety by scanning the outstanding relocation
> table and skipping the unprotect/reprotect if none of them lie in the
> RELRO range.)

Even better might be to do relocation normally and lazily unprotect RELRO
on first relocation that needs that, then reprotect once done with that DSO.
(i.e. without doing an additional scan, like your parenthesized statement
seems to suggest)

Alexander

Re: [musl] RELRO vs deferred binding

[Rich Felker]

On Wed, Mar 31, 2021 at 05:51:26PM +0300, Alexander Monakov wrote:
> On Wed, 31 Mar 2021, Rich Felker wrote:
> 
> > Thanks for raising this. I think deferred binding needs to be updated
> > either to ignore RELRO if there are outstanding relocations (possibly
> > deferring it until they are all resolved)
> 
> This seems undesirable as it leaves GOT unprotected for the rest of
> run time if unresolved relocations remain.

Yes, but in practice this is only for broken xorg modules and the
unresolved relocations are resolved by the time any attack-surface
code runs, no? Still I agree it's better to avoid this.

> > or to unprotect and
> > reprotect on every incremental link. (This could be optimized out and
> > preserve some further safety by scanning the outstanding relocation
> > table and skipping the unprotect/reprotect if none of them lie in the
> > RELRO range.)
> 
> Even better might be to do relocation normally and lazily unprotect RELRO
> on first relocation that needs that, then reprotect once done with that DSO.
> (i.e. without doing an additional scan, like your parenthesized statement
> seems to suggest)

That puts the additional branch/logic inside the hot path used by all
relocation processing rather than a path that's relegated to just
outstanding relocations on libraries that didn't declare their
dependencies properly.

My version looks something like, inside the for loop in
redo_lazy_relocs:

	need_unprotect = 0;
	for (i=0; i<size; i+=3);
		if ((uintptr_t)laddr(p, p->lazy[i])-relro_start < relro_end)
			need_unprotect = 1;
	if (need_unprotect) mprotect(...);
	do_relocs(...);
	if (need_unprotect) mprotect(...);

Does that look reasonable?

Rich

Re: [musl] RELRO vs deferred binding

[Alexander Monakov]

On Wed, 31 Mar 2021, Rich Felker wrote:

> > This seems undesirable as it leaves GOT unprotected for the rest of
> > run time if unresolved relocations remain.
> 
> Yes, but in practice this is only for broken xorg modules and the
> unresolved relocations are resolved by the time any attack-surface
> code runs, no? Still I agree it's better to avoid this.

Yeah, you never know what other software may depend on this in the future.

> That puts the additional branch/logic inside the hot path used by all
> relocation processing rather than a path that's relegated to just
> outstanding relocations on libraries that didn't declare their
> dependencies properly.
> 
> My version looks something like, inside the for loop in
> redo_lazy_relocs:
> 
> 	need_unprotect = 0;
> 	for (i=0; i<size; i+=3);
> 		if ((uintptr_t)laddr(p, p->lazy[i])-relro_start < relro_end)
> 			need_unprotect = 1;
> 	if (need_unprotect) mprotect(...);
> 	do_relocs(...);
> 	if (need_unprotect) mprotect(...);
> 
> Does that look reasonable?

Thanks, now I see what you had in mind. Sure, this looks nice.

Alexander