From: Kees Cook <keescook@chromium.org> To: matoro <matoro_mailinglist_kernel@matoro.tk> Cc: "Kees Cook" <keescook@chromium.org>, "Alexander Viro" <viro@zeniv.linux.org.uk>, "Eric Biederman" <ebiederm@xmission.com>, linux-fsdevel@vger.kernel.org, linux-mm@kvack.org, "John Paul Adrian Glaubitz" <glaubitz@physik.fu-berlin.de>, stable@vger.kernel.org, "Magnus Groß" <magnus.gross@rwth-aachen.de>, "Thorsten Leemhuis" <regressions@leemhuis.info>, "Anthony Yznaga" <anthony.yznaga@oracle.com>, "Andrew Morton" <akpm@linux-foundation.org>, regressions@lists.linux.dev, linux-ia64@vger.kernel.org, linux-kernel@vger.kernel.org, linux-hardening@vger.kernel.org Subject: [PATCH 5.16 v2] binfmt_elf: Avoid total_mapping_size for ET_EXEC Date: Mon, 28 Feb 2022 12:55:18 -0800 [thread overview] Message-ID: <20220228205518.1265798-1-keescook@chromium.org> (raw) Partially revert commit 5f501d555653 ("binfmt_elf: reintroduce using MAP_FIXED_NOREPLACE"). At least ia64 has ET_EXEC PT_LOAD segments that are not virtual-address contiguous (but _are_ file-offset contiguous). This would result in giant mapping attempts to cover the entire span, including the virtual address range hole. Disable total_mapping_size for ET_EXEC, which reduces the MAP_FIXED_NOREPLACE coverage to only the first PT_LOAD: $ readelf -lW /usr/bin/gcc ... Program Headers: Type Offset VirtAddr PhysAddr FileSiz MemSiz ... ... LOAD 0x000000 0x4000000000000000 0x4000000000000000 0x00b5a0 0x00b5a0 ... LOAD 0x00b5a0 0x600000000000b5a0 0x600000000000b5a0 0x0005ac 0x000710 ... ... ^^^^^^^^ ^^^^^^^^^^^^^^^^^^ ^^^^^^^^ ^^^^^^^^ File offset range : 0x000000-0x00bb4c 0x00bb4c bytes Virtual address range : 0x4000000000000000-0x600000000000bcb0 0x200000000000bcb0 bytes Ironically, this is the reverse of the problem that originally caused problems with ET_EXEC and MAP_FIXED_NOREPLACE: overlaps. This problem is with holes. Future work could restore full coverage if load_elf_binary() were to perform mappings in a separate phase from the loading (where it could resolve both overlaps and holes). Cc: Alexander Viro <viro@zeniv.linux.org.uk> Cc: Eric Biederman <ebiederm@xmission.com> Cc: linux-fsdevel@vger.kernel.org Cc: linux-mm@kvack.org Reported-by: matoro <matoro_mailinglist_kernel@matoro.tk> Reported-by: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de> Fixes: 5f501d555653 ("binfmt_elf: reintroduce using MAP_FIXED_NOREPLACE") Link: https://lore.kernel.org/r/a3edd529-c42d-3b09-135c-7e98a15b150f@leemhuis.info Cc: stable@vger.kernel.org Signed-off-by: Kees Cook <keescook@chromium.org> --- Here's the v5.16 backport. --- fs/binfmt_elf.c | 25 ++++++++++++++++++------- 1 file changed, 18 insertions(+), 7 deletions(-) diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c index f8c7f26f1fbb..911a9e7044f4 100644 --- a/fs/binfmt_elf.c +++ b/fs/binfmt_elf.c @@ -1135,14 +1135,25 @@ static int load_elf_binary(struct linux_binprm *bprm) * is then page aligned. */ load_bias = ELF_PAGESTART(load_bias - vaddr); - } - /* - * Calculate the entire size of the ELF mapping (total_size). - * (Note that load_addr_set is set to true later once the - * initial mapping is performed.) - */ - if (!load_addr_set) { + /* + * Calculate the entire size of the ELF mapping + * (total_size), used for the initial mapping, + * due to first_pt_load which is set to false later + * once the initial mapping is performed. + * + * Note that this is only sensible when the LOAD + * segments are contiguous (or overlapping). If + * used for LOADs that are far apart, this would + * cause the holes between LOADs to be mapped, + * running the risk of having the mapping fail, + * as it would be larger than the ELF file itself. + * + * As a result, only ET_DYN does this, since + * some ET_EXEC (e.g. ia64) may have virtual + * memory holes between LOADs. + * + */ total_size = total_mapping_size(elf_phdata, elf_ex->e_phnum); if (!total_size) { -- 2.32.0
WARNING: multiple messages have this Message-ID (diff)
From: Kees Cook <keescook@chromium.org> To: matoro <matoro_mailinglist_kernel@matoro.tk> Cc: "Kees Cook" <keescook@chromium.org>, "Alexander Viro" <viro@zeniv.linux.org.uk>, "Eric Biederman" <ebiederm@xmission.com>, linux-fsdevel@vger.kernel.org, linux-mm@kvack.org, "John Paul Adrian Glaubitz" <glaubitz@physik.fu-berlin.de>, stable@vger.kernel.org, "Magnus Groß" <magnus.gross@rwth-aachen.de>, "Thorsten Leemhuis" <regressions@leemhuis.info>, "Anthony Yznaga" <anthony.yznaga@oracle.com>, "Andrew Morton" <akpm@linux-foundation.org>, regressions@lists.linux.dev, linux-ia64@vger.kernel.org, linux-kernel@vger.kernel.org, linux-hardening@vger.kernel.org Subject: [PATCH 5.16 v2] binfmt_elf: Avoid total_mapping_size for ET_EXEC Date: Mon, 28 Feb 2022 20:55:18 +0000 [thread overview] Message-ID: <20220228205518.1265798-1-keescook@chromium.org> (raw) Partially revert commit 5f501d555653 ("binfmt_elf: reintroduce using MAP_FIXED_NOREPLACE"). At least ia64 has ET_EXEC PT_LOAD segments that are not virtual-address contiguous (but _are_ file-offset contiguous). This would result in giant mapping attempts to cover the entire span, including the virtual address range hole. Disable total_mapping_size for ET_EXEC, which reduces the MAP_FIXED_NOREPLACE coverage to only the first PT_LOAD: $ readelf -lW /usr/bin/gcc ... Program Headers: Type Offset VirtAddr PhysAddr FileSiz MemSiz ... ... LOAD 0x000000 0x4000000000000000 0x4000000000000000 0x00b5a0 0x00b5a0 ... LOAD 0x00b5a0 0x600000000000b5a0 0x600000000000b5a0 0x0005ac 0x000710 ... ... ^^^^^^^^ ^^^^^^^^^^^^^^^^^^ ^^^^^^^^ ^^^^^^^^ File offset range : 0x000000-0x00bb4c 0x00bb4c bytes Virtual address range : 0x4000000000000000-0x600000000000bcb0 0x200000000000bcb0 bytes Ironically, this is the reverse of the problem that originally caused problems with ET_EXEC and MAP_FIXED_NOREPLACE: overlaps. This problem is with holes. Future work could restore full coverage if load_elf_binary() were to perform mappings in a separate phase from the loading (where it could resolve both overlaps and holes). Cc: Alexander Viro <viro@zeniv.linux.org.uk> Cc: Eric Biederman <ebiederm@xmission.com> Cc: linux-fsdevel@vger.kernel.org Cc: linux-mm@kvack.org Reported-by: matoro <matoro_mailinglist_kernel@matoro.tk> Reported-by: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de> Fixes: 5f501d555653 ("binfmt_elf: reintroduce using MAP_FIXED_NOREPLACE") Link: https://lore.kernel.org/r/a3edd529-c42d-3b09-135c-7e98a15b150f@leemhuis.info Cc: stable@vger.kernel.org Signed-off-by: Kees Cook <keescook@chromium.org> --- Here's the v5.16 backport. --- fs/binfmt_elf.c | 25 ++++++++++++++++++------- 1 file changed, 18 insertions(+), 7 deletions(-) diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c index f8c7f26f1fbb..911a9e7044f4 100644 --- a/fs/binfmt_elf.c +++ b/fs/binfmt_elf.c @@ -1135,14 +1135,25 @@ static int load_elf_binary(struct linux_binprm *bprm) * is then page aligned. */ load_bias = ELF_PAGESTART(load_bias - vaddr); - } - /* - * Calculate the entire size of the ELF mapping (total_size). - * (Note that load_addr_set is set to true later once the - * initial mapping is performed.) - */ - if (!load_addr_set) { + /* + * Calculate the entire size of the ELF mapping + * (total_size), used for the initial mapping, + * due to first_pt_load which is set to false later + * once the initial mapping is performed. + * + * Note that this is only sensible when the LOAD + * segments are contiguous (or overlapping). If + * used for LOADs that are far apart, this would + * cause the holes between LOADs to be mapped, + * running the risk of having the mapping fail, + * as it would be larger than the ELF file itself. + * + * As a result, only ET_DYN does this, since + * some ET_EXEC (e.g. ia64) may have virtual + * memory holes between LOADs. + * + */ total_size = total_mapping_size(elf_phdata, elf_ex->e_phnum); if (!total_size) { -- 2.32.0
next reply other threads:[~2022-02-28 20:55 UTC|newest] Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top 2022-02-28 20:55 Kees Cook [this message] 2022-02-28 20:55 ` [PATCH 5.16 v2] binfmt_elf: Avoid total_mapping_size for ET_EXEC Kees Cook 2022-02-28 22:14 ` matoro 2022-02-28 22:14 ` matoro 2022-02-28 22:53 ` Kees Cook 2022-02-28 22:53 ` Kees Cook 2022-03-01 12:36 ` John Paul Adrian Glaubitz 2022-03-01 12:36 ` John Paul Adrian Glaubitz
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=20220228205518.1265798-1-keescook@chromium.org \ --to=keescook@chromium.org \ --cc=akpm@linux-foundation.org \ --cc=anthony.yznaga@oracle.com \ --cc=ebiederm@xmission.com \ --cc=glaubitz@physik.fu-berlin.de \ --cc=linux-fsdevel@vger.kernel.org \ --cc=linux-hardening@vger.kernel.org \ --cc=linux-ia64@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-mm@kvack.org \ --cc=magnus.gross@rwth-aachen.de \ --cc=matoro_mailinglist_kernel@matoro.tk \ --cc=regressions@leemhuis.info \ --cc=regressions@lists.linux.dev \ --cc=stable@vger.kernel.org \ --cc=viro@zeniv.linux.org.uk \ /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: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.