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 7EE3CC4332F for ; Thu, 27 Jan 2022 06:31:46 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id E340B6B0071; Thu, 27 Jan 2022 01:31:45 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id DE2866B0072; Thu, 27 Jan 2022 01:31:45 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id CA99F6B0073; Thu, 27 Jan 2022 01:31:45 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (relay039.a.hostedemail.com [64.99.140.39]) by kanga.kvack.org (Postfix) with ESMTP id B92BB6B0071 for ; Thu, 27 Jan 2022 01:31:45 -0500 (EST) Received: from smtpin01.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id 87587230B2 for ; Thu, 27 Jan 2022 06:31:45 +0000 (UTC) X-FDA: 79075096170.01.58A82FE Received: from mail-pj1-f42.google.com (mail-pj1-f42.google.com [209.85.216.42]) by imf11.hostedemail.com (Postfix) with ESMTP id C124B4001A for ; Thu, 27 Jan 2022 06:31:44 +0000 (UTC) Received: by mail-pj1-f42.google.com with SMTP id d5so1925324pjk.5 for ; Wed, 26 Jan 2022 22:31:44 -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:content-transfer-encoding:in-reply-to; bh=lbnwPKU+OhK3Q8fw1IuxNsJFONUjafO81x0YepRwpVU=; b=kO9cnRNWfjIxqhEdVxCFXalKyVLcLDpNmXISPWJKVrRl/8JOO7AgK6mLU7YlhwTgor pCioL/3CJ1Fmt8XwtuZPo7bxRC4d9cy0rws8xX3sp89q1pvSg2xrocgc9sgxD91sCWuT PaL/uBnscqfMv92i5lPUBveJNHc6A9V30lsuE= 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:content-transfer-encoding :in-reply-to; bh=lbnwPKU+OhK3Q8fw1IuxNsJFONUjafO81x0YepRwpVU=; b=PorHnbceWlw5wo1oBmghtj0f/lozJleQJYRFUDxdX/hPo3m261k1oEX8rgTSwj9Md3 M3svF/DHd3sE8UcY/vsz1mZeckVyC8wjzoLfSmCaEvUJjarg/pM6mSCPqEG16oTM9N60 SWwWMMBx+2ZN0Eh+i/OPisSCuogkzLyfIEknbEhSHGenKXt/3XZezzfuGCOTFxYpLUTk ngYEslvfk9QbYZDyy3Vey+ylhgizXLoclG7PruEcUr0Wz1bq/+ciukz8tsijv5XuaztW CWo10iQlOjLwjJ8gdFdkFgYlBWyX1DS2voZy9xixMi36FZTV6D4f1GnJycxpv1P4YKzr PSoQ== X-Gm-Message-State: AOAM533VE/N7txP49MbR55/FrNONyvM7pPbYSbigJGF6SwN3fsLf+H9Z 3canWgsz0qPsrCwM3f2qybdC3wdU+aK5PQ== X-Google-Smtp-Source: ABdhPJxvX+FZ7Zy2fT+BIbiP7+n2FkjaQpST0iFKTFY5IKtAfg1u2lmwN3kwf3oFPjUo3NWaIp2P4w== X-Received: by 2002:a17:902:ab5b:: with SMTP id ij27mr2171967plb.148.1643265103569; Wed, 26 Jan 2022 22:31:43 -0800 (PST) Received: from www.outflux.net (smtp.outflux.net. [198.145.64.163]) by smtp.gmail.com with ESMTPSA id l13sm18528130pgs.16.2022.01.26.22.31.43 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 26 Jan 2022 22:31:43 -0800 (PST) Date: Wed, 26 Jan 2022 22:31:42 -0800 From: Kees Cook To: Andrew Morton Cc: Magnus =?iso-8859-1?Q?Gro=DF?= , Alexander Viro , Eric Biederman , linux-fsdevel@vger.kernel.org, linux-mm@kvack.org, linux-hardening@vger.kernel.org Subject: Re: [PATCH] elf: Relax assumptions about vaddr ordering Message-ID: <202201262230.E16DF58B@keescook> References: <202201260845.FCBC0B5A06@keescook> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <202201260845.FCBC0B5A06@keescook> X-Rspamd-Server: rspam11 X-Rspamd-Queue-Id: C124B4001A X-Stat-Signature: qzagwsx8awbjiohcxbgjjgmzeg9ny5r1 Authentication-Results: imf11.hostedemail.com; dkim=pass header.d=chromium.org header.s=google header.b=kO9cnRNW; dmarc=pass (policy=none) header.from=chromium.org; spf=pass (imf11.hostedemail.com: domain of keescook@chromium.org designates 209.85.216.42 as permitted sender) smtp.mailfrom=keescook@chromium.org X-Rspam-User: nil X-HE-Tag: 1643265104-290475 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 Wed, Jan 26, 2022 at 08:50:15AM -0800, Kees Cook wrote: > On Wed, Jan 26, 2022 at 05:25:20PM +0100, Magnus Groß wrote: > > From ff4dde97e82727727bda711f2367c05663498b24 Mon Sep 17 00:00:00 2001 > > From: =?UTF-8?q?Magnus=20Gro=C3=9F?= > > Date: Wed, 26 Jan 2022 16:35:07 +0100 > > Subject: [PATCH] elf: Relax assumptions about vaddr ordering > > MIME-Version: 1.0 > > Content-Type: text/plain; charset=UTF-8 > > Content-Transfer-Encoding: 8bit > > > > Commit 5f501d555653 ("binfmt_elf: reintroduce using > > MAP_FIXED_NOREPLACE") introduced a regression, where the kernel now > > assumes that PT_LOAD segments are ordered by vaddr in load_elf_binary(). > > > > Specifically consider an ELF binary with the following PT_LOAD segments: > > > > Type Offset VirtAddr PhysAddr FileSiz MemSiz Flg Align > > LOAD 0x000000 0x08000000 0x08000000 0x474585 0x474585 R E 0x1000 > > LOAD 0x475000 0x08475000 0x08475000 0x090a4 0xc6c10 RW 0x1000 > > LOAD 0x47f000 0x00010000 0x00010000 0x00000 0x7ff0000 0x1000 > > > > Note how the last segment is actually the first segment and vice versa. > > > > Since total_mapping_size() only computes the difference between the > > first and the last segment in the order that they appear, it will return > > a size of 0 in this case, thus causing load_elf_binary() to fail, which > > did not happen before that change. > > > > Strictly speaking total_mapping_size() made that assumption already > > before that patch, but the issue did not appear because the old > > load_addr_set guards never allowed this call to total_mapping_size(). > > > > Instead of fixing this by reverting to the old load_addr_set logic, we > > fix this by comparing the correct first and last segments in > > total_mapping_size(). > > Ah, nice. Yeah, this is good. > > > Signed-off-by: Magnus Groß > > Fixes: 5f501d555653 ("binfmt_elf: reintroduce using MAP_FIXED_NOREPLACE") > Cc: stable@vger.kernel.org > Acked-by: Kees Cook Andrew, can you pick this up too? -Kees > > -Kees > > > --- > > fs/binfmt_elf.c | 18 ++++++++++++++---- > > 1 file changed, 14 insertions(+), 4 deletions(-) > > > > diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c > > index f8c7f26f1fbb..0caaad9eddd1 100644 > > --- a/fs/binfmt_elf.c > > +++ b/fs/binfmt_elf.c > > @@ -402,19 +402,29 @@ static unsigned long elf_map(struct file *filep, unsigned long addr, > > static unsigned long total_mapping_size(const struct elf_phdr *cmds, int nr) > > { > > int i, first_idx = -1, last_idx = -1; > > + unsigned long min_vaddr = ULONG_MAX, max_vaddr = 0; > > > > for (i = 0; i < nr; i++) { > > if (cmds[i].p_type == PT_LOAD) { > > - last_idx = i; > > - if (first_idx == -1) > > + /* > > + * The PT_LOAD segments are not necessarily ordered > > + * by vaddr. Make sure that we get the segment with > > + * minimum vaddr (maximum vaddr respectively) > > + */ > > + if (cmds[i].p_vaddr <= min_vaddr) { > > first_idx = i; > > + min_vaddr = cmds[i].p_vaddr; > > + } > > + if (cmds[i].p_vaddr >= max_vaddr) { > > + last_idx = i; > > + max_vaddr = cmds[i].p_vaddr; > > + } > > } > > } > > if (first_idx == -1) > > return 0; > > > > - return cmds[last_idx].p_vaddr + cmds[last_idx].p_memsz - > > - ELF_PAGESTART(cmds[first_idx].p_vaddr); > > + return max_vaddr + cmds[last_idx].p_memsz - ELF_PAGESTART(min_vaddr); > > } > > > > static int elf_read(struct file *file, void *buf, size_t len, loff_t pos) > > -- > > 2.34.1 > > -- > Kees Cook -- Kees Cook