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 169DFC7EE43 for ; Mon, 12 Jun 2023 08:08:55 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 7B2826B0072; Mon, 12 Jun 2023 04:08:55 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 7622B6B0074; Mon, 12 Jun 2023 04:08:55 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 651358E0002; Mon, 12 Jun 2023 04:08:55 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0011.hostedemail.com [216.40.44.11]) by kanga.kvack.org (Postfix) with ESMTP id 570B26B0072 for ; Mon, 12 Jun 2023 04:08:55 -0400 (EDT) Received: from smtpin19.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay05.hostedemail.com (Postfix) with ESMTP id 1EA0240151 for ; Mon, 12 Jun 2023 08:08:55 +0000 (UTC) X-FDA: 80893369830.19.1D5ED45 Received: from out03.mta.xmission.com (out03.mta.xmission.com [166.70.13.233]) by imf27.hostedemail.com (Postfix) with ESMTP id 9C0894000A for ; Mon, 12 Jun 2023 08:08:52 +0000 (UTC) Authentication-Results: imf27.hostedemail.com; dkim=none; dmarc=pass (policy=none) header.from=xmission.com; spf=pass (imf27.hostedemail.com: domain of ebiederm@xmission.com designates 166.70.13.233 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=1686557333; 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=uCtGRU8uXT1xudZcvneYOzXF5EytkklDloprBwhkpkk=; b=bSogWtOyGCfCe+stj+0L1crkNBDPD1JZURFMBMETXQCS/Qk6otOHo73cfi0XenlMqMdett jVy/k8l53w5cOl1Ormg8krRc+XISnfwTt/V4VMSvwRal+exOiILOOu3QHefMXsUn5baA8q HFjGIXA9dOZTy3HXS6xUbWhwWfBwcVw= ARC-Authentication-Results: i=1; imf27.hostedemail.com; dkim=none; dmarc=pass (policy=none) header.from=xmission.com; spf=pass (imf27.hostedemail.com: domain of ebiederm@xmission.com designates 166.70.13.233 as permitted sender) smtp.mailfrom=ebiederm@xmission.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1686557333; a=rsa-sha256; cv=none; b=GPX2Aml0N4cSh+HvRaprQe5bkc0G83Ea/lnLKiZbDCkBW0nZXZPc1OhiWOzUNwzVUhbMjs rEVw821ZYUT2CJ1TUNgGaBqonilJJW4tJw2hRsuB9sucIkKjmqgGLFUR5+hH+bya6KfcUn FzzbAaGfqv05WJtgRUfyl9hZO2UmMlg= Received: from in02.mta.xmission.com ([166.70.13.52]:49274) by out03.mta.xmission.com with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.93) (envelope-from ) id 1q8cba-00GVnS-6T; Mon, 12 Jun 2023 02:08:50 -0600 Received: from ip68-110-29-46.om.om.cox.net ([68.110.29.46]:41498 helo=email.froward.int.ebiederm.org.xmission.com) by in02.mta.xmission.com with esmtpsa (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.93) (envelope-from ) id 1q8cbY-00F8nl-Ss; Mon, 12 Jun 2023 02:08:49 -0600 From: "Eric W. Biederman" To: Christian Marangi Cc: Alexander Viro , Christian Brauner , Kees Cook , Mark Brown , Dave Martin , Catalin Marinas , linux-fsdevel@vger.kernel.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org, stable@vger.kernel.org References: <20230607144227.8956-1-ansuelsmth@gmail.com> Date: Mon, 12 Jun 2023 03:08:21 -0500 In-Reply-To: <20230607144227.8956-1-ansuelsmth@gmail.com> (Christian Marangi's message of "Wed, 7 Jun 2023 16:42:27 +0200") Message-ID: <875y7tumq2.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=1q8cbY-00F8nl-Ss;;;mid=<875y7tumq2.fsf@email.froward.int.ebiederm.org>;;;hst=in02.mta.xmission.com;;;ip=68.110.29.46;;;frm=ebiederm@xmission.com;;;spf=pass X-XM-AID: U2FsdGVkX19mPd4u3NrhQg/U/AK1EfBVUc3XuMgnfKI= X-SA-Exim-Connect-IP: 68.110.29.46 X-SA-Exim-Mail-From: ebiederm@xmission.com Subject: Re: [PATCH] binfmt_elf: dynamically allocate note.data in parse_elf_properties X-SA-Exim-Version: 4.2.1 (built Sat, 08 Feb 2020 21:53:50 +0000) X-SA-Exim-Scanned: Yes (on in02.mta.xmission.com) X-Rspamd-Queue-Id: 9C0894000A X-Rspam-User: X-Rspamd-Server: rspam02 X-Stat-Signature: h33jcc963tjdhiyx98xiiw77tt3i14h8 X-HE-Tag: 1686557332-2202 X-HE-Meta: U2FsdGVkX18/xyHL3z07x/Yi24EM4SN+mwDN/aM9YlEFRc3h3UMlHjiC38MsYYXgNbyUepF6XhAi6Y+PIM3qxNTx6JAmvFkPVv3WnnFjxrnzyh4NOhxhFczlQ4LIfu/DIhL3H79iQgSdVTu1YMo8i6RHF7M9isbjKV0Vw+MrKz6s97sFLv8tFfW9OaCapOAq4vMyb+sJ1mxP66Pfj+McqtE2dVI17IJOhLxwRAijlw04Pox6+1u0yoWQ/xrfTDXZC7ZobzeFnyzc3TLsvaRYLE/RWViyupQe/emxRU5oEcsQQF2Sm5UlI8rdkhRP6vJ5xjYsk56HKg9imSTNQsk92ekcFbhw+ATBGm8qUtoY37c3HRDGX3Tp954zWLvtze4PB3jzbiiUSIIf1uETJLNRuZgpbCYhqN+cYl6NQGmTx0hyAyqCekIkJAFFrcbDNBBmkSulWBdshCLmUApPFPQNopGSGEdXgCq3o5HhmwZoGtGvb5NrMPaiZzjkaYeZvXz9VKV/iiIep0YBy/LnZUhunDg3old8k8EDqjCkJAb3dHoOQFJ9E+pgMVZtMNYIzyHpm0O8oAl9xcJuQeeAFBhYDy55f8KQJ8kAtcUZ6fSw5POIhQ5Y9zUoHtnWUdS63hPvfGgn87g5fLZcHR880gqBU39gX7x7VPL5ePY1lXiU69XIQGotqFcpXipGDs6SAV/IVLwhFMaQQSAVDBn/8ww6XYqwtGkUpYYUnngYow8rHbxni1Pn8ZYlxGOuKr0saOk0FMeLeD/9xc9Ea3xoiwkkHQJRNSyqT74jRVXXQtpSubBTQFvPhDpG3lYiLtsn1+nWbzG1Tmyazrn0iOre+u2UScHrix/1YW4BU6awlkcNHydNda5sNw8K492YpJbb8r5o7M+0x7VLdaDC4X3Y7lMtEtH2diy1TYLjRmmVWCGV5FXNUDWYKGGTutoLFoQT4lwvM0261dkjN/J+twGg0lq siRyif1Q 8Iw/H5E7cKo2/brxTvWJts7ApOAPrj7ObdmxGcit6FBQC0OxdyHQDEYeEkI3kqFfksI/9rGNMDWzFah3IgTMpedNtEc/EQ1iWdiWU5Wpc5MqW+G2AvtJ4n9T0PYeeoiV3QBTl69Jgy9qhHk7/fLxn8lZIIg== 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: Christian Marangi writes: > Dynamically allocate note.data in parse_elf_properties to fix > compilation warning on some arch. > > On some arch note.data exceed the stack limit for a single function and > this cause the following compilation warning: > fs/binfmt_elf.c: In function 'parse_elf_properties.isra': > fs/binfmt_elf.c:821:1: error: the frame size of 1040 bytes is larger than 1024 bytes [-Werror=frame-larger-than=] > 821 | } > | ^ > cc1: all warnings being treated as errors > > Fix this by dynamically allocating the array. > Update the sizeof of the union to the biggest element allocated. > > Fixes: 00e19ceec80b ("ELF: Add ELF program property parsing support") > Signed-off-by: Christian Marangi > Cc: stable@vger.kernel.org # v5.8+ > --- > fs/binfmt_elf.c | 36 +++++++++++++++++++++++++----------- > 1 file changed, 25 insertions(+), 11 deletions(-) > > diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c > index 44b4c42ab8e8..90daa623ca13 100644 > --- a/fs/binfmt_elf.c > +++ b/fs/binfmt_elf.c > @@ -768,7 +768,7 @@ static int parse_elf_properties(struct file *f, const struct elf_phdr *phdr, > { > union { > struct elf_note nhdr; > - char data[NOTE_DATA_SZ]; > + char *data; > } note; If you are going to dynamically allocate this not that way please. Only dynamically allocating one member of a union is to put it politely completely broken. The entire union needs to be dynamically allocated. Given that the entire thing is 1024 bytes in size. I can understand the concern. Hmm. The entire union is a buffer for the entire note section. So 1K is understandable if it needs to hold all of the notes. Of course only a single note is a wrapper of a bunch of gnu_properties. Hopefully that single note comes first. The notehdr + name take 16 bytes. The only supported gnu_property takes 12 bytes. I think 16 in practice. Hmm. So we could do with a smaller buffer. Hmm. The code does not check that all phdr->p_filesz bytes are actually read. So I would suggest defining the union to be ELF_EXEC_PAGESIZE bytes, dynamically allocating it like we do all of the other buffers we read elf headers into, and then use elf_read to verify that we read all of phdr->p_filesz bytes. Just like we do for the elf program headers. I think having a second pattern for reading data is more likely to be a problem than a dynamic memory allocation. Especially since this code only runs on one architecture in practice. The changes will cost nothing except on arm64 and it will be as cheap as it can be, being simply a single page allocation. Either that or you can up your stack limit on 64bit architectures like everyone else. Eric