From: Fangrui Song <i@maskray.me>
To: musl@lists.openwall.com
Subject: Re: [musl] [PATCH] ldso: support DT_RELR relative relocation format
Date: Tue, 2 Aug 2022 13:17:42 -0700 [thread overview]
Message-ID: <20220802201742.nl77eqxwm3oemuqr@gmail.com> (raw)
In-Reply-To: <20220802191645.GT7074@brightrain.aerifal.cx>
On 2022-08-02, Rich Felker wrote:
>On Sun, Jun 05, 2022 at 11:16:33PM -0700, Fangrui Song wrote:
>> this resolves DT_RELR relocations in non-ldso objects.
>>
>> generic-abi pre-standard for DT_RELR:
>> https://groups.google.com/g/generic-abi/c/bX460iggiKg
>> FreeBSD rtld added DT_RELR support in 2021-08.
>> glibc added DT_RELR support in 2022-04.
>>
>> Since ld.lld 7, --pack-dyn-relocs=relr can generate DT_RELR.
>> Since binutils 2.38, GNU ld's x86 and powerpc64 ports supports -z
>> pack-relative-relocs to generate DT_RELR. ld.lld 15 also has the option.
>>
>> ---
>> Changes from https://www.openwall.com/lists/musl/2019/03/06/3
>> * rename some variables
>> * decode_vec: add `if (v[0] < 8*sizeof(long))`
>> ---
>> include/elf.h | 8 ++++++--
>> ldso/dynlink.c | 20 +++++++++++++++++++-
>> src/internal/dynlink.h | 2 +-
>> 3 files changed, 26 insertions(+), 4 deletions(-)
>>
>> diff --git a/include/elf.h b/include/elf.h
>> index 86e2f0bb..9e980a29 100644
>> --- a/include/elf.h
>> +++ b/include/elf.h
>> @@ -385,7 +385,8 @@ typedef struct {
>> #define SHT_PREINIT_ARRAY 16
>> #define SHT_GROUP 17
>> #define SHT_SYMTAB_SHNDX 18
>> -#define SHT_NUM 19
>> +#define SHT_RELR 19
>> +#define SHT_NUM 20
>> #define SHT_LOOS 0x60000000
>> #define SHT_GNU_ATTRIBUTES 0x6ffffff5
>> #define SHT_GNU_HASH 0x6ffffff6
>> @@ -754,7 +755,10 @@ typedef struct {
>> #define DT_PREINIT_ARRAY 32
>> #define DT_PREINIT_ARRAYSZ 33
>> #define DT_SYMTAB_SHNDX 34
>> -#define DT_NUM 35
>> +#define DT_RELRSZ 35
>> +#define DT_RELR 36
>> +#define DT_RELRENT 37
>> +#define DT_NUM 38
>> #define DT_LOOS 0x6000000d
>> #define DT_HIOS 0x6ffff000
>> #define DT_LOPROC 0x70000000
>> diff --git a/ldso/dynlink.c b/ldso/dynlink.c
>> index 5b9c8be4..a50ef00a 100644
>> --- a/ldso/dynlink.c
>> +++ b/ldso/dynlink.c
>> @@ -208,7 +208,8 @@ static void decode_vec(size_t *v, size_t *a, size_t cnt)
>> size_t i;
>> for (i=0; i<cnt; i++) a[i] = 0;
>> for (; v[0]; v+=2) if (v[0]-1<cnt-1) {
>> - a[0] |= 1UL<<v[0];
>> + if (v[0] < 8*sizeof(long))
>> + a[0] |= 1UL<<v[0];
>> a[v[0]] = v[1];
>> }
>> }
>> @@ -513,6 +514,22 @@ static void do_relocs(struct dso *dso, size_t *rel, size_t rel_size, size_t stri
>> }
>> }
>>
>> +static void do_relr_relocs(struct dso *dso, size_t *relr, size_t relr_size) {
>> + unsigned char *base = dso->base;
>> + size_t *reloc_addr;
>> + for (; relr_size; relr++, relr_size-=sizeof(size_t))
>> + if ((relr[0]&1) == 0) {
>> + reloc_addr = laddr(dso, relr[0]);
>> + *reloc_addr++ += (size_t)base;
>> + } else {
>> + int i = 0;
>> + for (size_t bitmap=relr[0]; (bitmap>>=1); i++)
>> + if (bitmap&1)
>> + reloc_addr[i] += (size_t)base;
>> + reloc_addr += 8*sizeof(size_t)-1;
>> + }
>> +}
>> +
>
>As written, this code is assuming !FDPIC, so should probably be
>conditional on that. However I think it can be written not to make
>this assumption, as:
>
> *reloc_addr = laddr(dso, *reloc_addr);
> reloc_addr++;
>
>instead of:
>
> *reloc_addr++ += base;
>
>and likewise in the second instance. The laddr() macro exists
>specifically to abstract translating "addresss in space of ELF image"
>to "address as mapped in memory" which I think is the intended
>semantic here. Is that correct?
I have to confess I don't know much about FDPIC (I merely read a little
bit about "ARM FDPIC ABI" a while ago.)
DT_RELR operations should resemble REL_RELATIVE for REL/RELA.
The REL_RELATIVE code does
*reloc_addr = (size_t)base + addend;
so I think `*reloc_addr++ += (size_t)base;` is fine.
>> static void redo_lazy_relocs()
>> {
>> struct dso *p = lazy_head, *next;
>> @@ -1355,6 +1372,7 @@ static void reloc_all(struct dso *p)
>> 2+(dyn[DT_PLTREL]==DT_RELA));
>> do_relocs(p, laddr(p, dyn[DT_REL]), dyn[DT_RELSZ], 2);
>> do_relocs(p, laddr(p, dyn[DT_RELA]), dyn[DT_RELASZ], 3);
>> + do_relr_relocs(p, laddr(p, dyn[DT_RELR]), dyn[DT_RELRSZ]);
>>
>> if (head != &ldso && p->relro_start != p->relro_end &&
>> mprotect(laddr(p, p->relro_start), p->relro_end-p->relro_start, PROT_READ)
>> diff --git a/src/internal/dynlink.h b/src/internal/dynlink.h
>> index 51c0639f..830354eb 100644
>> --- a/src/internal/dynlink.h
>> +++ b/src/internal/dynlink.h
>> @@ -93,7 +93,7 @@ struct fdpic_dummy_loadmap {
>> #endif
>>
>> #define AUX_CNT 32
>> -#define DYN_CNT 32
>> +#define DYN_CNT 37
>
>I was concerned about the decode_vec [0] slot bitmask usage, but you
>addressed that, and I've checked for other instances that might be
>wrong but couldn't find any, so I think it's okay.
>
>Rich
prev parent reply other threads:[~2022-08-02 20:18 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-06-06 6:16 Fangrui Song
2022-06-23 2:12 ` [musl] " Fangrui Song
2022-07-16 8:23 ` Fangrui Song
2022-08-02 19:16 ` [musl] " Rich Felker
2022-08-02 20:17 ` Fangrui Song [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20220802201742.nl77eqxwm3oemuqr@gmail.com \
--to=i@maskray.me \
--cc=musl@lists.openwall.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://git.vuxu.org/mirror/musl/
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).