diff mbox series

[v4,4/6] binfmt_elf: Use elf_load() for library

Message ID 20230929032435.2391507-4-keescook@chromium.org
State New
Headers
Series binfmt_elf: Support segments with 0 filesz and misaligned starts |

Commit Message

Kees Cook Sept. 29, 2023, 3:24 a.m. UTC 
  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.

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>
Signed-off-by: Kees Cook <keescook@chromium.org>
---
 fs/binfmt_elf.c | 23 ++++-------------------
 1 file changed, 4 insertions(+), 19 deletions(-)

Comments

Pedro Falcato Sept. 29, 2023, 12:12 p.m. UTC | #1 
On Fri, Sep 29, 2023 at 4:24 AM Kees Cook <keescook@chromium.org> wrote:
>
> 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.

While I understand you want to break as little as possible (as the ELF
loader maintainer), I'm wondering if we could axe CONFIG_USELIB
altogether? Since CONFIG_BINFMT_AOUT also got axed. Does this have
users anywhere?
Kees Cook Sept. 29, 2023, 5:06 p.m. UTC | #2 
On Fri, Sep 29, 2023 at 01:12:13PM +0100, Pedro Falcato wrote:
> On Fri, Sep 29, 2023 at 4:24 AM Kees Cook <keescook@chromium.org> wrote:
> >
> > 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.
> 
> While I understand you want to break as little as possible (as the ELF
> loader maintainer), I'm wondering if we could axe CONFIG_USELIB
> altogether? Since CONFIG_BINFMT_AOUT also got axed. Does this have
> users anywhere?

I can't even find a libc5 image I can test. :P

I made it non-default in '22:

7374fa33dc2d ("init/Kconfig: remove USELIB syscall by default")

I'm not sure we can drop it entirely, though.
diff mbox series

Patch

diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
index db47cb802f89..f8b4747f87ed 100644
--- a/fs/binfmt_elf.c
+++ b/fs/binfmt_elf.c
@@ -1351,30 +1351,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: