[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20220802191645.GT7074@brightrain.aerifal.cx>
Date: Tue, 2 Aug 2022 15:16:46 -0400
From: Rich Felker <dalias@...c.org>
To: Fangrui Song <i@...kray.me>
Cc: musl@...ts.openwall.com
Subject: Re: [PATCH] ldso: support DT_RELR relative relocation format
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
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
