linux-kernel - Re: [PATCH v3 1/2] fs/binfmt_elf: Use PT_LOAD p

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] [day] [month] [year] [list]

Message-Id: <20200820205116.3467a186cb4ac20342f0ee4b@linux-foundation.org>
Date:   Thu, 20 Aug 2020 20:51:16 -0700
From:   Andrew Morton <akpm@...ux-foundation.org>
To:     Chris Kennelly <ckennelly@...gle.com>
Cc:     Alexander Viro <viro@...iv.linux.org.uk>,
        Alexey Dobriyan <adobriyan@...il.com>,
        Song Liu <songliubraving@...com>,
        David Rientjes <rientjes@...gle.com>,
        Ian Rogers <irogers@...gle.com>,
        Hugh Dickens <hughd@...gle.com>,
        Suren Baghdasaryan <surenb@...gle.com>,
        Sandeep Patil <sspatil@...gle.com>,
        Fangrui Song <maskray@...gle.com>,
        Nick Desaulniers <ndesaulniers@...gle.com>,
        clang-built-linux@...glegroups.com, linux-fsdevel@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 1/2] fs/binfmt_elf: Use PT_LOAD p_align values for
 suitable start address.

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