Re: [PATCH v3 1/2] fs/binfmt_elf: Use PT_LOAD p_align values for suitable start address.

Andrew Morton
2020-08-21T03:51:22Z

On Thu, 20 Aug 2020 13:05:40 -0400 Chris Kennelly <ckennelly@google.com> wrote:

> The current ELF loading mechancism provides page-aligned mappings. This
> can lead to the program being loaded in a way unsuitable for
> file-backed, transparent huge pages when handling PIE executables.
>
> For binaries built with increased alignment, this limits the number of
> bits usable for ASLR, but provides some randomization over using fixed
> load addresses/non-PIE binaries.
>
> @@ -421,6 +422,24 @@ static int elf_read(struct file *file, void *buf, size_t len, loff_t pos)
> return 0;
> }
>
> +static unsigned long maximum_alignment(struct elf_phdr *cmds, int nr)
> +{
> + unsigned long alignment = 0;
> + int i;
> +
> + for (i = 0; i < nr; i++) {
> + if (cmds[i].p_type == PT_LOAD) {
> + /* skip non-power of two alignments */

Comment isn't terribly helpful. It explains "what" (which is utterly
obvious from the code anyway) but it fails to explain "why".

> + if (!is_power_of_2(cmds[i].p_align))
> + continue;
> + alignment = max(alignment, cmds[i].p_align);

generates a max() warning:

fs/binfmt_elf.c:435:16: note: in expansion of macro `max'
alignment = max(alignment, cmds[i].p_align);

p_align may be Elf64_Xword, may be Elf32_Word, may be something else.
That's quite unwieldy and I don't like max_t. How about this?

--- a/fs/binfmt_elf.c~fs-binfmt_elf-use-pt_load-p_align-values-for-suitable-start-address-fix
+++ a/fs/binfmt_elf.c
@@ -429,10 +429,12 @@ static unsigned long maximum_alignment(s

for (i = 0; i < nr; i++) {
if (cmds[i].p_type == PT_LOAD) {
+ unsigned long p_align = cmds[i].p_align;
+
/* skip non-power of two alignments */
- if (!is_power_of_2(cmds[i].p_align))
+ if (!is_power_of_2(p_align))
continue;
- alignment = max(alignment, cmds[i].p_align);
+ alignment = max(alignment, p_align);
}
}

_