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

[Brian Mak <makb@juniper.net>]

On Feb 19, 2025, at 12:38 PM, Brian Mak <makb@juniper.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;