From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-oo1-xc36.google.com (mail-oo1-xc36.google.com [IPv6:2607:f8b0:4864:20::c36]) by sourceware.org (Postfix) with ESMTPS id 0735F3858D1E for ; Wed, 29 Mar 2023 17:01:34 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 0735F3858D1E Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=linaro.org Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=linaro.org Received: by mail-oo1-xc36.google.com with SMTP id j12-20020a4ab1cc000000b0053e6062a9eeso992324ooo.8 for ; Wed, 29 Mar 2023 10:01:33 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1680109292; h=content-transfer-encoding:in-reply-to:organization:from:references :to:content-language:subject:user-agent:mime-version:date:message-id :from:to:cc:subject:date:message-id:reply-to; bh=r7flm2QDZQyS3+DMG53TGKHYVFy5qxTqxBXYwn6oTIM=; b=uJtCZD1RAabRk+YcebgQ7SrtapOj0uw94f0gjjKZsnEpy7XhMibLHbphFg1d2RqKBU X9q6Ex4BJ016xQHEMBRxlN7Btfy/SzVp21qTP/dXnaU+8al10JRvz25H3FY1uYIYcOF1 3OMmD2YatZQYuuTffrwxndY/SNurExBYJroyGh2ymfy7WC6w45k5bgB+IMJ3G8zAvKy8 7Q+HeWoET01g7otaHgwzIjrap7lNZB7K3Y+gKb50j1YV7ZwhKNi5IVkUcFjgLuYX9y/c I89ZP8Zjf4KNAY0h820UC8vGb/V3DaXJ+s8YIt6x2v41K16VBwTPww/dWUpTbP17BZp9 selA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1680109292; h=content-transfer-encoding:in-reply-to:organization:from:references :to:content-language:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=r7flm2QDZQyS3+DMG53TGKHYVFy5qxTqxBXYwn6oTIM=; b=nMQAjcMamotEI2IJaFj0nzKLDqn16hAoXB2MgsOIqZEg7rQ0bmHf7YYqCbQ90Feu6R oL+dTbpar1IoV9KDcATYkeAjTZUPM4pvRlURgU0ADmHhgw763SCFDr7CbFD0exfykiW3 YJvbqKnHWwm5JXwuJgHilebH4VYX5GeL6yjAcQfYtI+sz9cT9/45mSYQ9PH28nRzN9Vq 83oZji4lNzXpZnRz+08HtUql7GgEOeBpk7mOzBURaX/qvvq163Otp6PztXvDJWhGS3RZ gv7E0staoKF0ahIyiuVMU5hhoRoMO1yGcBrv9PxU3SoRWG+sVPLZ1U0RPNqgkogRO14+ 5Lwg== X-Gm-Message-State: AAQBX9dxv1CpZBLUbsKEIMmK3A+ukF0Fy37PBEsNThYlSjL0QoNgksxD BMI0UxVkm9YG+ffxbY48D2WcaJALlHtr5qFXsfh7Eg== X-Google-Smtp-Source: AKy350ZPpz5eZQjinkhKmAFvqmbzqYHtdqGOGoZNCvf47wht3vmXHXOKkP++g9VC9i1NATjOd1sXcA== X-Received: by 2002:a4a:45cb:0:b0:53e:5698:c0d2 with SMTP id y194-20020a4a45cb000000b0053e5698c0d2mr4055741ooa.7.1680109291559; Wed, 29 Mar 2023 10:01:31 -0700 (PDT) Received: from ?IPV6:2804:1b3:a7c1:60f9:1426:1d2d:d6b:1761? ([2804:1b3:a7c1:60f9:1426:1d2d:d6b:1761]) by smtp.gmail.com with ESMTPSA id s14-20020a9d758e000000b006a154373578sm2348853otk.39.2023.03.29.10.01.29 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 29 Mar 2023 10:01:30 -0700 (PDT) Message-ID: Date: Wed, 29 Mar 2023 14:01:28 -0300 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:102.0) Gecko/20100101 Thunderbird/102.9.0 Subject: Re: [PATCH 02/13] elf: switch _dl_map_segment() to anonymous mapping Content-Language: en-US To: libc-alpha@sourceware.org, Stas Sergeev References: <20230318165110.3672749-1-stsp2@yandex.ru> <20230318165110.3672749-3-stsp2@yandex.ru> From: Adhemerval Zanella Netto Organization: Linaro In-Reply-To: <20230318165110.3672749-3-stsp2@yandex.ru> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-11.3 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,NICE_REPLY_A,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,TXREP,URIBL_BLACK autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On 18/03/23 13:50, Stas Sergeev via Libc-alpha wrote: > _dl_map_segment() was mapping entire file image and then was skipping > the load of the first segment. Switch _dl_map_segment() to anonymous > mapping and do not skip the map of the first segment. > > Use PROT_READ|PROT_WRITE as a protection. _dl_map_segments() later > sets the proper protection for both file-mapped and anonymous parts. > > The test-suite was run on x86_64/64 and showed no regressions. > > Signed-off-by: Stas Sergeev > --- > elf/dl-map-segments.h | 73 +++++++++++++++++++++++-------------------- > 1 file changed, 39 insertions(+), 34 deletions(-) > > diff --git a/elf/dl-map-segments.h b/elf/dl-map-segments.h > index 504cfc0a41..9af8cae188 100644 > --- a/elf/dl-map-segments.h > +++ b/elf/dl-map-segments.h > @@ -22,18 +22,26 @@ > /* Map a segment and align it properly. */ > > static __always_inline ElfW(Addr) > -_dl_map_segment (const struct loadcmd *c, ElfW(Addr) mappref, > - const size_t maplength, int fd) > +_dl_map_segment (ElfW(Addr) mappref, size_t maplength, size_t mapalign) > { > - if (__glibc_likely (c->mapalign <= GLRO(dl_pagesize))) > - return (ElfW(Addr)) __mmap ((void *) mappref, maplength, c->prot, > - MAP_COPY|MAP_FILE, fd, c->mapoff); > + int err; > + unsigned map_flags = MAP_ANONYMOUS | MAP_PRIVATE; > + unsigned prot = PROT_READ | PROT_WRITE; glibc code guidelines [1] suggest to explicit define the types, so 'unsigned int' here. [1] https://sourceware.org/glibc/wiki/Style_and_Conventions > + > +#ifdef MAP_DENYWRITE > + /* Tell mmap() that we are mapping solib. This flag enables things > + like LD_PREFER_MAP_32BIT_EXEC. */ > + map_flags |= MAP_DENYWRITE; > +#endif Why do you need o add MAP_DENYWRITE? They are complete ignored by Linux, as stated in: include/linux/mman.h 31 /* 32 * The historical set of flags that all mmap implementations implicitly 33 * support when a ->mmap_validate() op is not provided in file_operations. 34 * 35 * MAP_EXECUTABLE and MAP_DENYWRITE are completely ignored throughout the 36 * kernel. 37 */ 38 #define LEGACY_MAP_MASK (MAP_SHARED \ (if you grep in Linux source code you will see there are only defined for historical/compatibility reasons, there is indeed no code that actually uses it). > + if (__glibc_likely (mapalign <= GLRO(dl_pagesize))) > + return (ElfW(Addr)) __mmap ((void *) mappref, maplength, prot, > + map_flags, -1, 0); > > /* If the segment alignment > the page size, alocate enough space to > ensure that the segment can be properly aligned. */ > - ElfW(Addr) maplen = (maplength >= c->mapalign > - ? (maplength + c->mapalign) > - : (2 * c->mapalign)); > + ElfW(Addr) maplen = (maplength >= mapalign > + ? (maplength + mapalign) > + : (2 * mapalign)); > ElfW(Addr) map_start = (ElfW(Addr)) __mmap ((void *) mappref, maplen, > PROT_NONE, > MAP_ANONYMOUS|MAP_PRIVATE, > @@ -41,26 +49,24 @@ _dl_map_segment (const struct loadcmd *c, ElfW(Addr) mappref, > if (__glibc_unlikely ((void *) map_start == MAP_FAILED)) > return map_start; > > - ElfW(Addr) map_start_aligned = ALIGN_UP (map_start, c->mapalign); > - map_start_aligned = (ElfW(Addr)) __mmap ((void *) map_start_aligned, > - maplength, c->prot, > - MAP_COPY|MAP_FILE|MAP_FIXED, > - fd, c->mapoff); > - if (__glibc_unlikely ((void *) map_start_aligned == MAP_FAILED)) > - __munmap ((void *) map_start, maplen); > - else > + ElfW(Addr) map_start_aligned = ALIGN_UP (map_start, mapalign); > + err = __mprotect ((void *) map_start_aligned, maplength, prot); > + if (__glibc_unlikely (err)) > { > - /* Unmap the unused regions. */ > - ElfW(Addr) delta = map_start_aligned - map_start; > - if (delta) > - __munmap ((void *) map_start, delta); > - ElfW(Addr) map_end = map_start_aligned + maplength; > - map_end = ALIGN_UP (map_end, GLRO(dl_pagesize)); > - delta = map_start + maplen - map_end; > - if (delta) > - __munmap ((void *) map_end, delta); > + __munmap ((void *) map_start, maplen); > + return (ElfW(Addr)) MAP_FAILED; > } > > + /* Unmap the unused regions. */ > + ElfW(Addr) delta = map_start_aligned - map_start; > + if (delta) > + __munmap ((void *) map_start, delta); > + ElfW(Addr) map_end = map_start_aligned + maplength; > + map_end = ALIGN_UP (map_end, GLRO(dl_pagesize)); > + delta = map_start + maplen - map_end; > + if (delta) > + __munmap ((void *) map_end, delta); > + > return map_start_aligned; > } > So basically it would add another mmap on program loading. For instance, loading a simple empty main programs: * Before: openat(AT_FDCWD, "./main", O_RDONLY|O_CLOEXEC) = 3 mmap(NULL, 16408, PROT_READ, MAP_PRIVATE|MAP_DENYWRITE, 3, 0) = 0x7f9d922a9000 mmap(0x7f9d922aa000, 4096, PROT_READ|PROT_EXEC, MAP_PRIVATE|MAP_FIXED|MAP_DENYWRITE, 3, 0x1000) = 0x7f9d922aa000 mmap(0x7f9d922ab000, 4096, PROT_READ, MAP_PRIVATE|MAP_FIXED|MAP_DENYWRITE, 3, 0x2000) = 0x7f9d922ab000 mmap(0x7f9d922ac000, 8192, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_FIXED|MAP_DENYWRITE, 3, 0x2000) = 0x7f9d922ac000 [...] openat(AT_FDCWD, "/home/azanella/Projects/glibc/build/x86_64-linux-gnu/libc.so.6", O_RDONLY|O_CLOEXEC) = 3 mmap(NULL, 2080624, PROT_READ, MAP_PRIVATE|MAP_DENYWRITE, 3, 0) = 0x7f9d920ab000 mprotect(0x7f9d920d1000, 1847296, PROT_NONE) = 0 mmap(0x7f9d920d1000, 1490944, PROT_READ|PROT_EXEC, MAP_PRIVATE|MAP_FIXED|MAP_DENYWRITE, 3, 0x26000) = 0x7f9d920d1000 mmap(0x7f9d9223d000, 352256, PROT_READ, MAP_PRIVATE|MAP_FIXED|MAP_DENYWRITE, 3, 0x192000) = 0x7f9d9223d000 mmap(0x7f9d92294000, 24576, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_FIXED|MAP_DENYWRITE, 3, 0x1e8000) = 0x7f9d92294000 mmap(0x7f9d9229a000, 53104, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_FIXED|MAP_ANONYMOUS, -1, 0) = 0x7f9d9229a000 * With this patch: openat(AT_FDCWD, "./main", O_RDONLY|O_CLOEXEC) = 3 mmap(NULL, 16408, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS|MAP_DENYWRITE, -1, 0) = 0x7f705da76000 mmap(0x7f705da76000, 4096, PROT_READ, MAP_PRIVATE|MAP_FIXED|MAP_DENYWRITE, 3, 0) = 0x7f705da76000 mmap(0x7f705da77000, 4096, PROT_READ|PROT_EXEC, MAP_PRIVATE|MAP_FIXED|MAP_DENYWRITE, 3, 0x1000) = 0x7f705da77000 mmap(0x7f705da78000, 4096, PROT_READ, MAP_PRIVATE|MAP_FIXED|MAP_DENYWRITE, 3, 0x2000) = 0x7f705da78000 mmap(0x7f705da79000, 8192, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_FIXED|MAP_DENYWRITE, 3, 0x2000) = 0x7f705da79000 [...] openat(AT_FDCWD, "/home/azanella/Projects/glibc/build/x86_64-linux-gnu/libc.so.6", O_RDONLY|O_CLOEXEC) = 3 mmap(NULL, 2080624, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS|MAP_DENYWRITE, -1, 0) = 0x7f705d878000 mprotect(0x7f705d89e000, 1847296, PROT_NONE) = 0 mmap(0x7f705d878000, 155648, PROT_READ, MAP_PRIVATE|MAP_FIXED|MAP_DENYWRITE, 3, 0) = 0x7f705d878000 mmap(0x7f705d89e000, 1490944, PROT_READ|PROT_EXEC, MAP_PRIVATE|MAP_FIXED|MAP_DENYWRITE, 3, 0x26000) = 0x7f705d89e000 mmap(0x7f705da0a000, 352256, PROT_READ, MAP_PRIVATE|MAP_FIXED|MAP_DENYWRITE, 3, 0x192000) = 0x7f705da0a000 mmap(0x7f705da61000, 24576, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_FIXED|MAP_DENYWRITE, 3, 0x1e8000) = 0x7f705da61000 mmap(0x7f705da67000, 53104, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_FIXED|MAP_ANONYMOUS, -1, 0) = 0x7f705da67000 And it also slight change the mapping, using the same program: * Before: 0x7ffff7dc2000 0x7ffff7de8000 0x26000 0x0 r--p /home/azanella/Projects/glibc/build/x86_64-linux-gnu/libc.so 0x7ffff7de8000 0x7ffff7f54000 0x16c000 0x26000 r-xp /home/azanella/Projects/glibc/build/x86_64-linux-gnu/libc.so 0x7ffff7f54000 0x7ffff7faa000 0x56000 0x192000 r--p /home/azanella/Projects/glibc/build/x86_64-linux-gnu/libc.so 0x7ffff7faa000 0x7ffff7fab000 0x1000 0x1e8000 ---p /home/azanella/Projects/glibc/build/x86_64-linux-gnu/libc.so 0x7ffff7fab000 0x7ffff7faf000 0x4000 0x1e8000 r--p /home/azanella/Projects/glibc/build/x86_64-linux-gnu/libc.so 0x7ffff7faf000 0x7ffff7fb1000 0x2000 0x1ec000 rw-p /home/azanella/Projects/glibc/build/x86_64-linux-gnu/libc.so * With this patch: 0x7ffff7dc1000 0x7ffff7de7000 0x26000 0x0 r--p /home/azanella/Projects/glibc/build/x86_64-linux-gnu/libc.so 0x7ffff7de7000 0x7ffff7f53000 0x16c000 0x26000 r-xp /home/azanella/Projects/glibc/build/x86_64-linux-gnu/libc.so 0x7ffff7f53000 0x7ffff7fa9000 0x56000 0x192000 r--p /home/azanella/Projects/glibc/build/x86_64-linux-gnu/libc.so 0x7ffff7fa9000 0x7ffff7faa000 0x1000 0x0 ---p 0x7ffff7faa000 0x7ffff7fae000 0x4000 0x1e8000 r--p /home/azanella/Projects/glibc/build/x86_64-linux-gnu/libc.so 0x7ffff7fae000 0x7ffff7fb0000 0x2000 0x1ec000 rw-p /home/azanella/Projects/glibc/build/x86_64-linux-gnu/libc.so So I am not seeing any advantage of this refactor: it slight increase the number of syscalls for library loading and changes the 'debuggability' of resulting shared library maps. > @@ -98,7 +104,7 @@ _dl_map_segments (struct link_map *l, int fd, > - MAP_BASE_ADDR (l)); > > /* Remember which part of the address space this object uses. */ > - l->l_map_start = _dl_map_segment (c, mappref, maplength, fd); > + l->l_map_start = _dl_map_segment (mappref, maplength, c->mapalign); > if (__glibc_unlikely ((void *) l->l_map_start == MAP_FAILED)) > return DL_MAP_SEGMENTS_ERROR_MAP_SEGMENT; > > @@ -123,14 +129,14 @@ _dl_map_segments (struct link_map *l, int fd, > } > > l->l_contiguous = 1; > - > - goto postmap; > } > - > - /* Remember which part of the address space this object uses. */ > - l->l_map_start = c->mapstart + l->l_addr; > - l->l_map_end = l->l_map_start + maplength; > - l->l_contiguous = !has_holes; > + else > + { > + /* Remember which part of the address space this object uses. */ > + l->l_map_start = c->mapstart + l->l_addr; > + l->l_map_end = l->l_map_start + maplength; > + l->l_contiguous = !has_holes; > + } > > while (c < &loadcmds[nloadcmds]) > { > @@ -143,7 +149,6 @@ _dl_map_segments (struct link_map *l, int fd, > == MAP_FAILED)) > return DL_MAP_SEGMENTS_ERROR_MAP_SEGMENT; > > - postmap: > _dl_postprocess_loadcmd (l, header, c); > > if (c->allocend > c->dataend)