From: Wenlin Kang wenlin.kang@windriver.com
The selftest tpdir2 terminated with a 'Segmentation fault' during loading.
root@localhost:~# cd linux-kenel/tools/testing/selftests/arm64/abi && make root@localhost:~/linux-kernel/tools/testing/selftests/arm64/abi# ./tpidr2 Segmentation fault
The cause of this is the __arch_clear_user() failure.
load_elf_binary() [fs/binfmt_elf.c] -> if (likely(elf_bss != elf_brk) && unlikely(padzero(elf_bes))) -> padzero() -> clear_user() [arch/arm64/include/asm/uaccess.h] -> __arch_clear_user() [arch/arm64/lib/clear_user.S]
For more details, please see: https://lore.kernel.org/lkml/1d0342f3-0474-482b-b6db-81ca7820a462@t-8ch.de/T...
This issue has been fixed in the mainline. Here I have backported the relevant commits for the linux-6.1.y branch and attached them. With these patches, tpdir2 works as:
root@localhost:~/linux-kernel/tools/testing/selftests/arm64/abi# ./tpidr2 TAP version 13 1..5 ok 0 skipped, TPIDR2 not supported ok 1 skipped, TPIDR2 not supported ok 2 skipped, TPIDR2 not supported ok 3 skipped, TPIDR2 not supported ok 4 skipped, TPIDR2 not supported
The first patch is just for alignment to apply the follow patches. This issue is resolved by the second patch. However, to ensure functional completeness, all related patches were backported according to the following link.
https://lore.kernel.org/all/20230929031716.it.155-kees@kernel.org/#t
Bo Liu (1): binfmt_elf: replace IS_ERR() with IS_ERR_VALUE()
Eric W. Biederman (1): binfmt_elf: Support segments with 0 filesz and misaligned starts
Kees Cook (5): binfmt_elf: elf_bss no longer used by load_elf_binary() binfmt_elf: Use elf_load() for interpreter binfmt_elf: Use elf_load() for library binfmt_elf: Only report padzero() errors when PROT_WRITE mm: Remove unused vm_brk()
fs/binfmt_elf.c | 221 ++++++++++++++++----------------------------- include/linux/mm.h | 3 +- mm/mmap.c | 6 -- mm/nommu.c | 5 - 4 files changed, 79 insertions(+), 156 deletions(-)
From: Bo Liu liubo03@inspur.com
commit dc64cc12bcd14219afb91b55d23192c3eb45aa43 upstream
Avoid typecasts that are needed for IS_ERR() and use IS_ERR_VALUE() instead.
Signed-off-by: Bo Liu liubo03@inspur.com Signed-off-by: Kees Cook keescook@chromium.org Link: https://lore.kernel.org/r/20221115031757.2426-1-liubo03@inspur.com Signed-off-by: Wenlin Kang wenlin.kang@windriver.com --- fs/binfmt_elf.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c index 89e7e4826efc..584b446494cf 100644 --- a/fs/binfmt_elf.c +++ b/fs/binfmt_elf.c @@ -1167,7 +1167,7 @@ static int load_elf_binary(struct linux_binprm *bprm) error = elf_map(bprm->file, load_bias + vaddr, elf_ppnt, elf_prot, elf_flags, total_size); if (BAD_ADDR(error)) { - retval = IS_ERR((void *)error) ? + retval = IS_ERR_VALUE(error) ? PTR_ERR((void*)error) : -EINVAL; goto out_free_dentry; } @@ -1252,7 +1252,7 @@ static int load_elf_binary(struct linux_binprm *bprm) interpreter, load_bias, interp_elf_phdata, &arch_state); - if (!IS_ERR((void *)elf_entry)) { + if (!IS_ERR_VALUE(elf_entry)) { /* * load_elf_interp() returns relocation * adjustment @@ -1261,7 +1261,7 @@ static int load_elf_binary(struct linux_binprm *bprm) elf_entry += interp_elf_ex->e_entry; } if (BAD_ADDR(elf_entry)) { - retval = IS_ERR((void *)elf_entry) ? + retval = IS_ERR_VALUE(elf_entry) ? (int)elf_entry : -EINVAL; goto out_free_dentry; }
[ Sasha's backport helper bot ]
Hi,
✅ All tests passed successfully. No issues detected. No action required from the submitter.
The upstream commit SHA1 provided is correct: dc64cc12bcd14219afb91b55d23192c3eb45aa43
WARNING: Author mismatch between patch and upstream commit: Backport author: Kang Wenlinwenlin.kang@windriver.com Commit author: Bo Liuliubo03@inspur.com
Status in newer kernel trees: 6.13.y | Present (exact SHA1) 6.12.y | Present (exact SHA1) 6.6.y | Present (exact SHA1)
Note: The patch differs from the upstream commit: --- 1: dc64cc12bcd14 ! 1: 35172eeb12462 binfmt_elf: replace IS_ERR() with IS_ERR_VALUE() @@ Metadata ## Commit message ## binfmt_elf: replace IS_ERR() with IS_ERR_VALUE()
+ commit dc64cc12bcd14219afb91b55d23192c3eb45aa43 upstream + Avoid typecasts that are needed for IS_ERR() and use IS_ERR_VALUE() instead.
Signed-off-by: Bo Liu liubo03@inspur.com Signed-off-by: Kees Cook keescook@chromium.org Link: https://lore.kernel.org/r/20221115031757.2426-1-liubo03@inspur.com + Signed-off-by: Wenlin Kang wenlin.kang@windriver.com
## fs/binfmt_elf.c ## @@ fs/binfmt_elf.c: static int load_elf_binary(struct linux_binprm *bprm) ---
Results of testing on various branches:
| Branch | Patch Apply | Build Test | |---------------------------|-------------|------------| | stable/linux-6.1.y | Success | Success |
From: "Eric W. Biederman" ebiederm@xmission.com
commit 585a018627b4d7ed37387211f667916840b5c5ea upstream
Implement a helper elf_load() that wraps elf_map() and performs all of the necessary work to ensure that when "memsz > filesz" the bytes described by "memsz > filesz" are zeroed.
An outstanding issue is if the first segment has filesz 0, and has a randomized location. But that is the same as today.
In this change I replaced an open coded padzero() that did not clear all of the way to the end of the page, with padzero() that does.
I also stopped checking the return of padzero() as there is at least one known case where testing for failure is the wrong thing to do. It looks like binfmt_elf_fdpic may have the proper set of tests for when error handling can be safely completed.
I found a couple of commits in the old history https://git.kernel.org/pub/scm/linux/kernel/git/tglx/history.git, that look very interesting in understanding this code.
commit 39b56d902bf3 ("[PATCH] binfmt_elf: clearing bss may fail") commit c6e2227e4a3e ("[SPARC64]: Missing user access return value checks in fs/binfmt_elf.c and fs/compat.c") commit 5bf3be033f50 ("v2.4.10.1 -> v2.4.10.2")
Looking at commit 39b56d902bf3 ("[PATCH] binfmt_elf: clearing bss may fail"):
commit 39b56d902bf35241e7cba6cc30b828ed937175ad Author: Pavel Machek pavel@ucw.cz Date: Wed Feb 9 22:40:30 2005 -0800
[PATCH] binfmt_elf: clearing bss may fail So we discover that Borland's Kylix application builder emits weird elf files which describe a non-writeable bss segment. So remove the clear_user() check at the place where we zero out the bss. I don't _think_ there are any security implications here (plus we've never checked that clear_user() return value, so whoops if it is a problem). Signed-off-by: Pavel Machek <pavel@suse.cz> Signed-off-by: Andrew Morton <akpm@osdl.org> Signed-off-by: Linus Torvalds <torvalds@osdl.org>
It seems pretty clear that binfmt_elf_fdpic with skipping clear_user() for non-writable segments and otherwise calling clear_user(), aka padzero(), and checking it's return code is the right thing to do.
I just skipped the error checking as that avoids breaking things.
And notably, it looks like Borland's Kylix died in 2005 so it might be safe to just consider read-only segments with memsz > filesz an error.
Reported-by: Sebastian Ott sebott@redhat.com Reported-by: Thomas Weißschuh linux@weissschuh.net Closes: https://lkml.kernel.org/r/20230914-bss-alloc-v1-1-78de67d2c6dd@weissschuh.ne... Signed-off-by: "Eric W. Biederman" ebiederm@xmission.com Link: https://lore.kernel.org/r/87sf71f123.fsf@email.froward.int.ebiederm.org Tested-by: Pedro Falcato pedro.falcato@gmail.com Signed-off-by: Sebastian Ott sebott@redhat.com Link: https://lore.kernel.org/r/20230929032435.2391507-1-keescook@chromium.org Signed-off-by: Kees Cook keescook@chromium.org Signed-off-by: Wenlin Kang wenlin.kang@windriver.com --- fs/binfmt_elf.c | 111 +++++++++++++++++++++--------------------------- 1 file changed, 48 insertions(+), 63 deletions(-)
diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c index 584b446494cf..90151e152a7f 100644 --- a/fs/binfmt_elf.c +++ b/fs/binfmt_elf.c @@ -109,25 +109,6 @@ static struct linux_binfmt elf_format = {
#define BAD_ADDR(x) (unlikely((unsigned long)(x) >= TASK_SIZE))
-static int set_brk(unsigned long start, unsigned long end, int prot) -{ - start = ELF_PAGEALIGN(start); - end = ELF_PAGEALIGN(end); - if (end > start) { - /* - * Map the last of the bss segment. - * If the header is requesting these pages to be - * executable, honour that (ppc32 needs this). - */ - int error = vm_brk_flags(start, end - start, - prot & PROT_EXEC ? VM_EXEC : 0); - if (error) - return error; - } - current->mm->start_brk = current->mm->brk = end; - return 0; -} - /* We need to explicitly zero any fractional pages after the data section (i.e. bss). This would contain the junk from the file that should not @@ -401,6 +382,51 @@ static unsigned long elf_map(struct file *filep, unsigned long addr, return(map_addr); }
+static unsigned long elf_load(struct file *filep, unsigned long addr, + const struct elf_phdr *eppnt, int prot, int type, + unsigned long total_size) +{ + unsigned long zero_start, zero_end; + unsigned long map_addr; + + if (eppnt->p_filesz) { + map_addr = elf_map(filep, addr, eppnt, prot, type, total_size); + if (BAD_ADDR(map_addr)) + return map_addr; + if (eppnt->p_memsz > eppnt->p_filesz) { + zero_start = map_addr + ELF_PAGEOFFSET(eppnt->p_vaddr) + + eppnt->p_filesz; + zero_end = map_addr + ELF_PAGEOFFSET(eppnt->p_vaddr) + + eppnt->p_memsz; + + /* Zero the end of the last mapped page */ + padzero(zero_start); + } + } else { + map_addr = zero_start = ELF_PAGESTART(addr); + zero_end = zero_start + ELF_PAGEOFFSET(eppnt->p_vaddr) + + eppnt->p_memsz; + } + if (eppnt->p_memsz > eppnt->p_filesz) { + /* + * Map the last of the segment. + * If the header is requesting these pages to be + * executable, honour that (ppc32 needs this). + */ + int error; + + zero_start = ELF_PAGEALIGN(zero_start); + zero_end = ELF_PAGEALIGN(zero_end); + + error = vm_brk_flags(zero_start, zero_end - zero_start, + prot & PROT_EXEC ? VM_EXEC : 0); + if (error) + map_addr = error; + } + return map_addr; +} + + static unsigned long total_mapping_size(const struct elf_phdr *phdr, int nr) { elf_addr_t min_addr = -1; @@ -830,7 +856,6 @@ static int load_elf_binary(struct linux_binprm *bprm) struct elf_phdr *elf_ppnt, *elf_phdata, *interp_elf_phdata = NULL; struct elf_phdr *elf_property_phdata = NULL; unsigned long elf_bss, elf_brk; - int bss_prot = 0; int retval, i; unsigned long elf_entry; unsigned long e_entry; @@ -1042,33 +1067,6 @@ static int load_elf_binary(struct linux_binprm *bprm) if (elf_ppnt->p_type != PT_LOAD) continue;
- if (unlikely (elf_brk > elf_bss)) { - unsigned long nbyte; - - /* There was a PT_LOAD segment with p_memsz > p_filesz - before this one. Map anonymous pages, if needed, - and clear the area. */ - retval = set_brk(elf_bss + load_bias, - elf_brk + load_bias, - bss_prot); - if (retval) - goto out_free_dentry; - nbyte = ELF_PAGEOFFSET(elf_bss); - if (nbyte) { - nbyte = ELF_MIN_ALIGN - nbyte; - if (nbyte > elf_brk - elf_bss) - nbyte = elf_brk - elf_bss; - if (clear_user((void __user *)elf_bss + - load_bias, nbyte)) { - /* - * This bss-zeroing can fail if the ELF - * file specifies odd protections. So - * we don't check the return value - */ - } - } - } - elf_prot = make_prot(elf_ppnt->p_flags, &arch_state, !!interpreter, false);
@@ -1164,7 +1162,7 @@ static int load_elf_binary(struct linux_binprm *bprm) } }
- error = elf_map(bprm->file, load_bias + vaddr, elf_ppnt, + error = elf_load(bprm->file, load_bias + vaddr, elf_ppnt, elf_prot, elf_flags, total_size); if (BAD_ADDR(error)) { retval = IS_ERR_VALUE(error) ? @@ -1219,10 +1217,8 @@ static int load_elf_binary(struct linux_binprm *bprm) if (end_data < k) end_data = k; k = elf_ppnt->p_vaddr + elf_ppnt->p_memsz; - if (k > elf_brk) { - bss_prot = elf_prot; + if (k > elf_brk) elf_brk = k; - } }
e_entry = elf_ex->e_entry + load_bias; @@ -1234,18 +1230,7 @@ static int load_elf_binary(struct linux_binprm *bprm) start_data += load_bias; end_data += load_bias;
- /* Calling set_brk effectively mmaps the pages that we need - * for the bss and break sections. We must do this before - * mapping in the interpreter, to make sure it doesn't wind - * up getting placed where the bss needs to go. - */ - retval = set_brk(elf_bss, elf_brk, bss_prot); - if (retval) - goto out_free_dentry; - if (likely(elf_bss != elf_brk) && unlikely(padzero(elf_bss))) { - retval = -EFAULT; /* Nobody gets to see this, but.. */ - goto out_free_dentry; - } + current->mm->start_brk = current->mm->brk = ELF_PAGEALIGN(elf_brk);
if (interpreter) { elf_entry = load_elf_interp(interp_elf_ex,
[ Sasha's backport helper bot ]
Hi,
✅ All tests passed successfully. No issues detected. No action required from the submitter.
The upstream commit SHA1 provided is correct: 585a018627b4d7ed37387211f667916840b5c5ea
WARNING: Author mismatch between patch and upstream commit: Backport author: Kang Wenlinwenlin.kang@windriver.com Commit author: Eric W. Biedermanebiederm@xmission.com
Status in newer kernel trees: 6.13.y | Present (exact SHA1) 6.12.y | Present (exact SHA1) 6.6.y | Not found
Note: The patch differs from the upstream commit: --- 1: 585a018627b4d ! 1: f14beae673802 binfmt_elf: Support segments with 0 filesz and misaligned starts @@ Metadata ## Commit message ## binfmt_elf: Support segments with 0 filesz and misaligned starts
+ commit 585a018627b4d7ed37387211f667916840b5c5ea upstream + Implement a helper elf_load() that wraps elf_map() and performs all of the necessary work to ensure that when "memsz > filesz" the bytes described by "memsz > filesz" are zeroed. @@ Commit message Signed-off-by: Sebastian Ott sebott@redhat.com Link: https://lore.kernel.org/r/20230929032435.2391507-1-keescook@chromium.org Signed-off-by: Kees Cook keescook@chromium.org + Signed-off-by: Wenlin Kang wenlin.kang@windriver.com
## fs/binfmt_elf.c ## @@ fs/binfmt_elf.c: static struct linux_binfmt elf_format = { @@ fs/binfmt_elf.c: static int load_elf_binary(struct linux_binprm *bprm)
- if (unlikely (elf_brk > elf_bss)) { - unsigned long nbyte; -- +- - /* There was a PT_LOAD segment with p_memsz > p_filesz - before this one. Map anonymous pages, if needed, - and clear the area. */ ---
Results of testing on various branches:
| Branch | Patch Apply | Build Test | |---------------------------|-------------|------------| | stable/linux-6.6.y | Success | Success |
From: Kees Cook keescook@chromium.org
commit 8ed2ef21ff564cf4a25c098ace510ee6513c9836 upstream
With the BSS handled generically via the new filesz/memsz mismatch handling logic in elf_load(), elf_bss no longer needs to be tracked. Drop the variable.
Cc: Eric Biederman ebiederm@xmission.com Cc: Alexander Viro viro@zeniv.linux.org.uk Cc: Christian Brauner brauner@kernel.org Cc: linux-fsdevel@vger.kernel.org Cc: linux-mm@kvack.org Suggested-by: Eric Biederman ebiederm@xmission.com Tested-by: Pedro Falcato pedro.falcato@gmail.com Signed-off-by: Sebastian Ott sebott@redhat.com Link: https://lore.kernel.org/r/20230929032435.2391507-2-keescook@chromium.org Signed-off-by: Kees Cook keescook@chromium.org Signed-off-by: Wenlin Kang wenlin.kang@windriver.com --- fs/binfmt_elf.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c index 90151e152a7f..631219c3ace9 100644 --- a/fs/binfmt_elf.c +++ b/fs/binfmt_elf.c @@ -855,7 +855,7 @@ static int load_elf_binary(struct linux_binprm *bprm) unsigned long error; struct elf_phdr *elf_ppnt, *elf_phdata, *interp_elf_phdata = NULL; struct elf_phdr *elf_property_phdata = NULL; - unsigned long elf_bss, elf_brk; + unsigned long elf_brk; int retval, i; unsigned long elf_entry; unsigned long e_entry; @@ -1047,7 +1047,6 @@ static int load_elf_binary(struct linux_binprm *bprm) if (retval < 0) goto out_free_dentry; - elf_bss = 0; elf_brk = 0;
start_code = ~0UL; @@ -1210,8 +1209,6 @@ static int load_elf_binary(struct linux_binprm *bprm)
k = elf_ppnt->p_vaddr + elf_ppnt->p_filesz;
- if (k > elf_bss) - elf_bss = k; if ((elf_ppnt->p_flags & PF_X) && end_code < k) end_code = k; if (end_data < k) @@ -1223,7 +1220,6 @@ static int load_elf_binary(struct linux_binprm *bprm)
e_entry = elf_ex->e_entry + load_bias; phdr_addr += load_bias; - elf_bss += load_bias; elf_brk += load_bias; start_code += load_bias; end_code += load_bias;
[ Sasha's backport helper bot ]
Hi,
✅ All tests passed successfully. No issues detected. No action required from the submitter.
The upstream commit SHA1 provided is correct: 8ed2ef21ff564cf4a25c098ace510ee6513c9836
WARNING: Author mismatch between patch and upstream commit: Backport author: Kang Wenlinwenlin.kang@windriver.com Commit author: Kees Cookkeescook@chromium.org
Status in newer kernel trees: 6.13.y | Present (exact SHA1) 6.12.y | Present (exact SHA1) 6.6.y | Not found
Note: The patch differs from the upstream commit: --- 1: 8ed2ef21ff564 ! 1: b68e62cd6249e binfmt_elf: elf_bss no longer used by load_elf_binary() @@ Metadata ## Commit message ## binfmt_elf: elf_bss no longer used by load_elf_binary()
+ commit 8ed2ef21ff564cf4a25c098ace510ee6513c9836 upstream + With the BSS handled generically via the new filesz/memsz mismatch handling logic in elf_load(), elf_bss no longer needs to be tracked. Drop the variable. @@ Commit message Signed-off-by: Sebastian Ott sebott@redhat.com Link: https://lore.kernel.org/r/20230929032435.2391507-2-keescook@chromium.org Signed-off-by: Kees Cook keescook@chromium.org + Signed-off-by: Wenlin Kang wenlin.kang@windriver.com
## fs/binfmt_elf.c ## @@ fs/binfmt_elf.c: static int load_elf_binary(struct linux_binprm *bprm) @@ fs/binfmt_elf.c: static int load_elf_binary(struct linux_binprm *bprm) @@ fs/binfmt_elf.c: static int load_elf_binary(struct linux_binprm *bprm) if (retval < 0) goto out_free_dentry; - + - elf_bss = 0; elf_brk = 0;
---
Results of testing on various branches:
| Branch | Patch Apply | Build Test | |---------------------------|-------------|------------| | stable/linux-6.6.y | Success | Success |
From: Kees Cook keescook@chromium.org
commit 8b04d32678e3c46b8a738178e0e55918eaa3be17 upstream
Handle arbitrary memsz>filesz in interpreter ELF segments, instead of only supporting it in the last segment (which is expected to be the BSS).
Cc: Eric Biederman ebiederm@xmission.com Cc: Alexander Viro viro@zeniv.linux.org.uk Cc: Christian Brauner brauner@kernel.org Cc: linux-fsdevel@vger.kernel.org Cc: linux-mm@kvack.org Reported-by: Pedro Falcato pedro.falcato@gmail.com Closes: https://lore.kernel.org/lkml/20221106021657.1145519-1-pedro.falcato@gmail.co... Tested-by: Pedro Falcato pedro.falcato@gmail.com Signed-off-by: Sebastian Ott sebott@redhat.com Link: https://lore.kernel.org/r/20230929032435.2391507-3-keescook@chromium.org Signed-off-by: Kees Cook keescook@chromium.org Signed-off-by: Wenlin Kang wenlin.kang@windriver.com --- fs/binfmt_elf.c | 46 +--------------------------------------------- 1 file changed, 1 insertion(+), 45 deletions(-)
diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c index 631219c3ace9..64c5e5cd0cd8 100644 --- a/fs/binfmt_elf.c +++ b/fs/binfmt_elf.c @@ -623,8 +623,6 @@ static unsigned long load_elf_interp(struct elfhdr *interp_elf_ex, struct elf_phdr *eppnt; unsigned long load_addr = 0; int load_addr_set = 0; - unsigned long last_bss = 0, elf_bss = 0; - int bss_prot = 0; unsigned long error = ~0UL; unsigned long total_size; int i; @@ -661,7 +659,7 @@ static unsigned long load_elf_interp(struct elfhdr *interp_elf_ex, else if (no_base && interp_elf_ex->e_type == ET_DYN) load_addr = -vaddr;
- map_addr = elf_map(interpreter, load_addr + vaddr, + map_addr = elf_load(interpreter, load_addr + vaddr, eppnt, elf_prot, elf_type, total_size); total_size = 0; error = map_addr; @@ -687,51 +685,9 @@ static unsigned long load_elf_interp(struct elfhdr *interp_elf_ex, error = -ENOMEM; goto out; } - - /* - * Find the end of the file mapping for this phdr, and - * keep track of the largest address we see for this. - */ - k = load_addr + eppnt->p_vaddr + eppnt->p_filesz; - if (k > elf_bss) - elf_bss = k; - - /* - * Do the same thing for the memory mapping - between - * elf_bss and last_bss is the bss section. - */ - k = load_addr + eppnt->p_vaddr + eppnt->p_memsz; - if (k > last_bss) { - last_bss = k; - bss_prot = elf_prot; - } } }
- /* - * Now fill out the bss section: first pad the last page from - * the file up to the page boundary, and zero it from elf_bss - * up to the end of the page. - */ - if (padzero(elf_bss)) { - error = -EFAULT; - goto out; - } - /* - * Next, align both the file and mem bss up to the page size, - * since this is where elf_bss was just zeroed up to, and where - * last_bss will end after the vm_brk_flags() below. - */ - elf_bss = ELF_PAGEALIGN(elf_bss); - last_bss = ELF_PAGEALIGN(last_bss); - /* Finally, if there is still more bss to allocate, do it. */ - if (last_bss > elf_bss) { - error = vm_brk_flags(elf_bss, last_bss - elf_bss, - bss_prot & PROT_EXEC ? VM_EXEC : 0); - if (error) - goto out; - } - error = load_addr; out: return error;
[ Sasha's backport helper bot ]
Hi,
✅ All tests passed successfully. No issues detected. No action required from the submitter.
The upstream commit SHA1 provided is correct: 8b04d32678e3c46b8a738178e0e55918eaa3be17
WARNING: Author mismatch between patch and upstream commit: Backport author: Kang Wenlinwenlin.kang@windriver.com Commit author: Kees Cookkeescook@chromium.org
Status in newer kernel trees: 6.13.y | Present (exact SHA1) 6.12.y | Present (exact SHA1) 6.6.y | Not found
Note: The patch differs from the upstream commit: --- 1: 8b04d32678e3c ! 1: 3b015308f7b5b binfmt_elf: Use elf_load() for interpreter @@ Metadata ## Commit message ## binfmt_elf: Use elf_load() for interpreter
+ commit 8b04d32678e3c46b8a738178e0e55918eaa3be17 upstream + Handle arbitrary memsz>filesz in interpreter ELF segments, instead of only supporting it in the last segment (which is expected to be the BSS). @@ Commit message Signed-off-by: Sebastian Ott sebott@redhat.com Link: https://lore.kernel.org/r/20230929032435.2391507-3-keescook@chromium.org Signed-off-by: Kees Cook keescook@chromium.org + Signed-off-by: Wenlin Kang wenlin.kang@windriver.com
## fs/binfmt_elf.c ## @@ fs/binfmt_elf.c: static unsigned long load_elf_interp(struct elfhdr *interp_elf_ex, ---
Results of testing on various branches:
| Branch | Patch Apply | Build Test | |---------------------------|-------------|------------| | stable/linux-6.6.y | Success | Success |
From: Kees Cook keescook@chromium.org
commit d5ca24f639588811af57ceac513183fa2004bd3a upstream
While load_elf_library() is a libc5-ism, we can still replace most of its contents with elf_load() as well, further simplifying the code.
Some historical context: - libc4 was a.out and used uselib (a.out support has been removed) - libc5 was ELF and used uselib (there may still be users) - libc6 is ELF and has never used uselib
Cc: Alexander Viro viro@zeniv.linux.org.uk Cc: Christian Brauner brauner@kernel.org Cc: linux-fsdevel@vger.kernel.org Cc: linux-mm@kvack.org Suggested-by: Eric Biederman ebiederm@xmission.com Tested-by: Pedro Falcato pedro.falcato@gmail.com Signed-off-by: Sebastian Ott sebott@redhat.com Link: https://lore.kernel.org/r/20230929032435.2391507-4-keescook@chromium.org Signed-off-by: Kees Cook keescook@chromium.org Signed-off-by: Wenlin Kang wenlin.kang@windriver.com --- fs/binfmt_elf.c | 24 ++++-------------------- 1 file changed, 4 insertions(+), 20 deletions(-)
diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c index 64c5e5cd0cd8..ba1ef7e3f9f3 100644 --- a/fs/binfmt_elf.c +++ b/fs/binfmt_elf.c @@ -1308,7 +1308,6 @@ static int load_elf_library(struct file *file) { struct elf_phdr *elf_phdata; struct elf_phdr *eppnt; - unsigned long elf_bss, bss, len; int retval, error, i, j; struct elfhdr elf_ex;
@@ -1353,30 +1352,15 @@ static int load_elf_library(struct file *file) eppnt++;
/* Now use mmap to map the library into memory. */ - error = vm_mmap(file, - ELF_PAGESTART(eppnt->p_vaddr), - (eppnt->p_filesz + - ELF_PAGEOFFSET(eppnt->p_vaddr)), + error = elf_load(file, ELF_PAGESTART(eppnt->p_vaddr), + eppnt, PROT_READ | PROT_WRITE | PROT_EXEC, MAP_FIXED_NOREPLACE | MAP_PRIVATE, - (eppnt->p_offset - - ELF_PAGEOFFSET(eppnt->p_vaddr))); - if (error != ELF_PAGESTART(eppnt->p_vaddr)) - goto out_free_ph; + 0);
- elf_bss = eppnt->p_vaddr + eppnt->p_filesz; - if (padzero(elf_bss)) { - error = -EFAULT; + if (error != ELF_PAGESTART(eppnt->p_vaddr)) goto out_free_ph; - }
- len = ELF_PAGEALIGN(eppnt->p_filesz + eppnt->p_vaddr); - bss = ELF_PAGEALIGN(eppnt->p_memsz + eppnt->p_vaddr); - if (bss > len) { - error = vm_brk(len, bss - len); - if (error) - goto out_free_ph; - } error = 0;
out_free_ph:
[ Sasha's backport helper bot ]
Hi,
✅ All tests passed successfully. No issues detected. No action required from the submitter.
The upstream commit SHA1 provided is correct: d5ca24f639588811af57ceac513183fa2004bd3a
WARNING: Author mismatch between patch and upstream commit: Backport author: Kang Wenlinwenlin.kang@windriver.com Commit author: Kees Cookkeescook@chromium.org
Status in newer kernel trees: 6.13.y | Present (exact SHA1) 6.12.y | Present (exact SHA1) 6.6.y | Not found
Note: The patch differs from the upstream commit: --- 1: d5ca24f639588 ! 1: ccf2de90c4543 binfmt_elf: Use elf_load() for library @@ Metadata ## Commit message ## binfmt_elf: Use elf_load() for library
+ commit d5ca24f639588811af57ceac513183fa2004bd3a upstream + While load_elf_library() is a libc5-ism, we can still replace most of its contents with elf_load() as well, further simplifying the code.
@@ Commit message Signed-off-by: Sebastian Ott sebott@redhat.com Link: https://lore.kernel.org/r/20230929032435.2391507-4-keescook@chromium.org Signed-off-by: Kees Cook keescook@chromium.org + Signed-off-by: Wenlin Kang wenlin.kang@windriver.com
## fs/binfmt_elf.c ## @@ fs/binfmt_elf.c: static int load_elf_library(struct file *file) ---
Results of testing on various branches:
| Branch | Patch Apply | Build Test | |---------------------------|-------------|------------| | stable/linux-6.6.y | Success | Success |
From: Kees Cook keescook@chromium.org
commit f9c0a39d95301a36baacfd3495374c6128d662fa upstream
Errors with padzero() should be caught unless we're expecting a pathological (non-writable) segment. Report -EFAULT only when PROT_WRITE is present.
Additionally add some more documentation to padzero(), elf_map(), and elf_load().
Cc: Eric Biederman ebiederm@xmission.com Cc: Alexander Viro viro@zeniv.linux.org.uk Cc: Christian Brauner brauner@kernel.org Cc: linux-fsdevel@vger.kernel.org Cc: linux-mm@kvack.org Suggested-by: Eric Biederman ebiederm@xmission.com Tested-by: Pedro Falcato pedro.falcato@gmail.com Signed-off-by: Sebastian Ott sebott@redhat.com Link: https://lore.kernel.org/r/20230929032435.2391507-5-keescook@chromium.org Signed-off-by: Kees Cook keescook@chromium.org Signed-off-by: Wenlin Kang wenlin.kang@windriver.com --- fs/binfmt_elf.c | 32 +++++++++++++++++++++++--------- 1 file changed, 23 insertions(+), 9 deletions(-)
diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c index ba1ef7e3f9f3..73e9e6131732 100644 --- a/fs/binfmt_elf.c +++ b/fs/binfmt_elf.c @@ -109,19 +109,19 @@ static struct linux_binfmt elf_format = {
#define BAD_ADDR(x) (unlikely((unsigned long)(x) >= TASK_SIZE))
-/* We need to explicitly zero any fractional pages - after the data section (i.e. bss). This would - contain the junk from the file that should not - be in memory +/* + * We need to explicitly zero any trailing portion of the page that follows + * p_filesz when it ends before the page ends (e.g. bss), otherwise this + * memory will contain the junk from the file that should not be present. */ -static int padzero(unsigned long elf_bss) +static int padzero(unsigned long address) { unsigned long nbyte;
- nbyte = ELF_PAGEOFFSET(elf_bss); + nbyte = ELF_PAGEOFFSET(address); if (nbyte) { nbyte = ELF_MIN_ALIGN - nbyte; - if (clear_user((void __user *) elf_bss, nbyte)) + if (clear_user((void __user *)address, nbyte)) return -EFAULT; } return 0; @@ -343,6 +343,11 @@ create_elf_tables(struct linux_binprm *bprm, const struct elfhdr *exec, return 0; }
+/* + * Map "eppnt->p_filesz" bytes from "filep" offset "eppnt->p_offset" + * into memory at "addr". (Note that p_filesz is rounded up to the + * next page, so any extra bytes from the file must be wiped.) + */ static unsigned long elf_map(struct file *filep, unsigned long addr, const struct elf_phdr *eppnt, int prot, int type, unsigned long total_size) @@ -382,6 +387,11 @@ static unsigned long elf_map(struct file *filep, unsigned long addr, return(map_addr); }
+/* + * Map "eppnt->p_filesz" bytes from "filep" offset "eppnt->p_offset" + * into memory at "addr". Memory from "p_filesz" through "p_memsz" + * rounded up to the next page is zeroed. + */ static unsigned long elf_load(struct file *filep, unsigned long addr, const struct elf_phdr *eppnt, int prot, int type, unsigned long total_size) @@ -399,8 +409,12 @@ static unsigned long elf_load(struct file *filep, unsigned long addr, zero_end = map_addr + ELF_PAGEOFFSET(eppnt->p_vaddr) + eppnt->p_memsz;
- /* Zero the end of the last mapped page */ - padzero(zero_start); + /* + * Zero the end of the last mapped page but ignore + * any errors if the segment isn't writable. + */ + if (padzero(zero_start) && (prot & PROT_WRITE)) + return -EFAULT; } } else { map_addr = zero_start = ELF_PAGESTART(addr);
[ Sasha's backport helper bot ]
Hi,
✅ All tests passed successfully. No issues detected. No action required from the submitter.
The upstream commit SHA1 provided is correct: f9c0a39d95301a36baacfd3495374c6128d662fa
WARNING: Author mismatch between patch and upstream commit: Backport author: Kang Wenlinwenlin.kang@windriver.com Commit author: Kees Cookkeescook@chromium.org
Status in newer kernel trees: 6.13.y | Present (exact SHA1) 6.12.y | Present (exact SHA1) 6.6.y | Not found
Note: The patch differs from the upstream commit: --- 1: f9c0a39d95301 ! 1: 6dcb8e418404b binfmt_elf: Only report padzero() errors when PROT_WRITE @@ Metadata ## Commit message ## binfmt_elf: Only report padzero() errors when PROT_WRITE
+ commit f9c0a39d95301a36baacfd3495374c6128d662fa upstream + Errors with padzero() should be caught unless we're expecting a pathological (non-writable) segment. Report -EFAULT only when PROT_WRITE is present. @@ Commit message Signed-off-by: Sebastian Ott sebott@redhat.com Link: https://lore.kernel.org/r/20230929032435.2391507-5-keescook@chromium.org Signed-off-by: Kees Cook keescook@chromium.org + Signed-off-by: Wenlin Kang wenlin.kang@windriver.com
## fs/binfmt_elf.c ## @@ fs/binfmt_elf.c: static struct linux_binfmt elf_format = { ---
Results of testing on various branches:
| Branch | Patch Apply | Build Test | |---------------------------|-------------|------------| | stable/linux-6.6.y | Success | Success |
On Mon, Mar 24, 2025 at 03:19:41PM +0800, Kang Wenlin wrote:
From: Kees Cook keescook@chromium.org
commit f9c0a39d95301a36baacfd3495374c6128d662fa upstream
We obviously can not take a patch for 6.1.y that is not already in 6.6.y, right? Otherwise when you upgrade to the newer version you will have a regression.
Please fix this up by sending the newer kernel backports first, and then, if they are accepted, send the older ones. As it is, we can't take this series at all, sorry.
greg k-h
On 4/1/25 17:58, Greg KH wrote:
We obviously can not take a patch for 6.1.y that is not already in 6.6.y, right? Otherwise when you upgrade to the newer version you will have a regression.
Please fix this up by sending the newer kernel backports first, and then, if they are accepted, send the older ones. As it is, we can't take this series at all, sorry.
Got it, thanks for the clarification. I'll submit the backports for the newer kernel(6.6.y) first, and follow up with the older ones if they get accepted.
From: Kees Cook keescook@chromium.org
commit 2632bb84d1d53cfd6cf65261064273ded4f759d5 upstream
With fs/binfmt_elf.c fully refactored to use the new elf_load() helper, there are no more users of vm_brk(), so remove it.
Cc: Andrew Morton akpm@linux-foundation.org Cc: linux-mm@kvack.org Suggested-by: Eric Biederman ebiederm@xmission.com Tested-by: Pedro Falcato pedro.falcato@gmail.com Signed-off-by: Sebastian Ott sebott@redhat.com Link: https://lore.kernel.org/r/20230929032435.2391507-6-keescook@chromium.org Signed-off-by: Kees Cook keescook@chromium.org Signed-off-by: Wenlin Kang wenlin.kang@windriver.com --- include/linux/mm.h | 3 +-- mm/mmap.c | 6 ------ mm/nommu.c | 5 ----- 3 files changed, 1 insertion(+), 13 deletions(-)
diff --git a/include/linux/mm.h b/include/linux/mm.h index 03357c196e0b..fe9df9f7ad2c 100644 --- a/include/linux/mm.h +++ b/include/linux/mm.h @@ -2826,8 +2826,7 @@ static inline void mm_populate(unsigned long addr, unsigned long len) static inline void mm_populate(unsigned long addr, unsigned long len) {} #endif
-/* These take the mm semaphore themselves */ -extern int __must_check vm_brk(unsigned long, unsigned long); +/* This takes the mm semaphore itself */ extern int __must_check vm_brk_flags(unsigned long, unsigned long, unsigned long); extern int vm_munmap(unsigned long, size_t); extern unsigned long __must_check vm_mmap(struct file *, unsigned long, diff --git a/mm/mmap.c b/mm/mmap.c index ebc3583fa612..6054a513ea94 100644 --- a/mm/mmap.c +++ b/mm/mmap.c @@ -3202,12 +3202,6 @@ int vm_brk_flags(unsigned long addr, unsigned long request, unsigned long flags) } EXPORT_SYMBOL(vm_brk_flags);
-int vm_brk(unsigned long addr, unsigned long len) -{ - return vm_brk_flags(addr, len, 0); -} -EXPORT_SYMBOL(vm_brk); - /* Release all mmaps. */ void exit_mmap(struct mm_struct *mm) { diff --git a/mm/nommu.c b/mm/nommu.c index 859ba6bdeb9c..586c0654f3d9 100644 --- a/mm/nommu.c +++ b/mm/nommu.c @@ -1572,11 +1572,6 @@ void exit_mmap(struct mm_struct *mm) mmap_write_unlock(mm); }
-int vm_brk(unsigned long addr, unsigned long len) -{ - return -ENOMEM; -} - /* * expand (or shrink) an existing mapping, potentially moving it at the same * time (controlled by the MREMAP_MAYMOVE flag and available VM space)
[ Sasha's backport helper bot ]
Hi,
✅ All tests passed successfully. No issues detected. No action required from the submitter.
The upstream commit SHA1 provided is correct: 2632bb84d1d53cfd6cf65261064273ded4f759d5
WARNING: Author mismatch between patch and upstream commit: Backport author: Kang Wenlinwenlin.kang@windriver.com Commit author: Kees Cookkeescook@chromium.org
Status in newer kernel trees: 6.13.y | Present (exact SHA1) 6.12.y | Present (exact SHA1) 6.6.y | Not found
Note: The patch differs from the upstream commit: --- 1: 2632bb84d1d53 ! 1: c5b8fd8379e4c mm: Remove unused vm_brk() @@ Metadata ## Commit message ## mm: Remove unused vm_brk()
+ commit 2632bb84d1d53cfd6cf65261064273ded4f759d5 upstream + With fs/binfmt_elf.c fully refactored to use the new elf_load() helper, there are no more users of vm_brk(), so remove it.
@@ Commit message Signed-off-by: Sebastian Ott sebott@redhat.com Link: https://lore.kernel.org/r/20230929032435.2391507-6-keescook@chromium.org Signed-off-by: Kees Cook keescook@chromium.org + Signed-off-by: Wenlin Kang wenlin.kang@windriver.com
## include/linux/mm.h ## @@ include/linux/mm.h: static inline void mm_populate(unsigned long addr, unsigned long len) ---
Results of testing on various branches:
| Branch | Patch Apply | Build Test | |---------------------------|-------------|------------| | stable/linux-6.6.y | Success | Success |
linux-stable-mirror@lists.linaro.org