From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 7DAEDC433FE for ; Sun, 6 Nov 2022 06:44:39 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 5C10F6B0072; Sun, 6 Nov 2022 01:44:38 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 572E76B0073; Sun, 6 Nov 2022 01:44:38 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 439A88E0001; Sun, 6 Nov 2022 01:44:38 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0016.hostedemail.com [216.40.44.16]) by kanga.kvack.org (Postfix) with ESMTP id 3359D6B0072 for ; Sun, 6 Nov 2022 01:44:38 -0500 (EST) Received: from smtpin02.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay02.hostedemail.com (Postfix) with ESMTP id EFB08120687 for ; Sun, 6 Nov 2022 06:44:37 +0000 (UTC) X-FDA: 80102078994.02.FD4E013 Received: from mail-pj1-f45.google.com (mail-pj1-f45.google.com [209.85.216.45]) by imf14.hostedemail.com (Postfix) with ESMTP id 7621B100003 for ; Sun, 6 Nov 2022 06:44:37 +0000 (UTC) Received: by mail-pj1-f45.google.com with SMTP id l6so7927411pjj.0 for ; Sat, 05 Nov 2022 23:44:37 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=albpaZd/W7/PPwS07aGM8eNZ3IHl+SmYaKFhbidGQmI=; b=5IG4pR1+sSn3sA/kSycusa+79aKH7uP5KmONE+G0uLsdt3cEExI4vW3RBY9epynSlz O2twIz39gA5NAGwzsep+XycHMUHVJ/XxsNMCrkjASaF132HmNMIcHT95QHzxlAD+LQ3V fktaD8TmImMjLhlXqy3rdz99ZpvUouPsCMq/twNEFGA7VKVq6vOiQOTS23RYiFpBGILB /+77ktLebGxH9QTA0DD2yIRaPm45aZyfnjFyvG3q0GeIuRayVdeKjZbT6jzKfYaXgMt2 1a1j83uN7/yWXPMPjEjs9KaAJvDdWoNn80uFM2NJAYciloz3cRwHXBDZc5cxknSSbxww GrmA== X-Gm-Message-State: ACrzQf1MuH4wRgjYPx+7+9xHm01XslcRsOiDt/NFVSZPfUSKQnUtVGxi IrR0JIv5/CDHxFJzoUj+sOM= X-Google-Smtp-Source: AMsMyM5nuHxzZeRNTf8GcGTRzfgy/kxEZ90P7tWKYSh2R9Xli2gwkWhkJd4NUEqTAGStd5jeFRaN9Q== X-Received: by 2002:a17:902:8211:b0:172:f722:8402 with SMTP id x17-20020a170902821100b00172f7228402mr602399pln.122.1667717076342; Sat, 05 Nov 2022 23:44:36 -0700 (PDT) Received: from localhost ([2601:647:6300:b760:4158:4a1c:f26e:e91]) by smtp.gmail.com with ESMTPSA id w15-20020a1709026f0f00b00181f8523f60sm2580198plk.225.2022.11.05.23.44.35 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 05 Nov 2022 23:44:35 -0700 (PDT) Date: Sat, 5 Nov 2022 23:44:34 -0700 From: Fangrui Song To: Pedro Falcato Cc: linux-kernel@vger.kernel.org, Kees Cook , linux-mm@kvack.org, sam@gentoo.org, Alexander Viro , Eric Biederman , linux-fsdevel@vger.kernel.org, Rich Felker Subject: Re: [PATCH] fs/binfmt_elf: Fix memsz > filesz handling Message-ID: <20221106064434.zrx5wjyzxtjgc2ly@gmail.com> References: <20221106021657.1145519-1-pedro.falcato@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii; format=flowed Content-Disposition: inline In-Reply-To: <20221106021657.1145519-1-pedro.falcato@gmail.com> ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1667717077; a=rsa-sha256; cv=none; b=wnW2Ij0tqN+mYDuLK/kMKlsblMQeotVyw6AiGz2MTa48LgG04cpzgzxTx1LcRyPWMt8Koj s9iWG4sz12zO+LMnDdeX6HtpOfYkDd7oF9vjcJ++K/VRu8p6eMzS7wyIp/HKBI7EzE1ziK Wlax/n4Kuo78/wqOOOeOSyBFg26iDzo= ARC-Authentication-Results: i=1; imf14.hostedemail.com; dkim=none; spf=pass (imf14.hostedemail.com: domain of emacsray@gmail.com designates 209.85.216.45 as permitted sender) smtp.mailfrom=emacsray@gmail.com; dmarc=none ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1667717077; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=albpaZd/W7/PPwS07aGM8eNZ3IHl+SmYaKFhbidGQmI=; b=bkAZsSe0v6QtV//sAuYYSAucaHhBOcHdwYu086t4+RdW9phclXw99sJcWl4/z7OAqTvEdr CJxaEhyquDEh2Au4REbk8jnFchcx10KHOjXj1gWyIf2DzPf51q4ygXjJbYoB4ZyzVnIdQ/ Qw/r+CJSv09JgkjoP6r0Q8ea8lcr85c= X-Stat-Signature: iwemwc1mxoptkktzbjd4fmyay6mjsdae X-Rspamd-Queue-Id: 7621B100003 Authentication-Results: imf14.hostedemail.com; dkim=none; spf=pass (imf14.hostedemail.com: domain of emacsray@gmail.com designates 209.85.216.45 as permitted sender) smtp.mailfrom=emacsray@gmail.com; dmarc=none X-Rspam-User: X-Rspamd-Server: rspam11 X-HE-Tag: 1667717077-426874 X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On 2022-11-06, Pedro Falcato wrote: >The old code for ELF interpreter loading could only handle >1 memsz > filesz segment. This is incorrect, as evidenced >by the elf program loading code, which could handle multiple >such segments. > >This patch fixes memsz > filesz handling for elf interpreters >and refactors interpreter/program BSS clearing into a common >codepath. > >This bug was uncovered on builds of ppc64le musl libc with >llvm lld 15.0.0, since ppc64 does not allocate file space >for its .plt. > >Cc: Rich Felker >Signed-off-by: Pedro Falcato >--- > fs/binfmt_elf.c | 170 ++++++++++++++++-------------------------------- > 1 file changed, 56 insertions(+), 114 deletions(-) > >diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c >index 6a11025e585..ca2961d80fa 100644 >--- a/fs/binfmt_elf.c >+++ b/fs/binfmt_elf.c >@@ -109,25 +109,6 @@ static struct linux_binfmt elf_format = { > > #define BAD_ADDR(x) (unlikely((unsigned long)(x) >= TASK_SIZE)) > >-static int set_brk(unsigned long start, unsigned long end, int prot) >-{ >- start = ELF_PAGEALIGN(start); >- end = ELF_PAGEALIGN(end); >- if (end > start) { >- /* >- * Map the last of the bss segment. >- * If the header is requesting these pages to be >- * executable, honour that (ppc32 needs this). >- */ >- int error = vm_brk_flags(start, end - start, >- prot & PROT_EXEC ? VM_EXEC : 0); >- if (error) >- return error; >- } >- current->mm->start_brk = current->mm->brk = end; >- return 0; >-} >- > /* We need to explicitly zero any fractional pages > after the data section (i.e. bss). This would > contain the junk from the file that should not >@@ -584,6 +565,41 @@ static inline int make_prot(u32 p_flags, struct arch_elf_state *arch_state, > return arch_elf_adjust_prot(prot, arch_state, has_interp, is_interp); > } > >+static int zero_bss(unsigned long start, unsigned long end, int prot) >+{ >+ /* >+ * First pad the last page from the file up to >+ * the page boundary, and zero it from elf_bss up to the end of the page. >+ */ >+ if (padzero(start)) >+ return -EFAULT; >+ >+ /* >+ * Next, align both the file and mem bss up to the page size, >+ * since this is where elf_bss was just zeroed up to, and where >+ * last_bss will end after the vm_brk_flags() below. >+ */ >+ >+ start = ELF_PAGEALIGN(start); >+ end = ELF_PAGEALIGN(end); >+ >+ /* Finally, if there is still more bss to allocate, do it. */ >+ >+ return (end > start ? vm_brk_flags(start, end - start, >+ prot & PROT_EXEC ? VM_EXEC : 0) : 0); >+} >+ >+static int set_brk(unsigned long start, unsigned long end, int prot) >+{ >+ int error = zero_bss(start, end, prot); >+ >+ if (error < 0) >+ return error; >+ >+ current->mm->start_brk = current->mm->brk = ELF_PAGEALIGN(end); >+ return 0; >+} >+ > /* This is much more generalized than the library routine read function, > so we keep this separate. Technically the library read function > is only provided so that we can read a.out libraries that have >@@ -597,8 +613,6 @@ static unsigned long load_elf_interp(struct elfhdr *interp_elf_ex, > struct elf_phdr *eppnt; > unsigned long load_addr = 0; > int load_addr_set = 0; >- unsigned long last_bss = 0, elf_bss = 0; >- int bss_prot = 0; > unsigned long error = ~0UL; > unsigned long total_size; > int i; >@@ -662,50 +676,21 @@ static unsigned long load_elf_interp(struct elfhdr *interp_elf_ex, > goto out; > } > >- /* >- * Find the end of the file mapping for this phdr, and >- * keep track of the largest address we see for this. >- */ >- k = load_addr + eppnt->p_vaddr + eppnt->p_filesz; >- if (k > elf_bss) >- elf_bss = k; >+ if (eppnt->p_memsz > eppnt->p_filesz) { >+ /* >+ * Handle BSS zeroing and mapping >+ */ >+ unsigned long start = load_addr + vaddr + eppnt->p_filesz; >+ unsigned long end = load_addr + vaddr + eppnt->p_memsz; > >- /* >- * Do the same thing for the memory mapping - between >- * elf_bss and last_bss is the bss section. >- */ >- k = load_addr + eppnt->p_vaddr + eppnt->p_memsz; >- if (k > last_bss) { >- last_bss = k; >- bss_prot = elf_prot; >+ error = zero_bss(start, end, elf_prot); >+ >+ if (error < 0) >+ goto out; > } > } > } > >- /* >- * Now fill out the bss section: first pad the last page from >- * the file up to the page boundary, and zero it from elf_bss >- * up to the end of the page. >- */ >- if (padzero(elf_bss)) { >- error = -EFAULT; >- goto out; >- } >- /* >- * Next, align both the file and mem bss up to the page size, >- * since this is where elf_bss was just zeroed up to, and where >- * last_bss will end after the vm_brk_flags() below. >- */ >- elf_bss = ELF_PAGEALIGN(elf_bss); >- last_bss = ELF_PAGEALIGN(last_bss); >- /* Finally, if there is still more bss to allocate, do it. */ >- if (last_bss > elf_bss) { >- error = vm_brk_flags(elf_bss, last_bss - elf_bss, >- bss_prot & PROT_EXEC ? VM_EXEC : 0); >- if (error) >- goto out; >- } >- > error = load_addr; > out: > return error; >@@ -829,8 +814,6 @@ static int load_elf_binary(struct linux_binprm *bprm) > unsigned long error; > struct elf_phdr *elf_ppnt, *elf_phdata, *interp_elf_phdata = NULL; > struct elf_phdr *elf_property_phdata = NULL; >- unsigned long elf_bss, elf_brk; >- int bss_prot = 0; > int retval, i; > unsigned long elf_entry; > unsigned long e_entry; >@@ -1020,9 +1003,6 @@ static int load_elf_binary(struct linux_binprm *bprm) > executable_stack); > if (retval < 0) > goto out_free_dentry; >- >- elf_bss = 0; >- elf_brk = 0; > > start_code = ~0UL; > end_code = 0; >@@ -1041,33 +1021,6 @@ static int load_elf_binary(struct linux_binprm *bprm) > if (elf_ppnt->p_type != PT_LOAD) > continue; > >- if (unlikely (elf_brk > elf_bss)) { >- unsigned long nbyte; >- >- /* There was a PT_LOAD segment with p_memsz > p_filesz >- before this one. Map anonymous pages, if needed, >- and clear the area. */ >- retval = set_brk(elf_bss + load_bias, >- elf_brk + load_bias, >- bss_prot); >- if (retval) >- goto out_free_dentry; >- nbyte = ELF_PAGEOFFSET(elf_bss); >- if (nbyte) { >- nbyte = ELF_MIN_ALIGN - nbyte; >- if (nbyte > elf_brk - elf_bss) >- nbyte = elf_brk - elf_bss; >- if (clear_user((void __user *)elf_bss + >- load_bias, nbyte)) { >- /* >- * This bss-zeroing can fail if the ELF >- * file specifies odd protections. So >- * we don't check the return value >- */ >- } >- } >- } >- > elf_prot = make_prot(elf_ppnt->p_flags, &arch_state, > !!interpreter, false); > >@@ -1211,41 +1164,30 @@ static int load_elf_binary(struct linux_binprm *bprm) > > k = elf_ppnt->p_vaddr + elf_ppnt->p_filesz; > >- if (k > elf_bss) >- elf_bss = k; >+ >+ if (elf_ppnt->p_memsz > elf_ppnt->p_filesz) { >+ unsigned long seg_end = elf_ppnt->p_vaddr + >+ elf_ppnt->p_memsz + load_bias; >+ retval = set_brk(k + load_bias, >+ seg_end, >+ elf_prot); >+ if (retval) >+ goto out_free_dentry; >+ } >+ > if ((elf_ppnt->p_flags & PF_X) && end_code < k) > end_code = k; > if (end_data < k) > end_data = k; >- k = elf_ppnt->p_vaddr + elf_ppnt->p_memsz; >- if (k > elf_brk) { >- bss_prot = elf_prot; >- elf_brk = k; >- } > } > > e_entry = elf_ex->e_entry + load_bias; > phdr_addr += load_bias; >- elf_bss += load_bias; >- elf_brk += load_bias; > start_code += load_bias; > end_code += load_bias; > start_data += load_bias; > end_data += load_bias; > >- /* Calling set_brk effectively mmaps the pages that we need >- * for the bss and break sections. We must do this before >- * mapping in the interpreter, to make sure it doesn't wind >- * up getting placed where the bss needs to go. >- */ >- retval = set_brk(elf_bss, elf_brk, bss_prot); >- if (retval) >- goto out_free_dentry; >- if (likely(elf_bss != elf_brk) && unlikely(padzero(elf_bss))) { >- retval = -EFAULT; /* Nobody gets to see this, but.. */ >- goto out_free_dentry; >- } >- > if (interpreter) { > elf_entry = load_elf_interp(interp_elf_ex, > interpreter, >-- >2.38.1 I have a write-up about the issue: https://maskray.me/blog/2022-11-05-lld-musl-powerpc64 and used a `.section .toc,"aw",@nobits` trick to verify that this patch makes two RW PT_LOAD (p_filesz < p_memsz) work. Reviewed-by: Fangrui Song Tested-by: Fangrui Song