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 bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 2BB7AC433EF for ; Tue, 8 Mar 2022 00:02:30 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=jCXY2nlzh07S5a5euGhV5SX1UrccTRaz/7beLKV1Q5Q=; b=eEsfI0hh5UsWbR 7MnCHXP9WGKvzFLARAelsBh98QUIU5LVWZZ8iZC43eV6TAi3JG3xRGtfLH2+BKTPdlFx6qETpCxY1 RLhjPtgNKcT0zXLlrVa4Qb82UJE0FipIByolat8qNNygz79m5up4VK1Outuk/wK2E5VhPGHlQxSLx kHfhPOEXaUqhLEY22hZxtTmrxYoDn+xP97hnjvTboZpmCnOhYVtXigP4oJc1YWeo42YRXfvTD2uUL GTpKc0LaHNS6zIAIuCibG/MMxdE6bdmWCGhta4pUKbpvroBmywOhY49dRFEd9IK2dNuIDplWGB4/B Lyn45EP5vz8MhRBk1Kiw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1nRNH2-00219Z-V8; Tue, 08 Mar 2022 00:00:21 +0000 Received: from mail-pf1-x42e.google.com ([2607:f8b0:4864:20::42e]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1nRNH0-00218f-1K for linux-arm-kernel@lists.infradead.org; Tue, 08 Mar 2022 00:00:19 +0000 Received: by mail-pf1-x42e.google.com with SMTP id s8so11755471pfk.12 for ; Mon, 07 Mar 2022 16:00:17 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=CSChNH+lUdAbaPTUG1zJh3zIL8Yt/GuhG02ImIJkwsA=; b=hJ6HSu1DtgxnnOolcgDpDd+f9cHxxGgMPxupO96VRQXTbydy7fPYwJM6r0kvkLQrVS 6i0415EqzFGhhyo7ZtUfXJQ3xEVwvjVMBh1QVWE2gZ/vFwbrYKQ8JglhynXwiotCnJWn 7Rgv09FEOcqiw1iH3AUiPwsC9K9T+Q4Vc1DYw= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=CSChNH+lUdAbaPTUG1zJh3zIL8Yt/GuhG02ImIJkwsA=; b=S8EMHMIYqttXXkPkAqr0Igi+o/z+1hOAVgVsVhKYPK5P5xONkGyIwTJ7dLbl10MLH7 TCOgPKo6HffqEzPV44zFtuo2Zy4VhOgJ9eA3wvULJZVVD5r5O51P92lWcHKPL6CkIEGL Qibm4JK2G7rPHfzm6Z5sDzOTzaOREWlYspqOpC1JtU2emVEl1O1jEWAuF4UV1V68pScl nO+f8kNGj+8b2JRbK+/fyuGlsDFYLeSCOEcyPPgn8iJmApM/Lr77hfmoyC881/mHIB1v eGOh/5xsCVj0RbZYFejOaa1ZT6VEjjDkpNA1R7CzlxZp0qTxy0JXIm1gA48t5Xq/MX3d SYSA== X-Gm-Message-State: AOAM5329/E6CSi53VBFSHzucpVqhqSPQaEhATejVNq/b4/0M2Yllcc74 4IVMdfNthm3NuvF5DfkeroJM2Q== X-Google-Smtp-Source: ABdhPJxDSuiVE8cbbycz0Ergwkg83bRoKMz9OjxmT3mGTQV67u5NSLYDiOHNckRjeKg+omngneNRog== X-Received: by 2002:a65:610e:0:b0:378:d43b:9703 with SMTP id z14-20020a65610e000000b00378d43b9703mr11685997pgu.229.1646697616613; Mon, 07 Mar 2022 16:00:16 -0800 (PST) Received: from www.outflux.net (smtp.outflux.net. [198.145.64.163]) by smtp.gmail.com with ESMTPSA id u18-20020a056a00159200b004f708ecd48esm4742514pfk.149.2022.03.07.16.00.16 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 07 Mar 2022 16:00:16 -0800 (PST) Date: Mon, 7 Mar 2022 16:00:15 -0800 From: Kees Cook To: Mark Brown Cc: Catalin Marinas , Will Deacon , Szabolcs Nagy , Jeremy Linton , "H . J . Lu" , Yu-cheng Yu , Eric Biederman , linux-arch@vger.kernel.org, linux-arm-kernel@lists.infradead.org, libc-alpha@sourceware.org, Dave Martin Subject: Re: [PATCH v10 1/2] elf: Allow architectures to parse properties on the main executable Message-ID: <202203071551.DBABE01@keescook> References: <20220228130606.1070960-1-broonie@kernel.org> <20220228130606.1070960-2-broonie@kernel.org> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20220228130606.1070960-2-broonie@kernel.org> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20220307_160018_123428_40837FE8 X-CRM114-Status: GOOD ( 33.70 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Mon, Feb 28, 2022 at 01:06:05PM +0000, Mark Brown wrote: > Currently the ELF code only attempts to parse properties on the image > that will start execution, either the interpreter or for statically linked > executables the main executable. The expectation is that any property > handling for the main executable will be done by the interpreter. This is > a bit inconsistent since we do map the executable and is causing problems > for the arm64 BTI support when used in conjunction with systemd's use of > seccomp to implement MemoryDenyWriteExecute which stops the dynamic linker > adjusting the permissions of executable segments. > > Allow architectures to handle properties for both the dynamic linker and > main executable, adjusting arch_parse_elf_properties() to have a new > flag is_interp flag as with arch_elf_adjust_prot() and calling it for > both the main executable and any intepreter. > > The user of this code, arm64, is adapted to ensure that there is no > functional change. > > Signed-off-by: Mark Brown > Tested-by: Jeremy Linton > Reviewed-by: Dave Martin > Reviewed-by: Catalin Marinas > --- > arch/arm64/include/asm/elf.h | 3 ++- > fs/binfmt_elf.c | 32 +++++++++++++++++++++++--------- > include/linux/elf.h | 4 +++- > 3 files changed, 28 insertions(+), 11 deletions(-) > > diff --git a/arch/arm64/include/asm/elf.h b/arch/arm64/include/asm/elf.h > index 97932fbf973d..5cc002376abe 100644 > --- a/arch/arm64/include/asm/elf.h > +++ b/arch/arm64/include/asm/elf.h > @@ -259,6 +259,7 @@ struct arch_elf_state { > > static inline int arch_parse_elf_property(u32 type, const void *data, > size_t datasz, bool compat, > + bool has_interp, bool is_interp, > struct arch_elf_state *arch) Adding more and more args to a functions like this gives me the sense that some kind of argument structure is needed. Once I get enough unit testing written in here, I'm hoping to refactor a bunch of this. To the future! :) > @@ -828,6 +832,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; > + struct elf_phdr *interp_elf_property_phdata = NULL; > unsigned long elf_bss, elf_brk; > int bss_prot = 0; > int retval, i; > @@ -865,6 +870,9 @@ static int load_elf_binary(struct linux_binprm *bprm) > for (i = 0; i < elf_ex->e_phnum; i++, elf_ppnt++) { > char *elf_interpreter; > > + if (interpreter && elf_property_phdata) > + break; > + This is not okay. This introduces a memory resource leak for malicious ELF files with multiple INTERP headers. > if (elf_ppnt->p_type == PT_GNU_PROPERTY) { > elf_property_phdata = elf_ppnt; > continue; > @@ -919,7 +927,7 @@ static int load_elf_binary(struct linux_binprm *bprm) > if (retval < 0) > goto out_free_dentry; > > - break; > + continue; Because of this. As a fix, I'd expect the PT_INTERP test to be updated: if (interpreter || elf_ppnt->p_type != PT_INTERP) continue; > > out_free_interp: > kfree(elf_interpreter); > @@ -963,12 +971,11 @@ static int load_elf_binary(struct linux_binprm *bprm) > goto out_free_dentry; > > /* Pass PT_LOPROC..PT_HIPROC headers to arch code */ > - elf_property_phdata = NULL; > elf_ppnt = interp_elf_phdata; > for (i = 0; i < interp_elf_ex->e_phnum; i++, elf_ppnt++) > switch (elf_ppnt->p_type) { > case PT_GNU_PROPERTY: > - elf_property_phdata = elf_ppnt; > + interp_elf_property_phdata = elf_ppnt; > break; > > case PT_LOPROC ... PT_HIPROC: > @@ -979,10 +986,17 @@ static int load_elf_binary(struct linux_binprm *bprm) > goto out_free_dentry; > break; > } > + > + retval = parse_elf_properties(interpreter, > + interp_elf_property_phdata, > + true, true, &arch_state); > + if (retval) > + goto out_free_dentry; > + > } > > - retval = parse_elf_properties(interpreter ?: bprm->file, > - elf_property_phdata, &arch_state); > + retval = parse_elf_properties(bprm->file, elf_property_phdata, > + interpreter, false, &arch_state); > if (retval) > goto out_free_dentry; > > diff --git a/include/linux/elf.h b/include/linux/elf.h > index c9a46c4e183b..1c45ecf29147 100644 > --- a/include/linux/elf.h > +++ b/include/linux/elf.h > @@ -88,13 +88,15 @@ struct arch_elf_state; > #ifndef CONFIG_ARCH_USE_GNU_PROPERTY > static inline int arch_parse_elf_property(u32 type, const void *data, > size_t datasz, bool compat, > + bool has_interp, bool is_interp, > struct arch_elf_state *arch) > { > return 0; > } > #else > extern int arch_parse_elf_property(u32 type, const void *data, size_t datasz, > - bool compat, struct arch_elf_state *arch); > + bool compat, bool has_interp, bool is_interp, > + struct arch_elf_state *arch); > #endif > > #ifdef CONFIG_ARCH_HAVE_ELF_PROT > -- > 2.30.2 > -- Kees Cook _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel