On Wed, Feb 19, 2025 at 05:20:17PM +0100, Jan Kara wrote: > On Tue 18-02-25 19:53:51, Brian Mak wrote: > > On Feb 18, 2025, at 12:54 AM, Michael Stapelberg <michael@xxxxxxxxxxxxx> wrote: > > > > > I think in your testing, you probably did not try the eu-stack tool > > > from the elfutils package, because I think I found a bug: > > > > Hi Michael, > > > > Thanks for the report. I can confirm that this issue does seem to be > > from this commit. I tested it with Juniper's Linux kernel with and > > without the changes. > > > > You're correct that the original testing done did not include the > > eu-stack tool. > > > > > Current elfutils cannot symbolize core dumps created by Linux 6.12+. > > > I noticed this because systemd-coredump(8) uses elfutils, and when > > > a program crashed on my machine, syslog did not show function names. > > > > > > I reported this issue with elfutils at: > > > https://urldefense.com/v3/__https://sourceware.org/bugzilla/show_bug.cgi?id=32713__;!!NEt6yMaO-gk!DbttKuHxkBdrV4Cj9axM3ED6mlBHXeQGY3NVzvfDlthl-K39e9QIrZcwT8iCXLRu0OivWRGgficcD-aCuus$ > > > …but figured it would be good to give a heads-up here, too. > > > > > > Is this breakage sufficient reason to revert the commit? > > > Or are we saying userspace just needs to be updated to cope? > > > > The way I see it is that, as long as we're in compliance with the > > applicable ELF specifications, then the issue lies with userspace apps > > to ensure that they are not making additional erroneous assumptions. > > > > However, Eric mentioned a while ago in v1 of this patch that he believes > > that the ELF specification requires program headers be written in memory > > order. Digging through the ELF specifications, I found that any loadable > > segment entries in the program header table must be sorted on the > > virtual address of the first byte of which the segment resides in > > memory. > > > > This indicates that we have deviated from the ELF specification with > > this commit. One thing we can do to remedy this is to have program > > headers sorted according to the specification, but then continue dumping > > in VMA size ordering. This would make the dumping logic significantly > > more complex though. > > > > Seeing how most popular userspace apps, with the exception of eu-stack, > > seem to work, we could also just leave it, and tell userspace apps to > > fix it on their end. > > Well, it does not seem eu-stack is that unpopular and we really try hard to > avoid user visible regressions. So I think we should revert the change. Also > the fact that the patch breaks ELF spec is an indication there may be other > tools that would get confused by this and another reason for a revert... Yeah, I think we need to make this a tunable. Updating the kernel breaks elftools, which isn't some weird custom corner case. :P So, while it took a few months, here is a report of breakage that I said we'd need to watch for[1]. :) Is anyone able to test this patch? And Brian will setting a sysctl be okay for your use-case? diff --git a/Documentation/admin-guide/sysctl/kernel.rst b/Documentation/admin-guide/sysctl/kernel.rst index a43b78b4b646..35d5d86cff69 100644 --- a/Documentation/admin-guide/sysctl/kernel.rst +++ b/Documentation/admin-guide/sysctl/kernel.rst @@ -222,6 +222,17 @@ and ``core_uses_pid`` is set, then .PID will be appended to the filename. +core_sort_vma +============= + +The default coredump writes VMAs in address order. By setting +``core_sort_vma`` to 1, VMAs will be written from smallest size +to largest size. This is known to break at least elfutils, but +can be handy when dealing with very large (and truncated) +coredumps where the more useful debugging details are included +in the smaller VMAs. + + ctrl-alt-del ============ diff --git a/fs/coredump.c b/fs/coredump.c index 591700e1b2ce..4375c70144d0 100644 --- a/fs/coredump.c +++ b/fs/coredump.c @@ -63,6 +63,7 @@ static void free_vma_snapshot(struct coredump_params *cprm); static int core_uses_pid; static unsigned int core_pipe_limit; +static unsigned int core_sort_vma; static char core_pattern[CORENAME_MAX_SIZE] = "core"; static int core_name_size = CORENAME_MAX_SIZE; unsigned int core_file_note_size_limit = CORE_FILE_NOTE_SIZE_DEFAULT; @@ -1026,6 +1027,15 @@ static const struct ctl_table coredump_sysctls[] = { .extra1 = (unsigned int *)&core_file_note_size_min, .extra2 = (unsigned int *)&core_file_note_size_max, }, + { + .procname = "core_sort_vma", + .data = &core_sort_vma, + .maxlen = sizeof(int), + .mode = 0644, + .proc_handler = proc_douintvec_minmax, + .extra1 = SYSCTL_ZERO, + .extra2 = SYSCTL_ONE, + }, }; static int __init init_fs_coredump_sysctls(void) @@ -1256,8 +1266,9 @@ static bool dump_vma_snapshot(struct coredump_params *cprm) cprm->vma_data_size += m->dump_size; } - sort(cprm->vma_meta, cprm->vma_count, sizeof(*cprm->vma_meta), - cmp_vma_size, NULL); + if (core_sort_vma) + sort(cprm->vma_meta, cprm->vma_count, sizeof(*cprm->vma_meta), + cmp_vma_size, NULL); return true; } -Kees [1] https://lore.kernel.org/all/202408092104.FCE51021@keescook/ -- Kees Cook
