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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <875xs93glh.fsf@email.froward.int.ebiederm.org>
Date: Fri, 09 Aug 2024 09:39:54 -0500
From: "Eric W. Biederman" <ebiederm@...ssion.com>
To: Linus Torvalds <torvalds@...ux-foundation.org>
Cc: Brian Mak <makb@...iper.net>,  Kees Cook <kees@...nel.org>,  Alexander
 Viro <viro@...iv.linux.org.uk>,  Christian Brauner <brauner@...nel.org>,
  Jan Kara <jack@...e.cz>,  "linux-fsdevel@...r.kernel.org"
 <linux-fsdevel@...r.kernel.org>,  "linux-mm@...ck.org"
 <linux-mm@...ck.org>,  "linux-kernel@...r.kernel.org"
 <linux-kernel@...r.kernel.org>,  Oleg Nesterov <oleg@...hat.com>
Subject: Re: [PATCH v3] binfmt_elf: Dump smaller VMAs first in ELF cores

Linus Torvalds <torvalds@...ux-foundation.org> writes:

> On Tue, 6 Aug 2024 at 11:16, Brian Mak <makb@...iper.net> wrote:
>>
>> @@ -1253,5 +1266,8 @@ 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);
>> +
>>         return true;
>>  }
>
> Hmm. Realistically we only dump core in ELF, and the order of the
> segments shouldn't matter.
>
> But I wonder if we should do this in the ->core_dump() function
> itself, in case it would have mattered for other dump formats?
>
> IOW, instead of being at the bottom of dump_vma_snapshot(), maybe the
> sorting should be at the top of elf_core_dump()?
>
> And yes, in practice I doubt we'll ever have other dump formats, and
> no, a.out isn't doing some miraculous comeback either.
>
> But I bet you didn't test elf_fdpic_core_dump() even if I bet it (a)
> works and (b) nobody cares.
>
> So moving it to the ELF side might be conceptually the right thing to do?
>
> (Or is there some reason it needs to be done at snapshot time that I
> just didn't fully appreciate?)

I asked him to perform this at snapshot time.  Plus it is obvious at
snapshot time that you can change the allocated array, while it is
not so obvious in the ->core_dump methods.

I would argue that the long term maintainable thing to do is to
merge elf_core_dump and elf_fdpic_core_dump and put all of the code
in fs/coredump.c

Performing the sort at snapshot time avoids introducing one extra reason
why the two elf implementations of elf coredumping are different.

I did read through the elf fdpic code quickly and it looks like it
should just work no matter which order the vma's are dumped in.  Just
like the other elf coredump code does.

My practical concern is that someone has a coredump thing that walks
through the program headers and short circuits the walk because it knows
the program headers are all written in order.  But the only way to find
one of those is to just try it.

Eric