[musl] [PATCH] ldso: support DT_RELR relative relocation format

[musl] [PATCH] ldso: support DT_RELR relative relocation format

[Fangrui Song]

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;
+		}
+}
+
 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
 
 typedef void (*stage2_func)(unsigned char *, size_t *);
 
-- 
2.36.1

[musl] Re: [PATCH] ldso: support DT_RELR relative relocation format

[Fangrui Song]

On 2022-06-05, 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.

Ping

[musl] Re: [PATCH] ldso: support DT_RELR relative relocation format

[Fangrui Song]

On Wed, Jun 22, 2022 at 7:12 PM Fangrui Song <i@maskray.me> wrote:
>
> On 2022-06-05, 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.
>
> Ping

Ping^2

Re: [musl] [PATCH] ldso: support DT_RELR relative relocation format

[Rich Felker]

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?

>  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

Re: [musl] [PATCH] ldso: support DT_RELR relative relocation format

[Fangrui Song]

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