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 2E088C2BD09 for ; Thu, 27 Jun 2024 23:00:48 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 6A0516B009D; Thu, 27 Jun 2024 19:00:47 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 64F106B009E; Thu, 27 Jun 2024 19:00:47 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 4F0206B009F; Thu, 27 Jun 2024 19:00:47 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0010.hostedemail.com [216.40.44.10]) by kanga.kvack.org (Postfix) with ESMTP id 3002B6B009D for ; Thu, 27 Jun 2024 19:00:47 -0400 (EDT) Received: from smtpin02.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id AF7ADA4444 for ; Thu, 27 Jun 2024 23:00:46 +0000 (UTC) X-FDA: 82278190092.02.E5BA706 Received: from mgamail.intel.com (mgamail.intel.com [198.175.65.21]) by imf17.hostedemail.com (Postfix) with ESMTP id 5935A40021 for ; Thu, 27 Jun 2024 23:00:43 +0000 (UTC) Authentication-Results: imf17.hostedemail.com; dkim=pass header.d=intel.com header.s=Intel header.b=RLedVjUk; spf=none (imf17.hostedemail.com: domain of ak@linux.intel.com has no SPF policy when checking 198.175.65.21) smtp.mailfrom=ak@linux.intel.com; dmarc=pass (policy=none) header.from=intel.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1719529235; 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:dkim-signature; bh=DwhoKRs8SddPs8RFTc9TzGB0yUmZAolyQcXNimmagzw=; b=UiemJ4XWopq0L/2xY7wr34Efl2onChsm4f5WFPtiq0Y4OVNTGWkUvXPKZSk5JeSUV7F1a5 yRSrk5h5b+5aFI2OY6/BfvbQbg6v1fZdG4POFMtZwIX/W8LfMQgwMMakHbZjnvQlHuB5Yg cmUoukuScWoOj50bNeRaM3JD+crmOeE= ARC-Authentication-Results: i=1; imf17.hostedemail.com; dkim=pass header.d=intel.com header.s=Intel header.b=RLedVjUk; spf=none (imf17.hostedemail.com: domain of ak@linux.intel.com has no SPF policy when checking 198.175.65.21) smtp.mailfrom=ak@linux.intel.com; dmarc=pass (policy=none) header.from=intel.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1719529235; a=rsa-sha256; cv=none; b=sp8TkNNPuXNQwiWm1GBClTWNOUueO4QmJgSbnCNZrGvOr7kW7PIgY9vq/YKkckh58vIDMX SssmUhNKj0He8T2MbcttMXrBQFwlMlBBFRIlBw12uKNhExm/9jCf/o4sbaLQNQ2Tei6WYF R4p+lX6V/VCcYKI39oRzUImVwvtKznE= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1719529244; x=1751065244; h=from:to:cc:subject:in-reply-to:references:date: message-id:mime-version; bh=bgGQ/d7DqnsysiddreU9LWxiDKypE1sKJ6+96W8e904=; b=RLedVjUkK1kjPW/KN0eks0dXl5TCea8YCMPAGDSVUva0x1lvxQ7QzQXg bqnqRf9HMdibOS3PVn9zx1IK5RawWRP7ZrOEONeozdulYgK32/k7fz6Pn g0q122zmYu2XlYzfrNNEtHaojna8yny2suO2JECTIYwbZJiAL6YC63LkS csV0gh3CPJh0i1g18DY1pbGqD1wdCwfYM5nzzCxFFA6MPQI4Afm3wD1y2 ElhWxBLffUiGTAQOSJ00zjzGVTh1QkAXAhrPaqE4b7LMph8cOtKQzIhe4 y6E58DR8v1OL8By1O3CivS6I8y0sBkZmz8shqbkhqliiQJaa4rURFbXh8 g==; X-CSE-ConnectionGUID: f6BZT3gASy+Ap4/tHgf7sw== X-CSE-MsgGUID: 1ksZQc5mTUKebYaDv6Qx1g== X-IronPort-AV: E=McAfee;i="6700,10204,11116"; a="16652792" X-IronPort-AV: E=Sophos;i="6.09,167,1716274800"; d="scan'208";a="16652792" Received: from orviesa008.jf.intel.com ([10.64.159.148]) by orvoesa113.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 27 Jun 2024 16:00:42 -0700 X-CSE-ConnectionGUID: a32kIUurQHCzq+VY0Rxsqw== X-CSE-MsgGUID: IL3M8t2ZTxuiKTxjvYN2zQ== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="6.09,167,1716274800"; d="scan'208";a="45290914" Received: from tassilo.jf.intel.com (HELO tassilo.localdomain) ([10.54.38.190]) by orviesa008.jf.intel.com with ESMTP; 27 Jun 2024 16:00:43 -0700 Received: by tassilo.localdomain (Postfix, from userid 1000) id 7A82C302A3B; Thu, 27 Jun 2024 16:00:41 -0700 (PDT) From: Andi Kleen To: Andrii Nakryiko Cc: linux-fsdevel@vger.kernel.org, brauner@kernel.org, viro@zeniv.linux.org.uk, akpm@linux-foundation.org, linux-kernel@vger.kernel.org, bpf@vger.kernel.org, gregkh@linuxfoundation.org, linux-mm@kvack.org, liam.howlett@oracle.com, surenb@google.com, rppt@kernel.org, adobriyan@gmail.com Subject: Re: [PATCH v6 3/6] fs/procfs: add build ID fetching to PROCMAP_QUERY API In-Reply-To: <20240627170900.1672542-4-andrii@kernel.org> (Andrii Nakryiko's message of "Thu, 27 Jun 2024 10:08:55 -0700") References: <20240627170900.1672542-1-andrii@kernel.org> <20240627170900.1672542-4-andrii@kernel.org> Date: Thu, 27 Jun 2024 16:00:41 -0700 Message-ID: <878qyqyorq.fsf@linux.intel.com> User-Agent: Gnus/5.13 (Gnus v5.13) MIME-Version: 1.0 Content-Type: text/plain X-Rspamd-Server: rspam03 X-Rspam-User: X-Rspamd-Queue-Id: 5935A40021 X-Stat-Signature: q1ugnc8tj93kkqzeg95c99qti7au4k7o X-HE-Tag: 1719529243-747971 X-HE-Meta: U2FsdGVkX1/nJu1AObsCnxXhISWaE0Y8+SRajmBQQfxYW0I99sw3ltu44b1JlCxeku740dOS6WrkwWWbabpRlWuxSh7CTz6mbvlX6quX4A4b/R5EjXDoFydqTFR15tT3zlAFH6Mgk00d7dQ+3bVC9aO2y2xBkU2jKmm67a4lhVaykhRkGFaF7agXlgUczIRVpX+yVK2nzP8uXNzZF3SQuVKrRIVMPHcgpFjTkOCf710oITydsdyGnTrDdsRDLbyM8bQZroYiQLzpM35/1WhCHM4DMB5hB68JRMxXiGHKJIYfsoxTR3TpuF64iqmtx/z4KeCzVk4DSpn7SWaZI6rNud2IvZkcALxUeCVQm0PxktTo0zoFbqtcEIV/2z1Es9oKkeXh9Y10kqp9yYpz448MEJGO4Ke9E0NYBrYXb3KYC6IsP0syzWnbpqRht+rHYklvoRIGYc+r6kmEqjs1YTQX3HDKwM5nDzzVZHiqDRtZChDCbSnKhDypQYPIlhwY8hWSJCnSZHKN4zcR/bNB4luTS4mEENnoDcX4e93HTtwsPtIT5kOR3ua83UmlmpUEt7N8BAlhzONY81FU0evCMBR+JmPdP91sILi5vppzA+NPTTZLsk63YVTWEI9z5TeTiQJNZ/oTHf5MWMKjNQL2byA+LxDoP1hzHWjhV/c+ZHSArR51V9INwvE+1yVldgGqC0znsD89gb1zpOaAuLekwUhl8aXXJUazLnZ1U87Ur9Fdx0LWa6XR2h1uZFygZ7hlEx3esSG/Nyu0GQXFzcF2s6LJg3NL98jcCe1fREglWHdtiPxJeJnTTFhfUIZ6Ml/hP/byz6I6bPurHVXSSUvp33lMkygDM9hAHGFb1WVDtCqBKOaRt5Fo4TLfmmFNM3NDQAp6XgCibDO+jd/NobxAPnFotTcWiXdZGxx6KH+m+kbIiJj/aCxgpzq9wJrSZpU2hggA5hhueXXEO760oeOnsQM 3Knoz0op cwp1HaPYoUn4LcJfU4mDaGvXc/BdT769E+KFjoaQ8oD+/GhoDhFGze9KEn0bX1wXeL/w78ZqNhv4aa/XPL7mW5Sly8Su1foCG+8huDHRTnEPKw02PKsHZztvXQ37M4SgSsOzArUwkP2ySO66A4tdrvZC82Sjqe/TdbTmRILEpzeaseooGdWYGekFsLIDqbkukZuc/ 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: List-Subscribe: List-Unsubscribe: Andrii Nakryiko writes: > The need to get ELF build ID reliably is an important aspect when > dealing with profiling and stack trace symbolization, and > /proc//maps textual representation doesn't help with this. > > To get backing file's ELF build ID, application has to first resolve > VMA, then use it's start/end address range to follow a special > /proc//map_files/- symlink to open the ELF file (this > is necessary because backing file might have been removed from the disk > or was already replaced with another binary in the same file path. > > Such approach, beyond just adding complexity of having to do a bunch of > extra work, has extra security implications. Because application opens > underlying ELF file and needs read access to its entire contents (as far > as kernel is concerned), kernel puts additional capable() checks on > following /proc//map_files/- symlink. And that makes > sense in general. I was curious about this statement. It has still certainly potential for side channels e.g. for files that are execute only, or with some other special protection. But actually just looking at the parsing code it seems to fail basic TOCTTOU rules, and since you don't check if the VMA mapping is executable (I think), so there's no EBUSY checking for writes, it likely is exploitable. /* only supports phdr that fits in one page */ if (ehdr->e_phnum > (PAGE_SIZE - sizeof(Elf64_Ehdr)) / sizeof(Elf64_Phdr)) <---------- check in memory return -EINVAL; phdr = (Elf64_Phdr *)(page_addr + sizeof(Elf64_Ehdr)); <---- but page is shared in the page cache. So if anybody manages to map it for write for (i = 0; i < ehdr->e_phnum; ++i) { <----- this loop can go off into the next page. if (phdr[i].p_type == PT_NOTE && !parse_build_id(page_addr, build_id, size, page_addr + phdr[i].p_offset, phdr[i].p_filesz)) return 0; Here's an untested patch diff --git a/lib/buildid.c b/lib/buildid.c index 7954dd92e36c..6c022fcd03ec 100644 --- a/lib/buildid.c +++ b/lib/buildid.c @@ -72,19 +72,20 @@ static int get_build_id_32(const void *page_addr, unsigned char *build_id, Elf32_Ehdr *ehdr = (Elf32_Ehdr *)page_addr; Elf32_Phdr *phdr; int i; + unsigned phnum = READ_ONCE(ehdr->e_phnum); /* only supports phdr that fits in one page */ - if (ehdr->e_phnum > + if (phnum > (PAGE_SIZE - sizeof(Elf32_Ehdr)) / sizeof(Elf32_Phdr)) return -EINVAL; phdr = (Elf32_Phdr *)(page_addr + sizeof(Elf32_Ehdr)); - for (i = 0; i < ehdr->e_phnum; ++i) { + for (i = 0; i < phnum; ++i) { if (phdr[i].p_type == PT_NOTE && !parse_build_id(page_addr, build_id, size, page_addr + phdr[i].p_offset, - phdr[i].p_filesz)) + READ_ONCE(phdr[i].p_filesz))) return 0; } return -EINVAL; @@ -97,15 +98,16 @@ static int get_build_id_64(const void *page_addr, unsigned char *build_id, Elf64_Ehdr *ehdr = (Elf64_Ehdr *)page_addr; Elf64_Phdr *phdr; int i; + unsigned phnum = READ_ONCE(ehdr->e_phnum); /* only supports phdr that fits in one page */ - if (ehdr->e_phnum > + if (phnum > (PAGE_SIZE - sizeof(Elf64_Ehdr)) / sizeof(Elf64_Phdr)) return -EINVAL; phdr = (Elf64_Phdr *)(page_addr + sizeof(Elf64_Ehdr)); - for (i = 0; i < ehdr->e_phnum; ++i) { + for (i = 0; i < phnum; ++i) { if (phdr[i].p_type == PT_NOTE && !parse_build_id(page_addr, build_id, size, page_addr + phdr[i].p_offset,