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

[Andrew Morton]

On Thu, 20 Aug 2020 13:05:40 -0400 Chris Kennelly <ckennelly@xxxxxxxxxx> 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);
 		}
 	}
 
_