linux-kernel - Re: [PATCH v3] binfmt_elf: Dump smaller VMAs first in ELF cores

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <F9EA3BEC-4E23-4DBB-8CBC-08EEBB39D28F@juniper.net>
Date: Sat, 22 Feb 2025 02:13:06 +0000
From: Brian Mak <makb@...iper.net>
To: Kees Cook <kees@...nel.org>
CC: Jan Kara <jack@...e.cz>, Michael Stapelberg <michael@...pelberg.ch>,
        Christian Brauner <brauner@...nel.org>,
        "Eric W. Biederman"
	<ebiederm@...ssion.com>,
        "linux-fsdevel@...r.kernel.org"
	<linux-fsdevel@...r.kernel.org>,
        "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>,
        "linux-mm@...ck.org" <linux-mm@...ck.org>,
        Oleg Nesterov <oleg@...hat.com>,
        Linus Torvalds
	<torvalds@...ux-foundation.org>,
        Alexander Viro <viro@...iv.linux.org.uk>
Subject: Re: [PATCH v3] binfmt_elf: Dump smaller VMAs first in ELF cores

On Feb 19, 2025, at 12:38 PM, Brian Mak <makb@...iper.net> wrote

> I will also scratch up a patch to bring us back into compliance with the
> ELF specifications, and see if that fixes the userspace breakage with
> elfutils, while not breaking gdb or rr.

I did scratch up something for this to fix up the program header
ordering, but it seems eu-stack is still broken, even with the fix. GDB
continues to work fine with the fix.

Given that there's no known utilities that get fixed as a result of the
program header sorting, I'm not sure if it's worth taking the patch.
Maybe we can just proceed with the sysctl + sorting if the core dump
size limit is hit, and leave it at that. Thoughts?

The program header ordering fix is below if someone wants to peek at it.

Best,
Brian

diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
index 8054f44d39cf..8cf2bbc3cedf 100644
--- a/fs/binfmt_elf.c
+++ b/fs/binfmt_elf.c
@@ -2021,6 +2021,7 @@ static int elf_core_dump(struct coredump_params *cprm)
 	struct elf_shdr *shdr4extnum = NULL;
 	Elf_Half e_phnum;
 	elf_addr_t e_shoff;
+	struct elf_phdr *phdrs = NULL;
 
 	/*
 	 * The number of segs are recored into ELF header as 16bit value.
@@ -2084,7 +2085,11 @@ static int elf_core_dump(struct coredump_params *cprm)
 	if (!dump_emit(cprm, phdr4note, sizeof(*phdr4note)))
 		goto end_coredump;
 
-	/* Write program headers for segments dump */
+	phdrs = kvmalloc_array(cprm->vma_count, sizeof(*phdrs), GFP_KERNEL);
+	if (!phdrs)
+		goto end_coredump;
+
+	/* Construct sorted program headers for segments dump */
 	for (i = 0; i < cprm->vma_count; i++) {
 		struct core_vma_metadata *meta = cprm->vma_meta + i;
 		struct elf_phdr phdr;
@@ -2104,8 +2109,14 @@ static int elf_core_dump(struct coredump_params *cprm)
 		if (meta->flags & VM_EXEC)
 			phdr.p_flags |= PF_X;
 		phdr.p_align = ELF_EXEC_PAGESIZE;
+		phdrs[meta->index] = phdr;
+	}
+
+	/* Write program headers for segments dump */
+	for (i = 0; i < cprm->vma_count; i++) {
+		struct elf_phdr *phdr = phdrs + i;
 
-		if (!dump_emit(cprm, &phdr, sizeof(phdr)))
+		if (!dump_emit(cprm, phdr, sizeof(*phdr)))
 			goto end_coredump;
 	}
 
@@ -2140,6 +2151,7 @@ static int elf_core_dump(struct coredump_params *cprm)
 
 end_coredump:
 	free_note_info(&info);
+	kvfree(phdrs);
 	kfree(shdr4extnum);
 	kfree(phdr4note);
 	return has_dumped;
diff --git a/fs/coredump.c b/fs/coredump.c
index 591700e1b2ce..0ddd75c3a914 100644
--- a/fs/coredump.c
+++ b/fs/coredump.c
@@ -1226,6 +1226,7 @@ static bool dump_vma_snapshot(struct coredump_params *cprm)
 	while ((vma = coredump_next_vma(&vmi, vma, gate_vma)) != NULL) {
 		struct core_vma_metadata *m = cprm->vma_meta + i;
 
+		m->index = i;
 		m->start = vma->vm_start;
 		m->end = vma->vm_end;
 		m->flags = vma->vm_flags;
diff --git a/include/linux/coredump.h b/include/linux/coredump.h
index 77e6e195d1d6..cf1b9e53cd1e 100644
--- a/include/linux/coredump.h
+++ b/include/linux/coredump.h
@@ -9,6 +9,7 @@
 
 #ifdef CONFIG_COREDUMP
 struct core_vma_metadata {
+	unsigned int  index;
 	unsigned long start, end;
 	unsigned long flags;
 	unsigned long dump_size;