* [musl] Invalid page size reference in __dls2 @ 2022-11-28 19:47 Markus Wichmann 2022-11-30 15:12 ` Rich Felker 0 siblings, 1 reply; 5+ messages in thread From: Markus Wichmann @ 2022-11-28 19:47 UTC (permalink / raw) To: musl Hi all, __dls2 calls kernel_mapped_dso(), and that one uses the PAGE_SIZE macro. Whenever <bits/limits.h> does not define PAGESIZE, PAGE_SIZE is defined as libc.page_size. That variable is only initialized at the start of __dls3, so the DSO descriptor for libc ends up being wrong. Since the libc object has static storage duration, page_size is initialized with zero. So at least nothing undefined happens. The impact is, it will calculate the relro pointers as being zero, so no relro will happen, and it will calculate maximum and minimum addresses as being zero, therefore setting map to base and map_len to zero. This will cause dladdr() not to find the libc. Yeah, not the biggest of impacts. This, again, affects all architectures that don't define PAGESIZE, so at this time those are aarch64 arm m68k microblaze mips mips64 mipsn32 powerpc powerpc64 riscv64 I don't know whether references to libc are even valid in __dls2, but it is defined as "hidden", so that ought to be good enough. In that case it may be enough to just move the initialization. Otherwise it may be necessary to add page size as parameter to kernel_mapped_dso(). Then __dls2 can look it up in the aux vector at its leisure. Ciao, Markus ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [musl] Invalid page size reference in __dls2 2022-11-28 19:47 [musl] Invalid page size reference in __dls2 Markus Wichmann @ 2022-11-30 15:12 ` Rich Felker 2022-11-30 23:28 ` A. Wilcox 0 siblings, 1 reply; 5+ messages in thread From: Rich Felker @ 2022-11-30 15:12 UTC (permalink / raw) To: Markus Wichmann; +Cc: musl [-- Attachment #1: Type: text/plain, Size: 1577 bytes --] On Mon, Nov 28, 2022 at 08:47:40PM +0100, Markus Wichmann wrote: > Hi all, > > __dls2 calls kernel_mapped_dso(), and that one uses the PAGE_SIZE macro. > Whenever <bits/limits.h> does not define PAGESIZE, PAGE_SIZE is defined > as libc.page_size. That variable is only initialized at the start of > __dls3, so the DSO descriptor for libc ends up being wrong. > > Since the libc object has static storage duration, page_size is > initialized with zero. So at least nothing undefined happens. The impact > is, it will calculate the relro pointers as being zero, so no relro will > happen, and it will calculate maximum and minimum addresses as being > zero, therefore setting map to base and map_len to zero. This will cause > dladdr() not to find the libc. Yeah, not the biggest of impacts. > > This, again, affects all architectures that don't define PAGESIZE, so at > this time those are > > aarch64 > arm > m68k > microblaze > mips > mips64 > mipsn32 > powerpc > powerpc64 > riscv64 > > I don't know whether references to libc are even valid in __dls2, but it > is defined as "hidden", so that ought to be good enough. In that case it > may be enough to just move the initialization. Otherwise it may be > necessary to add page size as parameter to kernel_mapped_dso(). Then > __dls2 can look it up in the aux vector at its leisure. Nice catch. The references to libc are not valid in __dls2. If they were, I would just re-run kernel_mapped_dso() from __dls2b or something to get the right relro map, but I think instead we should do something like the attached. Rich [-- Attachment #2: ldso_page_size.diff --] [-- Type: text/plain, Size: 752 bytes --] diff --git a/ldso/dynlink.c b/ldso/dynlink.c index 8068fb37..fb13a7b1 100644 --- a/ldso/dynlink.c +++ b/ldso/dynlink.c @@ -21,9 +21,15 @@ #include <sys/membarrier.h> #include "pthread_impl.h" #include "fork_impl.h" -#include "libc.h" #include "dynlink.h" +static size_t ldso_page_size; +#ifndef PAGE_SIZE +define PAGE_SIZE ldso_page_size; +#endif + +#include "libc.h" + #define malloc __libc_malloc #define calloc __libc_calloc #define realloc __libc_realloc @@ -1723,6 +1729,7 @@ hidden void __dls2(unsigned char *base, size_t *sp) ldso.phnum = ehdr->e_phnum; ldso.phdr = laddr(&ldso, ehdr->e_phoff); ldso.phentsize = ehdr->e_phentsize; + search_vec(auxv, &ldso_page_size, AT_PAGESZ); kernel_mapped_dso(&ldso); decode_dyn(&ldso); ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [musl] Invalid page size reference in __dls2 2022-11-30 15:12 ` Rich Felker @ 2022-11-30 23:28 ` A. Wilcox 2022-12-01 0:09 ` Rich Felker 0 siblings, 1 reply; 5+ messages in thread From: A. Wilcox @ 2022-11-30 23:28 UTC (permalink / raw) To: musl [-- Attachment #1: Type: text/plain, Size: 480 bytes --] On Nov 30, 2022, at 9:12 AM, Rich Felker <dalias@libc.org> wrote: > > Nice catch. The references to libc are not valid in __dls2. If they > were, I would just re-run kernel_mapped_dso() from __dls2b or > something to get the right relro map, but I think instead we should do > something like the attached. > > Rich > <ldso_page_size.diff> LGTM but needs a # before the ‘define’. This explains a weird error I was seeing with gcompat on aarch64. Best, -A. [-- Attachment #2: Type: text/html, Size: 5248 bytes --] ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [musl] Invalid page size reference in __dls2 2022-11-30 23:28 ` A. Wilcox @ 2022-12-01 0:09 ` Rich Felker 2022-12-01 0:19 ` A. Wilcox 0 siblings, 1 reply; 5+ messages in thread From: Rich Felker @ 2022-12-01 0:09 UTC (permalink / raw) To: A. Wilcox; +Cc: musl [-- Attachment #1: Type: text/plain, Size: 585 bytes --] On Wed, Nov 30, 2022 at 05:28:38PM -0600, A. Wilcox wrote: > > On Nov 30, 2022, at 9:12 AM, Rich Felker <dalias@libc.org> wrote: > > > > Nice catch. The references to libc are not valid in __dls2. If they > > were, I would just re-run kernel_mapped_dso() from __dls2b or > > something to get the right relro map, but I think instead we should do > > something like the attached. > > > > Rich > > <ldso_page_size.diff> > > LGTM but needs a # before the ‘define’. > > This explains a weird error I was seeing with gcompat on aarch64. Does this look better (message included)? [-- Attachment #2: 0001-ldso-fix-invalid-early-references-to-extern-linkage-.patch --] [-- Type: text/plain, Size: 2218 bytes --] From f47a8cdd250d9163fcfb39bf4e9d813957c0b187 Mon Sep 17 00:00:00 2001 From: Rich Felker <dalias@aerifal.cx> Date: Wed, 30 Nov 2022 18:59:08 -0500 Subject: [PATCH] ldso: fix invalid early references to extern-linkage libc.page_size when PAGE_SIZE is not constant, internal/libc.h defines it to expand to libc.page_size. however, kernel_mapped_dso, reachable from stage 2 of the dynamic linker bootstrap (__dls2), needs PAGE_SIZE to interpret the relro range. at this point the libc object is both uninitialized and invalid to access according to our model for bootstrapping, which does not assume any external-linkage objects are accessible until stages 2b/3. in practice it likely worked because hidden visibility tends to behave like internal linkage, but this is not a property that the dynamic linker was designed to rely upon. this bug likely manifested as relro malfunction on archs with variable page size, due to incorrect mask when aligning the relro bounds to page boundaries. while there are certainly more direct ways to fix the known problem point here, a maximally future-proof way is to just bypass the libc.h PAGE_SIZE definition in the dynamic linker and instead have dynlink.c define its own internal-linkage object for variable page size. then, if anything else in stage 2 ever ends up referencing PAGE_SIZE, it will just automatically work right. --- ldso/dynlink.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/ldso/dynlink.c b/ldso/dynlink.c index 8068fb37..09f3b0a8 100644 --- a/ldso/dynlink.c +++ b/ldso/dynlink.c @@ -21,9 +21,15 @@ #include <sys/membarrier.h> #include "pthread_impl.h" #include "fork_impl.h" -#include "libc.h" #include "dynlink.h" +static size_t ldso_page_size; +#ifndef PAGE_SIZE +#define PAGE_SIZE ldso_page_size +#endif + +#include "libc.h" + #define malloc __libc_malloc #define calloc __libc_calloc #define realloc __libc_realloc @@ -1723,6 +1729,7 @@ hidden void __dls2(unsigned char *base, size_t *sp) ldso.phnum = ehdr->e_phnum; ldso.phdr = laddr(&ldso, ehdr->e_phoff); ldso.phentsize = ehdr->e_phentsize; + search_vec(auxv, &ldso_page_size, AT_PAGESZ); kernel_mapped_dso(&ldso); decode_dyn(&ldso); -- 2.21.0 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [musl] Invalid page size reference in __dls2 2022-12-01 0:09 ` Rich Felker @ 2022-12-01 0:19 ` A. Wilcox 0 siblings, 0 replies; 5+ messages in thread From: A. Wilcox @ 2022-12-01 0:19 UTC (permalink / raw) To: Rich Felker; +Cc: musl [-- Attachment #1: Type: text/plain, Size: 199 bytes --] On Nov 30, 2022, at 6:09 PM, Rich Felker <dalias@libc.org> wrote: > > Does this look better (message included)? > > <0001-ldso-fix-invalid-early-references-to-extern-linkage-.patch> LGTM, +1. -A. [-- Attachment #2: Type: text/html, Size: 1741 bytes --] ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-12-01 0:19 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-11-28 19:47 [musl] Invalid page size reference in __dls2 Markus Wichmann 2022-11-30 15:12 ` Rich Felker 2022-11-30 23:28 ` A. Wilcox 2022-12-01 0:09 ` Rich Felker 2022-12-01 0:19 ` A. Wilcox
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).