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 06C28E82CCF for ; Wed, 27 Sep 2023 20:19:05 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 7C1198D0072; Wed, 27 Sep 2023 16:19:04 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 76EF68D001C; Wed, 27 Sep 2023 16:19:04 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 60FE68D0072; Wed, 27 Sep 2023 16:19:04 -0400 (EDT) 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 4AAEA8D001C for ; Wed, 27 Sep 2023 16:19:04 -0400 (EDT) Received: from smtpin10.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay04.hostedemail.com (Postfix) with ESMTP id 1EDD71A019E for ; Wed, 27 Sep 2023 20:19:04 +0000 (UTC) X-FDA: 81283491408.10.46A3775 Received: from out01.mta.xmission.com (out01.mta.xmission.com [166.70.13.231]) by imf06.hostedemail.com (Postfix) with ESMTP id A44BF18001E for ; Wed, 27 Sep 2023 20:19:01 +0000 (UTC) Authentication-Results: imf06.hostedemail.com; dkim=none; dmarc=pass (policy=none) header.from=xmission.com; spf=pass (imf06.hostedemail.com: domain of ebiederm@xmission.com designates 166.70.13.231 as permitted sender) smtp.mailfrom=ebiederm@xmission.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1695845941; 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=Dd+svkvP7KW4QdMMN85lkpl/b2kDbZTV1aowbj2aReY=; b=o0yQDrvvlySH3Dvz1u5Jcrr9wMgLaY26P2nitSkPidNVVr/SbtIOxZ52FvoBmVLsaCeloj j+TZU0S8VYI9oSJF4IkrBG3Rajk/kBRAmjSgX/xAi95DcgqXq7ThOX3yJDYpAoGoGAk0SW cFucaKnndPFZ///ZGK5fCf8bq5kizQE= ARC-Authentication-Results: i=1; imf06.hostedemail.com; dkim=none; dmarc=pass (policy=none) header.from=xmission.com; spf=pass (imf06.hostedemail.com: domain of ebiederm@xmission.com designates 166.70.13.231 as permitted sender) smtp.mailfrom=ebiederm@xmission.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1695845941; a=rsa-sha256; cv=none; b=vezp8x7XXHvdT0gspo8KLcy4lcSbwcCEyP0rD34UGeTLms2zRSTxcPWFDtYaod/1Q5hMEf 13Wm20vWeE+c6xqgVDgJj57qlBnJ+Em1k0ZK5pg0DA8rmYw6hVkImKetKUS4Fkxk1DVgCo ZSi7vIRgIQQbP9S3gifmcogOY1tw0NU= Received: from in01.mta.xmission.com ([166.70.13.51]:46428) by out01.mta.xmission.com with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.93) (envelope-from ) id 1qlazr-005jV1-5R; Wed, 27 Sep 2023 14:18:59 -0600 Received: from ip68-227-168-167.om.om.cox.net ([68.227.168.167]:42922 helo=email.froward.int.ebiederm.org.xmission.com) by in01.mta.xmission.com with esmtpsa (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.93) (envelope-from ) id 1qlazp-005ur2-UD; Wed, 27 Sep 2023 14:18:58 -0600 From: "Eric W. Biederman" To: Kees Cook Cc: Alexander Viro , Christian Brauner , linux-fsdevel@vger.kernel.org, linux-mm@kvack.org, Sebastian Ott , Thomas =?utf-8?Q?Wei=C3=9Fschuh?= , Pedro Falcato , linux-kernel@vger.kernel.org, linux-hardening@vger.kernel.org References: <20230927033634.make.602-kees@kernel.org> <20230927034223.986157-3-keescook@chromium.org> Date: Wed, 27 Sep 2023 15:18:34 -0500 In-Reply-To: <20230927034223.986157-3-keescook@chromium.org> (Kees Cook's message of "Tue, 26 Sep 2023 20:42:20 -0700") Message-ID: <87y1gr8j51.fsf@email.froward.int.ebiederm.org> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/27.1 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-XM-SPF: eid=1qlazp-005ur2-UD;;;mid=<87y1gr8j51.fsf@email.froward.int.ebiederm.org>;;;hst=in01.mta.xmission.com;;;ip=68.227.168.167;;;frm=ebiederm@xmission.com;;;spf=pass X-XM-AID: U2FsdGVkX19RBgBS8g27Lxlq2bg39xuBcUJPOTSRDNI= X-SA-Exim-Connect-IP: 68.227.168.167 X-SA-Exim-Mail-From: ebiederm@xmission.com Subject: Re: [PATCH v3 3/4] binfmt_elf: Provide prot bits as context for padzero() errors X-SA-Exim-Version: 4.2.1 (built Sat, 08 Feb 2020 21:53:50 +0000) X-SA-Exim-Scanned: Yes (on in01.mta.xmission.com) X-Rspamd-Server: rspam09 X-Rspamd-Queue-Id: A44BF18001E X-Stat-Signature: o6qu6hrm9fqt9yybprwh818jhtum6rc6 X-Rspam-User: X-HE-Tag: 1695845941-520525 X-HE-Meta: U2FsdGVkX1/w/xcJec7YV7pfClS2Ijpc+Q26PpQ9RGpMNebz+uH1pcv3wFgqIIW0c6WbXuO8ysRJdB9TGfvnknO23EoTCjsgyhqvszZptlTsQ0bgszS7L/FNQkLf6/CBB56DvXfF5av8akntns8W4uuwqTmk3Rg75YW9zZf2e0lfsRuXw8vmyq6iiCNsHOn+9/R+o2HNN9nsI6Te+dECu9B7226PUwrunXDD96YoM2UL4gP/NyzN83oR6waqtMMnFtWHCjIPwOV1tVxWafJHjlAwsJvqPcByndRZh7Ilz6p8a4DDB9EhmtNmGCQwtuk/LEuq9r+TIOsIMLWygkaikb6J2L+SZHjSS9pskeeAG0Q0ow8sX0qNDvns3BLQpVRAN56rQF/vvYbD3DkmWVQVKCf4hjfioe/edKKNrykQ3YNOJ5P5FrsY3TuMLjV7fxXPhamJmMGMyi0qsKDTbXO5nX2s6tJVhrqUjn7+sJ9bcgLWd8CSSRE+6RipPap4f2imiuCzP3/JK36nc+idS/01BEfRDzc/T/mkR3l2qx2aTwKJN7vtEhez1HtzItqJ3nq2hWkN8TxRHdyoT7uBTypaAZmha39NNf4psZ4+4EYmM9T61TJSLJsPjhW1hOuaZZdsjikqzwxQvnq6Err/5mbIidPNRST6luw54t7OzmYZyirfktV58pExnTnYXu7wgFEuRDqt6iI4GL+GE9xYEyvl3y+W9S2QqbLet37b2M1J4m/OwU8Jn+dMAyQkWA6ntJJP+HnqTDAVmX4OzZuv0XAI2XEJpfQYjRjDkVmhdfuRqfR/NogmADcP8reLHcT9/Mtlcy8C2WcAV9TP50B2owIfXps16SJZ9ftvOLXhGyhNT0CQ4FPAhcRzmhmEmoeYNzRaGWhiFIwrEd8JLDHvbwb51ht/MME766C2WuvaXMQnT9V9Cah/qMt1pqqXKO5lNDNB8NZocvGJLjhAxBJJJqL o1M/ky7H 23bhnH/qS2UkRn/j34OYDLf+Zi76ezb+y3e9jCIuN0+dgGO7nGhy1Vx8s3kx6CXLSdghjKQn6UxtgPDb4xwdNbhhuwT5wCJ9dJ3+YWTJvjt581dh8oD+zH8pSo/NInAnzAdktg7yG7lM+reK7u4AhPHtvEIX9IbKE6fFJC9Rg+xGAUwtUt8vR5rOUKN19t16BdcGWXFHhKPx+59H01qXp97TDnlzccLbw05SjHSz4pPaF//QzEdRZKVNjXvzIJiyBjUJvMyefiB0c/H/5M3SKSRWAnQ3UneovZvUZ34A/WXuFFqPdg+uOjx4fjzLZg0XHSA7fxqikW44670luayEW0eNaD/L8oGoMT8D1qhtuXR+DBps5hXlNNCCKTApX7JDB9E4UTbf5QZyoxReicHQJKXixQ6pdR2Z++fb4XBqT4LDGt0FYUvqusqPNkQ== 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: Kees Cook writes: > Errors with padzero() should be caught unless we're expecting a > pathological (non-writable) segment. Report -EFAULT only when PROT_WRITE > is present. > > Additionally add some more documentation to padzero(), elf_map(), and > elf_load(). I wonder if this might be easier to just perform the PROT_WRITE test in elf_load, and to completely skip padzero of PROT_WRITE is not present. Eric > Cc: Eric Biederman > Cc: Alexander Viro > Cc: Christian Brauner > Cc: linux-fsdevel@vger.kernel.org > Cc: linux-mm@kvack.org > Suggested-by: Eric Biederman > Signed-off-by: Kees Cook > --- > fs/binfmt_elf.c | 33 +++++++++++++++++++++++---------- > 1 file changed, 23 insertions(+), 10 deletions(-) > > diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c > index 0214d5a949fc..b939cfe3215c 100644 > --- a/fs/binfmt_elf.c > +++ b/fs/binfmt_elf.c > @@ -110,19 +110,21 @@ static struct linux_binfmt elf_format = { > > #define BAD_ADDR(x) (unlikely((unsigned long)(x) >= TASK_SIZE)) > > -/* 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 > - be in memory > +/* > + * We need to explicitly zero any trailing portion of the page that follows > + * p_filesz when it ends before the page ends (e.g. bss), otherwise this > + * memory will contain the junk from the file that should not be present. > */ > -static int padzero(unsigned long elf_bss) > +static int padzero(unsigned long address, int prot) > { > unsigned long nbyte; > > - nbyte = ELF_PAGEOFFSET(elf_bss); > + nbyte = ELF_PAGEOFFSET(address); > if (nbyte) { > nbyte = ELF_MIN_ALIGN - nbyte; > - if (clear_user((void __user *) elf_bss, nbyte)) > + /* Only report errors when the segment is writable. */ > + if (clear_user((void __user *)address, nbyte) && > + prot & PROT_WRITE) > return -EFAULT; > } > return 0; > @@ -348,6 +350,11 @@ create_elf_tables(struct linux_binprm *bprm, const struct elfhdr *exec, > return 0; > } > > +/* > + * Map "eppnt->p_filesz" bytes from "filep" offset "eppnt->p_offset" > + * into memory at "addr". (Note that p_filesz is rounded up to the > + * next page, so any extra bytes from the file must be wiped.) > + */ > static unsigned long elf_map(struct file *filep, unsigned long addr, > const struct elf_phdr *eppnt, int prot, int type, > unsigned long total_size) > @@ -387,6 +394,11 @@ static unsigned long elf_map(struct file *filep, unsigned long addr, > return(map_addr); > } > > +/* > + * Map "eppnt->p_filesz" bytes from "filep" offset "eppnt->p_offset" > + * into memory at "addr". Memory from "p_filesz" through "p_memsz" > + * rounded up to the next page is zeroed. > + */ > static unsigned long elf_load(struct file *filep, unsigned long addr, > const struct elf_phdr *eppnt, int prot, int type, > unsigned long total_size) > @@ -405,7 +417,8 @@ static unsigned long elf_load(struct file *filep, unsigned long addr, > eppnt->p_memsz; > > /* Zero the end of the last mapped page */ > - padzero(zero_start); > + if (padzero(zero_start, prot)) > + return -EFAULT; > } > } else { > map_addr = zero_start = ELF_PAGESTART(addr); > @@ -712,7 +725,7 @@ static unsigned long load_elf_interp(struct elfhdr *interp_elf_ex, > * the file up to the page boundary, and zero it from elf_bss > * up to the end of the page. > */ > - if (padzero(elf_bss)) { > + if (padzero(elf_bss, bss_prot)) { > error = -EFAULT; > goto out; > } > @@ -1407,7 +1420,7 @@ static int load_elf_library(struct file *file) > goto out_free_ph; > > elf_bss = eppnt->p_vaddr + eppnt->p_filesz; > - if (padzero(elf_bss)) { > + if (padzero(elf_bss, PROT_WRITE)) { > error = -EFAULT; > goto out_free_ph; > }