Hi Michael,
On 13 April 2015 at 23:49, Michael Davidson <md@xxxxxxxxxx> wrote:
> With CONFIG_ARCH_BINFMT_ELF_RANDOMIZE_PIE enabled, and a normal
> top-down address allocation strategy, load_elf_binary() will
> attempt to map a PIE binary into an address range immediately
> below mm->mmap_base.
>
> Unfortunately, load_elf_ binary() does not take account of the
> need to allocate sufficient space for the entire binary which
> means that, while the first PT_LOAD segment is mapped below
> mm->mmap_base, the subsequent PT_LOAD segment(s) end up being
> mapped above mm->mmap_base into the are that is supposed to
> be the "gap" between the stack and the binary.
>
> Since the size of the "gap" on x86_64 is only guaranteed to be
> 128MB this means that binaries with large data segments > 128MB
> can end up mapping part of their data segment over their stack
> resulting in corruption of the stack (and the data segment once
> the binary starts to run).
>
> Any PIE binary with a data segment > 128MB is vulnerable to this
> although address randomization means that the actual gap between
> the stack and the end of the binary is normally greater than 128MB.
> The larger the data segment of the binary the higher the probability
> of failure.
>
> Fix this by calculating the total size of the binary in the same
> way as load_elf_interp().
>
> Signed-off-by: Michael Davidson <md@xxxxxxxxxx>
> ---
> fs/binfmt_elf.c | 9 ++++++++-
> 1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
> index 995986b..d925f55 100644
> --- a/fs/binfmt_elf.c
> +++ b/fs/binfmt_elf.c
> @@ -862,6 +862,7 @@ static int load_elf_binary(struct linux_binprm *bprm)
> i < loc->elf_ex.e_phnum; i++, elf_ppnt++) {
> int elf_prot = 0, elf_flags;
> unsigned long k, vaddr;
> + unsigned long total_size = 0;
>
> if (elf_ppnt->p_type != PT_LOAD)
> continue;
> @@ -924,10 +925,16 @@ static int load_elf_binary(struct linux_binprm *bprm)
> #else
> load_bias = ELF_PAGESTART(ELF_ET_DYN_BASE - vaddr);
> #endif
> + total_size = total_mapping_size(elf_phdata,
> + loc->elf_ex.e_phnum);
> + if (!total_size) {
> + error = -EINVAL;
I was just printk debugging this function, and this stood out. Should
that be retval instead of error?
Cheers
James
> + goto out_free_dentry;
> + }
> }
>
> error = elf_map(bprm->file, load_bias + vaddr, elf_ppnt,
> - elf_prot, elf_flags, 0);
> + elf_prot, elf_flags, total_size);
> if (BAD_ADDR(error)) {
> retval = IS_ERR((void *)error) ?
> PTR_ERR((void*)error) : -EINVAL;
> --
> 2.2.0.rc0.207.ga3a616c
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
