From patchwork Wed Sep 27 03:42:18 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Kees Cook X-Patchwork-Id: 13399904 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 vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id B3381E80A80 for ; Wed, 27 Sep 2023 04:35:14 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229837AbjI0EfN (ORCPT ); Wed, 27 Sep 2023 00:35:13 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:41704 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229619AbjI0EeX (ORCPT ); Wed, 27 Sep 2023 00:34:23 -0400 Received: from mail-pl1-x630.google.com (mail-pl1-x630.google.com [IPv6:2607:f8b0:4864:20::630]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id ED7915FF5 for ; Tue, 26 Sep 2023 20:42:25 -0700 (PDT) Received: by mail-pl1-x630.google.com with SMTP id d9443c01a7336-1c3f97f2239so91738445ad.0 for ; Tue, 26 Sep 2023 20:42:25 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; t=1695786145; x=1696390945; darn=vger.kernel.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=q1S2QHXg38kETw2Vrnc3ITAH/+VCqfp/XhwxJe7fVnI=; b=k0E+QxA4OV8E5mylkojffngpwkYwv8qFWaHbI/u9tlv2Eui5TWOn7AGnT2TVhC/Oju D8aSUUT7g7I3iyZNtsHfMNqOQb90FVcHzcDKep/BaUWwF3V8IRyAEo/RgKhxhvvPxzAT aBRvTqi//8cZF9+LD42mrDAUkPuwbZKsRGspY= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1695786145; x=1696390945; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=q1S2QHXg38kETw2Vrnc3ITAH/+VCqfp/XhwxJe7fVnI=; b=PM9qsqmPJqA4HGe0L5vOGbc4LRHjF5W7m27lFCkmXAcKxDziR+qDV18e/y9ZeizXd9 vLzv3AHo0j7pR4ZhoH85u1sIhRnsQqQHZ405xrBHPx4FJTTe6nYpICozLXPjyCqrOI8F ue5JI1c1mioaqOUe7m6/catAzs2tjRDpFzNlR8ovvd7NmGTVB1LUWM6kITa4DgNESID2 r/aVEX4qFXEuQDq+UjDmEgVUnXTlItgFWuFgE8c1noWShGpJOPXmuHSAcSdhCfKDrzMi AcWVz0lrFtQSqTPfRPJ4iRd7HktdUlZYaBjCq6abBMQ91vLc4P518KuWRfDZm+jejsJ0 y3hA== X-Gm-Message-State: AOJu0YxqbeRIY16mYKSY/2OJqUJ/vl4507jnddTV3n/9CWUc7bmsYuL+ pPO+L8SyNKvDWibEi9cd1wr6hQ== X-Google-Smtp-Source: AGHT+IFe7SiLGGQxcesMYUTekhGYP9tx7X00nTSSjxQpTyzG+5pYTZ/MtjR/GtWHi7oYLAWe7SLIfQ== X-Received: by 2002:a17:902:db0a:b0:1c6:2dbb:e5fa with SMTP id m10-20020a170902db0a00b001c62dbbe5famr755412plx.44.1695786145303; Tue, 26 Sep 2023 20:42:25 -0700 (PDT) Received: from www.outflux.net (198-0-35-241-static.hfc.comcastbusiness.net. [198.0.35.241]) by smtp.gmail.com with ESMTPSA id iz19-20020a170902ef9300b001c3bc7b8816sm11900661plb.284.2023.09.26.20.42.24 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 26 Sep 2023 20:42:24 -0700 (PDT) From: Kees Cook To: Eric Biederman Cc: Kees Cook , Sebastian Ott , =?utf-8?q?Thomas_Wei=C3=9Fschuh?= , Al Viro , Christian Brauner , Pedro Falcato , linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org, linux-mm@kvack.org, linux-hardening@vger.kernel.org Subject: [PATCH v3 1/4] binfmt_elf: Support segments with 0 filesz and misaligned starts Date: Tue, 26 Sep 2023 20:42:18 -0700 Message-Id: <20230927034223.986157-1-keescook@chromium.org> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20230927033634.make.602-kees@kernel.org> References: <20230927033634.make.602-kees@kernel.org> MIME-Version: 1.0 X-Developer-Signature: v=1; a=openpgp-sha256; l=8056; i=keescook@chromium.org; h=from:subject; bh=17HFsxzRR6qjNPl2OqeDyONMEClWu0Ld8l5K3CgB/PI=; b=owEBbQKS/ZANAwAKAYly9N/cbcAmAcsmYgBlE6Sd9ky4C5SoI1ZcjBN8kT+abWcWJ1uQcH9Lv 1XdD+58PfSJAjMEAAEKAB0WIQSlw/aPIp3WD3I+bhOJcvTf3G3AJgUCZROknQAKCRCJcvTf3G3A JoKdEACLlARnwXh8gO+l0ringMMre5I4XKiCn1NVJQv/JS/kN/l3wYxfL7OoFGJto2s+6iml7nK peWKblSMEhaNZAO8qfqYUyi/ZpP3RBKGBcBlze989hvAGAq75DOpEdN8PoeowwFXFUbACsQcX8Q OtIJfFZIiP6qNwdHVe2tudNOTtW5glJyjdBoEm8Bs0tLgHa+H8PDBqU2e+SCFQQzYLktWLPY1DQ Rt1+dVHxoom31wssNi01Pm2Fwna3qyPOWL8xB162Mq+0lTRlxBsLJJEfDYn3xdeVPLs3l+5xGYB u+DgPFiv2QGDibshSWyYSvsaCmG5jLBuIbV37VJPE9rnj/DM7N/PHGkV2EMgUzNuHcPtUfr414T jM5KuTByaqkRJia081X3TfoAawDH9SPSRrs8fAKq1ErKhi81NdP2bljkE7VknEsJ4FfLBb0rhts ciyyJj4tpRVZek8NF6AxjqZrIvV0XSK8OO3qntjbUkVGX6p+XCrW8peffqsynyr2eV0D8Io0EQg YZ3nGO5w1nG6RxWJYAP51wZZ74RhpWRfOxDkqOxu7XG345mE0IJgdel6mb87HZhY+i7W/TmcfVl 5rlv5Ll1/Vne3C9HV5g3tHhcOdIzX0C2TpVOSyEa4z4b/2ru9eR7hJdsF7O6potoe4OVwkXAkxr zsu/qJWtB/3bjDQ== X-Developer-Key: i=keescook@chromium.org; a=openpgp; fpr=A5C3F68F229DD60F723E6E138972F4DFDC6DC026 Precedence: bulk List-ID: X-Mailing-List: linux-fsdevel@vger.kernel.org From: "Eric W. Biederman" Implement a helper elf_load() that wraps elf_map() and performs all of the necessary work to ensure that when "memsz > filesz" the bytes described by "memsz > filesz" are zeroed. An outstanding issue is if the first segment has filesz 0, and has a randomized location. But that is the same as today. In this change I replaced an open coded padzero() that did not clear all of the way to the end of the page, with padzero() that does. I also stopped checking the return of padzero() as there is at least one known case where testing for failure is the wrong thing to do. It looks like binfmt_elf_fdpic may have the proper set of tests for when error handling can be safely completed. I found a couple of commits in the old history https://git.kernel.org/pub/scm/linux/kernel/git/tglx/history.git, that look very interesting in understanding this code. commit 39b56d902bf3 ("[PATCH] binfmt_elf: clearing bss may fail") commit c6e2227e4a3e ("[SPARC64]: Missing user access return value checks in fs/binfmt_elf.c and fs/compat.c") commit 5bf3be033f50 ("v2.4.10.1 -> v2.4.10.2") Looking at commit 39b56d902bf3 ("[PATCH] binfmt_elf: clearing bss may fail"): > commit 39b56d902bf35241e7cba6cc30b828ed937175ad > Author: Pavel Machek > Date: Wed Feb 9 22:40:30 2005 -0800 > > [PATCH] binfmt_elf: clearing bss may fail > > So we discover that Borland's Kylix application builder emits weird elf > files which describe a non-writeable bss segment. > > So remove the clear_user() check at the place where we zero out the bss. I > don't _think_ there are any security implications here (plus we've never > checked that clear_user() return value, so whoops if it is a problem). > > Signed-off-by: Pavel Machek > Signed-off-by: Andrew Morton > Signed-off-by: Linus Torvalds It seems pretty clear that binfmt_elf_fdpic with skipping clear_user() for non-writable segments and otherwise calling clear_user(), aka padzero(), and checking it's return code is the right thing to do. I just skipped the error checking as that avoids breaking things. And notably, it looks like Borland's Kylix died in 2005 so it might be safe to just consider read-only segments with memsz > filesz an error. Reported-by: Sebastian Ott Reported-by: Thomas Weißschuh Closes: https://lkml.kernel.org/r/20230914-bss-alloc-v1-1-78de67d2c6dd@weissschuh.net Signed-off-by: "Eric W. Biederman" Link: https://lore.kernel.org/r/87sf71f123.fsf@email.froward.int.ebiederm.org Signed-off-by: Kees Cook --- fs/binfmt_elf.c | 111 +++++++++++++++++++++--------------------------- 1 file changed, 48 insertions(+), 63 deletions(-) diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c index 7b3d2d491407..2a615f476e44 100644 --- a/fs/binfmt_elf.c +++ b/fs/binfmt_elf.c @@ -110,25 +110,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 @@ -406,6 +387,51 @@ static unsigned long elf_map(struct file *filep, unsigned long addr, return(map_addr); } +static unsigned long elf_load(struct file *filep, unsigned long addr, + const struct elf_phdr *eppnt, int prot, int type, + unsigned long total_size) +{ + unsigned long zero_start, zero_end; + unsigned long map_addr; + + if (eppnt->p_filesz) { + map_addr = elf_map(filep, addr, eppnt, prot, type, total_size); + if (BAD_ADDR(map_addr)) + return map_addr; + if (eppnt->p_memsz > eppnt->p_filesz) { + zero_start = map_addr + ELF_PAGEOFFSET(eppnt->p_vaddr) + + eppnt->p_filesz; + zero_end = map_addr + ELF_PAGEOFFSET(eppnt->p_vaddr) + + eppnt->p_memsz; + + /* Zero the end of the last mapped page */ + padzero(zero_start); + } + } else { + map_addr = zero_start = ELF_PAGESTART(addr); + zero_end = zero_start + ELF_PAGEOFFSET(eppnt->p_vaddr) + + eppnt->p_memsz; + } + if (eppnt->p_memsz > eppnt->p_filesz) { + /* + * Map the last of the segment. + * If the header is requesting these pages to be + * executable, honour that (ppc32 needs this). + */ + int error; + + zero_start = ELF_PAGEALIGN(zero_start); + zero_end = ELF_PAGEALIGN(zero_end); + + error = vm_brk_flags(zero_start, zero_end - zero_start, + prot & PROT_EXEC ? VM_EXEC : 0); + if (error) + map_addr = error; + } + return map_addr; +} + + static unsigned long total_mapping_size(const struct elf_phdr *phdr, int nr) { elf_addr_t min_addr = -1; @@ -829,7 +855,6 @@ static int load_elf_binary(struct linux_binprm *bprm) 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; @@ -1040,33 +1065,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); @@ -1162,7 +1160,7 @@ static int load_elf_binary(struct linux_binprm *bprm) } } - error = elf_map(bprm->file, load_bias + vaddr, elf_ppnt, + error = elf_load(bprm->file, load_bias + vaddr, elf_ppnt, elf_prot, elf_flags, total_size); if (BAD_ADDR(error)) { retval = IS_ERR_VALUE(error) ? @@ -1217,10 +1215,8 @@ static int load_elf_binary(struct linux_binprm *bprm) if (end_data < k) end_data = k; k = elf_ppnt->p_vaddr + elf_ppnt->p_memsz; - if (k > elf_brk) { - bss_prot = elf_prot; + if (k > elf_brk) elf_brk = k; - } } e_entry = elf_ex->e_entry + load_bias; @@ -1232,18 +1228,7 @@ static int load_elf_binary(struct linux_binprm *bprm) 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; - } + current->mm->start_brk = current->mm->brk = ELF_PAGEALIGN(elf_brk); if (interpreter) { elf_entry = load_elf_interp(interp_elf_ex, From patchwork Wed Sep 27 03:42:19 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kees Cook X-Patchwork-Id: 13399888 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 vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id B9F68E7E62A for ; Wed, 27 Sep 2023 04:24:10 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229691AbjI0EYK (ORCPT ); Wed, 27 Sep 2023 00:24:10 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:53188 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229694AbjI0EXJ (ORCPT ); Wed, 27 Sep 2023 00:23:09 -0400 Received: from mail-pl1-x633.google.com (mail-pl1-x633.google.com [IPv6:2607:f8b0:4864:20::633]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 9CA596E8A for ; Tue, 26 Sep 2023 20:42:27 -0700 (PDT) Received: by mail-pl1-x633.google.com with SMTP id d9443c01a7336-1c3d8fb23d9so73314345ad.0 for ; Tue, 26 Sep 2023 20:42:27 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; t=1695786147; x=1696390947; darn=vger.kernel.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=oGhwDBPbCWZ00dYqQrvOzcYwuI2lZJAyT/mcJzKB6R4=; b=Gn8gM/3yYje83klxRkXj59yeqOCnHVvIfl5xp35mK1+zd4V5d3CWCJm4tEEz9YTVKL VNhqkn4Tq8YGWWoYhX7OZARQbCrLzrJyZr1NOwyRrjiKeHW62gkqrK3gE+PFxXXJve2N wxm8+6oDs5Uh/VRqIIlMzadsY06M/mBMkxju0= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1695786147; x=1696390947; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=oGhwDBPbCWZ00dYqQrvOzcYwuI2lZJAyT/mcJzKB6R4=; b=RWJdgwL0EXiLZdOsLI2WgWvCqXMqsbGF62F0cKPG6CYHKru+Pk8c940cWQJtydjgWw /6jX2SVVC+HKy7JdQ9gkJaGhZqWzn+ICHtCpazxk34ctmAUuUj2s3ns/1YOrIUc33q5E swzl/sZwhb9x3HjxW3fnl9R24DByppBEYJAgM52QpAwJzdJBTqmQiaud0/bkNLKO0E9a vB5EK93HbWJPsoT94SotJ/CYhEVNUCC2kh87lT4dftkRLASNlnLJguqd7B1lHUYuWTct GZxlLAgVrjO5tdrUOHG9xqd8t4owbnWEBoYOMq9r/sn+LP4/Sph4hwBVmKNEW17whFZ8 ubhA== X-Gm-Message-State: AOJu0YwaQXN7zdyL8EqEwBbesCLS7F7QSrae/WBdq2oza1+XnGjS1ISy svAG2PYSe1KGYMh0vBWtopqSjg== X-Google-Smtp-Source: AGHT+IEMRh07dxmRBvxAFfE5SwpELfOldF0XUoKYTyABrKV6bOgIMJW1wNCR/iQbnr++EfKTyUevcg== X-Received: by 2002:a17:902:e74f:b0:1c6:112f:5d02 with SMTP id p15-20020a170902e74f00b001c6112f5d02mr741873plf.55.1695786147186; Tue, 26 Sep 2023 20:42:27 -0700 (PDT) Received: from www.outflux.net (198-0-35-241-static.hfc.comcastbusiness.net. [198.0.35.241]) by smtp.gmail.com with ESMTPSA id v7-20020a170902b7c700b001c61073b064sm5974160plz.69.2023.09.26.20.42.24 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 26 Sep 2023 20:42:24 -0700 (PDT) From: Kees Cook To: Eric Biederman Cc: Kees Cook , Alexander Viro , Christian Brauner , linux-fsdevel@vger.kernel.org, linux-mm@kvack.org, Sebastian Ott , =?utf-8?q?Thomas_Wei?= =?utf-8?q?=C3=9Fschuh?= , Pedro Falcato , linux-kernel@vger.kernel.org, linux-hardening@vger.kernel.org Subject: [PATCH v3 2/4] binfmt_elf: elf_bss no longer used by load_elf_binary() Date: Tue, 26 Sep 2023 20:42:19 -0700 Message-Id: <20230927034223.986157-2-keescook@chromium.org> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20230927033634.make.602-kees@kernel.org> References: <20230927033634.make.602-kees@kernel.org> MIME-Version: 1.0 X-Developer-Signature: v=1; a=openpgp-sha256; l=1738; i=keescook@chromium.org; h=from:subject; bh=G8gKivjP+bPsGdacm75CduzZVarN6sAt0dfrtCVUjtY=; b=owEBbQKS/ZANAwAKAYly9N/cbcAmAcsmYgBlE6Sd2BuLhX7iIAkgIu+LW3P3r4pxnZlwRaDRf vA/0AICBj2JAjMEAAEKAB0WIQSlw/aPIp3WD3I+bhOJcvTf3G3AJgUCZROknQAKCRCJcvTf3G3A JoZiEACxnnqWBzMIIDewc78beSt5xMPBmGmN6i2luWvAqAMFgVuNI0nDLE57tnMWit7yDj2XgJ1 2Z/pa68XIcntr/l4Ox2lUHBpmeW82kMxXbte18Ecfkifetz5bSTNhT4eqz9iInkevtzRWO3uUpq i3Sjs/gANX1ErFrzlbJdGorFoQ2WChDbSXKcGCifcF+7ByszsUMc7ciRkTfJ+c69ymHfkIjFnB4 NABq4TtLUWJgQ4fWWHfAXP7ibJqTV+vue/6haJy4EBtrPblhUpyXmaPHPlLFqfB4NWp67PspCGn Xt+4d8whIqgJJO12woWoD2kad16SwfEWwy6rtb5glKxP4P8lwFkwUY2npYvmLb7oLp/JOBGWtT4 zN8stmcyYoHO5JH1fxDZ/M4IbnvhDjIyT/v4bcz2Sn/t26aRdaLx2dvl1fz3GLavqQm1IhRhQ+f XxYEJIe0C5lZ5EdXJrERty9WadxoFIpFZuQtqKSIfGNDtWQIQ1fwZPbObiOfsYvT6LcbQLKMUG1 /xXJ+v8+u3jW+HTFnzZ+EE7ig4RRSprVCTd5QEzX4RbC8KXboi2yMozBdmUhOTOlkUJSU/uUZ1+ VZHcMo6rE0t3wu84BkMjvDVBAPXSGtNLDJ9xoujVrtMltIzQDhdxPz2ce74LXQ2WEx1wn991e8y t1zbNpVW5qc63Kw== X-Developer-Key: i=keescook@chromium.org; a=openpgp; fpr=A5C3F68F229DD60F723E6E138972F4DFDC6DC026 Precedence: bulk List-ID: X-Mailing-List: linux-fsdevel@vger.kernel.org With the BSS handled generically via the new filesz/memsz mismatch handling logic in elf_load(), elf_bss no longer needs to be tracked. Drop the variable. 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 | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c index 2a615f476e44..0214d5a949fc 100644 --- a/fs/binfmt_elf.c +++ b/fs/binfmt_elf.c @@ -854,7 +854,7 @@ 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; + unsigned long elf_brk; int retval, i; unsigned long elf_entry; unsigned long e_entry; @@ -1045,7 +1045,6 @@ static int load_elf_binary(struct linux_binprm *bprm) if (retval < 0) goto out_free_dentry; - elf_bss = 0; elf_brk = 0; start_code = ~0UL; @@ -1208,8 +1207,6 @@ 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_flags & PF_X) && end_code < k) end_code = k; if (end_data < k) @@ -1221,7 +1218,6 @@ static int load_elf_binary(struct linux_binprm *bprm) 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; From patchwork Wed Sep 27 03:42:20 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kees Cook X-Patchwork-Id: 13399886 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 vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id D245DE7E62A for ; Wed, 27 Sep 2023 04:20:43 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229791AbjI0EUl (ORCPT ); Wed, 27 Sep 2023 00:20:41 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:53892 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229616AbjI0ETb (ORCPT ); Wed, 27 Sep 2023 00:19:31 -0400 Received: from mail-pf1-x435.google.com (mail-pf1-x435.google.com [IPv6:2607:f8b0:4864:20::435]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 8EFE35FF3 for ; Tue, 26 Sep 2023 20:42:25 -0700 (PDT) Received: by mail-pf1-x435.google.com with SMTP id d2e1a72fcca58-690bf8fdd1aso8066245b3a.2 for ; Tue, 26 Sep 2023 20:42:25 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; t=1695786145; x=1696390945; darn=vger.kernel.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=fmhcOt0Phh+K4fShkXGPXzhG2+0EGgvMOT3Anhb5JtI=; b=ZYsp9X2L/K4sq6nHJ/InBMtpPeI6AC7q+P6owshAAH8QJH6wLbLAYABxQO17n6vIwP FjMdtDMz2/Gjw9Jtf6KVV/kwTK/UXRDxjd8c4mLhhOPLs53oDqJGPV/oEuNWV+BSxbph ZABsEBD/b4uQDrO7SOdPnc9aHU6r4MoyJjsYY= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1695786145; x=1696390945; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=fmhcOt0Phh+K4fShkXGPXzhG2+0EGgvMOT3Anhb5JtI=; b=JCtjvEM+uFPjqMTUQIHI9FmSkJJSy8zXyU0fTwW0kIIDQvu7AbmQc/dLnEN28Ru+NN eVCHj0IEOM+b/ITLojxMxWUXIVx1SmvJSUvECfFU43Fx/Wa4uaFKgCvWMEeyTf72nv0L tKVfo/emcTj3d6grxRtW+tnd9yQ0iL8KFzolqVddrbWPOPIFDt972ZBQFi4cs8KB62ah FZhWZd6bc9JV2r1yQ61cQy+XnDJIYK29dgOWYKbxx4Fd2IbfgHlHOL40IE/kjp5RkBDb 4CbQ679vAwbr0a6PIm1nf9IDlAnq0Sf1HmTvTtwc9pjM6mo+c4vtxv55/ILJtitVBHud mdmA== X-Gm-Message-State: AOJu0YypAScjO1i1TEL3jMgQD5hd87chpLAX5HDz/V0hMWMiOp0NS+vG 3nJjJMdvvGN705zs1GViiLcqhA== X-Google-Smtp-Source: AGHT+IEnkEHkAMyfzsQkaO55Avyrk0ulpfRxCywQvRCp4n7UkR+naKdiK+KW8D+X2wQeWkwJJchcfQ== X-Received: by 2002:a05:6a00:cc2:b0:690:ca4e:661b with SMTP id b2-20020a056a000cc200b00690ca4e661bmr1032600pfv.13.1695786144968; Tue, 26 Sep 2023 20:42:24 -0700 (PDT) Received: from www.outflux.net (198-0-35-241-static.hfc.comcastbusiness.net. [198.0.35.241]) by smtp.gmail.com with ESMTPSA id i13-20020aa78b4d000000b00688965c5227sm3826941pfd.120.2023.09.26.20.42.24 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 26 Sep 2023 20:42:24 -0700 (PDT) From: Kees Cook To: Eric Biederman Cc: Kees Cook , Alexander Viro , Christian Brauner , linux-fsdevel@vger.kernel.org, linux-mm@kvack.org, Sebastian Ott , =?utf-8?q?Thomas_Wei?= =?utf-8?q?=C3=9Fschuh?= , Pedro Falcato , linux-kernel@vger.kernel.org, linux-hardening@vger.kernel.org Subject: [PATCH v3 3/4] binfmt_elf: Provide prot bits as context for padzero() errors Date: Tue, 26 Sep 2023 20:42:20 -0700 Message-Id: <20230927034223.986157-3-keescook@chromium.org> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20230927033634.make.602-kees@kernel.org> References: <20230927033634.make.602-kees@kernel.org> MIME-Version: 1.0 X-Developer-Signature: v=1; a=openpgp-sha256; l=3658; i=keescook@chromium.org; h=from:subject; bh=wBfTuibkI1ZafUlyw4M9Ln83egyXuZyKwCYBlE+Ln0Q=; b=owEBbQKS/ZANAwAKAYly9N/cbcAmAcsmYgBlE6SdGv8DDqtwm5Ol0LgrgyjZaP+4ZO6Wq6Zvc ewLHcLJGtKJAjMEAAEKAB0WIQSlw/aPIp3WD3I+bhOJcvTf3G3AJgUCZROknQAKCRCJcvTf3G3A JgOCD/9Yn5Qu3vikEH/yBMhn9z8VzLRLFdyRg9vmKyaQg1Y1oUqxhPpmdoBiT9lo4FEZcAz3Pbo slZs7xhhxScfjwwayUC/3eCa2sGC4pDlomjzWuLKZx+rhUKBTXLGk57l/T030w/BTGRk8OApPaS 19sBIkkgXFNfHvlcgw1zzeX4/JPF7yvsr936vFS/ypc4nuzEzdrRoMKxuTMgP+eVrsgNAri296j DeMk8JiLE+PzpbV1vRrhK/nP6G5m/OdH6OVZP3UIOq0+M1GnvJS0F1LVRL9T1aolQyan5mAn1/3 Tt2f5wsLeGNsy09fY4++KWw22oYo6YHGAYfwbp93cdnjtqCniroSVvEYPNiPdsd6OXtZm8O2ERQ IhyT1RCKLWjAXpK6Ow8yZYZwS2FmI0TRhU8BgygE3Qh5XkzdsjY0oi+QUkh3s0ktiqKyZxXrW40 k6zgbv7iGb6zTzbkqeecY4Tju47/elEUbwqNCxRnPcmd3SfCF0jwKWVwGGxUcBRaWdao36it2H0 09ADuzAuiBrMa1Z24c5C99+xO7kPruaySYYC7DV703FB3q5+qWFThooJ4gzw8qx5HyOZhtzvQhz zyn1WsqYfrF04pj19ZYwP3oimZ2OctqtY4fYgdF0pZadU5BzKZlwRvxwrEMy713xxETHnY6nu8b rYdT6wNKfO8hgTw== X-Developer-Key: i=keescook@chromium.org; a=openpgp; fpr=A5C3F68F229DD60F723E6E138972F4DFDC6DC026 Precedence: bulk List-ID: X-Mailing-List: linux-fsdevel@vger.kernel.org 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(). 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; } From patchwork Wed Sep 27 03:42:21 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kees Cook X-Patchwork-Id: 13399922 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 vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 89FC0E80A82 for ; Wed, 27 Sep 2023 04:49:38 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229800AbjI0Etg (ORCPT ); Wed, 27 Sep 2023 00:49:36 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:39646 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229520AbjI0Esw (ORCPT ); Wed, 27 Sep 2023 00:48:52 -0400 Received: from mail-pf1-x433.google.com (mail-pf1-x433.google.com [IPv6:2607:f8b0:4864:20::433]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 598CF4237 for ; Tue, 26 Sep 2023 20:42:25 -0700 (PDT) Received: by mail-pf1-x433.google.com with SMTP id d2e1a72fcca58-6907e44665bso9118734b3a.1 for ; Tue, 26 Sep 2023 20:42:25 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; t=1695786145; x=1696390945; darn=vger.kernel.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=psNe55NiIGmm2xWNk7Y7WSenoVYReQWIa41A35qkMTo=; b=DMgt7E8zbjdDxpmCg9lVKkaaMN5PJ8SCsJBHXx4/bPe0fcnr4RiVcettQb2AyiHLJt WRYfU4rkd/G3iLCuExCN+SdDhRs6JghrEnjQNd6OpCYRqQr8S4xGdKG/Uyo1piDLPEN4 IKGkUT2o5cda7EDvkdd5tYH44m2IAQYre1fi8= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1695786145; x=1696390945; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=psNe55NiIGmm2xWNk7Y7WSenoVYReQWIa41A35qkMTo=; b=TBYtQxctxiFGxnU1DUeU55QgSqqQ+B0tT0HkdrBjjzLX7JQvVapXdbT/z4NaEYPO9F wAnd1T7iBtT+oOLXKnVZnzzSX9RQvQ28sPLyCxWWVTcTbnMU+npjwyyeb0LlthR2+W7F 3ThnPEm+QdS4Q14ja+sMioF4dcCJ2EHFkogABeTJjgpbVt26y6IscMMpNyj1Mgkw820u NXB7FI3FmvG5ERXyiwbVwONS2yH1/JiyGajiZOpQj8tSo/7Psfdfku9Ca+MGWRsO4qKj 0YQe7O4PICI1VTtFU5FR+K4sXHqc7K5aiG/Zct8gvS2+BmP+v7iHZpfGq0t3kYIKB6Ev N6cg== X-Gm-Message-State: AOJu0YxEWQP8qIVhfBZv99JqD8izZKJ42y9HMXaytxnQ3PZlgUBxt9Z2 AoDDpfl6jIaghR7svDFhNfvtIg== X-Google-Smtp-Source: AGHT+IH5EAvrsjyvsdjMBX+hRb48/9Odz2jhGyduyLfeui8bYT0jgwoq8H4vxWQ8plZNKiKb1c39bg== X-Received: by 2002:a05:6a00:1745:b0:690:3b59:cc7a with SMTP id j5-20020a056a00174500b006903b59cc7amr1223964pfc.23.1695786144630; Tue, 26 Sep 2023 20:42:24 -0700 (PDT) Received: from www.outflux.net (198-0-35-241-static.hfc.comcastbusiness.net. [198.0.35.241]) by smtp.gmail.com with ESMTPSA id l21-20020a62be15000000b0068fe5a5a566sm11050779pff.142.2023.09.26.20.42.24 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 26 Sep 2023 20:42:24 -0700 (PDT) From: Kees Cook To: Eric Biederman Cc: Kees Cook , Alexander Viro , Christian Brauner , linux-fsdevel@vger.kernel.org, linux-mm@kvack.org, Pedro Falcato , Sebastian Ott , =?utf-8?q?Thomas_Wei=C3=9Fschuh?= , linux-kernel@vger.kernel.org, linux-hardening@vger.kernel.org Subject: [PATCH v3 4/4] binfmt_elf: Use elf_load() for interpreter Date: Tue, 26 Sep 2023 20:42:21 -0700 Message-Id: <20230927034223.986157-4-keescook@chromium.org> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20230927033634.make.602-kees@kernel.org> References: <20230927033634.make.602-kees@kernel.org> MIME-Version: 1.0 X-Developer-Signature: v=1; a=openpgp-sha256; l=2946; i=keescook@chromium.org; h=from:subject; bh=ESjAlwugvTKH54m807EA0V7P1zfqWIUst+9ebRisI1g=; b=owEBbQKS/ZANAwAKAYly9N/cbcAmAcsmYgBlE6SdTC81Lo/IK1HXDqbNXd8+1ZQdNAkLqgYld ib+qt6Vsy+JAjMEAAEKAB0WIQSlw/aPIp3WD3I+bhOJcvTf3G3AJgUCZROknQAKCRCJcvTf3G3A JtadEACKjgl6qtJqAh0AMdgrhwl3xNt5d2qNuR5potUxETk/BXU7u2GTXYtP2F9RDpOrkDiKZgs kHaZ+K0S58tXoM1EESrVf9DgwoH2Yiu3a5Wnx/9cSLQ39gLchJc9iacJBevN5PWRNSQGhPINVHD ppnkc1dc2MgRLHRWccbGhLJoWY9ypb0+DB2+c326VYEE9A6ZcH0XF2mBBGZWXFdJf5FqvPjjP7d 3aH1zRNKNI0RS4QKXR0yRxvLTkQu1CHaVV2EUyc2z8A2QyuQlLPUE/1Sndgz+rJf4zaBAGoUu15 xtdsaw1/9QllvwqXSugoOkXpIIDx4n8QOYsgxRkRVM1a+KkJy54IQyjYj7yhw+IlJhX1NVUKiwd 5TyNEMzi/OHgIlQ0Zx61x8PDB9FHITdRrPmmhjgmSMl5lVf/iutiyYsdRdW/udNxhJwecomGd+V FPKrzpAU5XCcgfQavEm4SC012nzVWQXMKBChSTYtRcchUfBI8yKMx0vBF3AUzEGnUxkenFTBZCX 501e3882lR4jnNHar/3lok+SU91dFINJ79VHpEjsURQH5gylPvdbtX+i1nqijpI8fY16XKQPVsO A9WZlk3iYIvwmclIZazzIknfX61fR/qhvIhhre1iOr1xSWFFYOOhAzzXr+/bT6hmLGWrqrb36DM i6jNABcOoxu5qhA== X-Developer-Key: i=keescook@chromium.org; a=openpgp; fpr=A5C3F68F229DD60F723E6E138972F4DFDC6DC026 Precedence: bulk List-ID: X-Mailing-List: linux-fsdevel@vger.kernel.org Handle arbitrary memsz>filesz in interpreter ELF segments, instead of only supporting it in the last segment (which is expected to be the BSS). Cc: Eric Biederman Cc: Alexander Viro Cc: Christian Brauner Cc: linux-fsdevel@vger.kernel.org Cc: linux-mm@kvack.org Reported-by: Pedro Falcato Closes: https://lore.kernel.org/lkml/20221106021657.1145519-1-pedro.falcato@gmail.com/ Signed-off-by: Kees Cook --- fs/binfmt_elf.c | 46 +--------------------------------------------- 1 file changed, 1 insertion(+), 45 deletions(-) diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c index b939cfe3215c..74af5c8319a0 100644 --- a/fs/binfmt_elf.c +++ b/fs/binfmt_elf.c @@ -635,8 +635,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; @@ -673,7 +671,7 @@ static unsigned long load_elf_interp(struct elfhdr *interp_elf_ex, else if (no_base && interp_elf_ex->e_type == ET_DYN) load_addr = -vaddr; - map_addr = elf_map(interpreter, load_addr + vaddr, + map_addr = elf_load(interpreter, load_addr + vaddr, eppnt, elf_prot, elf_type, total_size); total_size = 0; error = map_addr; @@ -699,51 +697,9 @@ static unsigned long load_elf_interp(struct elfhdr *interp_elf_ex, error = -ENOMEM; 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; - - /* - * 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; - } } } - /* - * 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, bss_prot)) { - 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;