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 6BE0AC433EF for ; Thu, 20 Jan 2022 12:23:59 +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-Type: Content-Transfer-Encoding:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:In-Reply-To:From:References:Cc:To:Subject: MIME-Version:Date:Message-ID:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=ifzLPOCHxigT6uifsG+pzYM+WSloiASLj0ISGMP0LIY=; b=RXHX4t/XhbcLub 9hP0boKBgKg+tsGeOzb8xnnvF2F32aIouoXq8ZI0HSLRPaAM88Q/9DB4wVLkN7o9YriYRuHVthFXy SSVO3jrweog5Mr0HzsmaztwvqAMjv57YpiilgxD7F01uPuYwMfrOM4llOBurtwblNtEcwRZzUXtTi 6UYvugHM+ihD6dN5ohuKs22pS3q5c/w2v0Qb8zUrnjDX+ZLolpY6iqtEv0e5+i+TSZa7OatX3e+U4 V5I0nqIJdQnxbjthpjwNFa5gPu/561aS6fUMnJ+kLW6AChuLCsMBfCZe3oiFJ6Rr42umfm3VGvNK3 FAnehucY97EDFdeRd15g==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1nAWSl-00BLVg-2M; Thu, 20 Jan 2022 12:22:47 +0000 Received: from foss.arm.com ([217.140.110.172]) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1nAWSh-00BLUU-I1 for linux-arm-kernel@lists.infradead.org; Thu, 20 Jan 2022 12:22:44 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 91B2FED1; Thu, 20 Jan 2022 04:22:42 -0800 (PST) Received: from [10.57.68.26] (unknown [10.57.68.26]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 97FE53F774; Thu, 20 Jan 2022 04:22:40 -0800 (PST) Message-ID: <319b09bc-56a2-207f-6180-3cc7d8cd43d1@arm.com> Date: Thu, 20 Jan 2022 12:22:35 +0000 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; rv:91.0) Gecko/20100101 Thunderbird/91.5.0 Subject: Re: [PATCH] vmap(): don't allow invalid pages Content-Language: en-GB To: "Russell King (Oracle)" Cc: Matthew Wilcox , Yury Norov , Catalin Marinas , Will Deacon , Andrew Morton , Nicholas Piggin , Ding Tianhong , Anshuman Khandual , Alexey Klimov , linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, linux-mm@kvack.org References: <20220118235244.540103-1-yury.norov@gmail.com> From: Robin Murphy In-Reply-To: X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20220120_042243_679570_D4F63296 X-CRM114-Status: GOOD ( 16.44 ) 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-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On 2022-01-19 19:12, Russell King (Oracle) wrote: > On Wed, Jan 19, 2022 at 06:43:10PM +0000, Robin Murphy wrote: >> Indeed, my impression is that the only legitimate way to get hold of a page >> pointer without assumed provenance is via pfn_to_page(), which is where >> pfn_valid() comes in. Thus pfn_valid(page_to_pfn()) really *should* be a >> tautology. > > That can only be true if pfn == page_to_pfn(pfn_to_page(pfn)) for all > values of pfn. > > Given how pfn_to_page() is defined in the sparsemem case: > > #define __pfn_to_page(pfn) \ > ({ unsigned long __pfn = (pfn); \ > struct mem_section *__sec = __pfn_to_section(__pfn); \ > __section_mem_map_addr(__sec) + __pfn; \ > }) > #define page_to_pfn __page_to_pfn > > that isn't the case, especially when looking at page_to_pfn(): > > #define __page_to_pfn(pg) \ > ({ const struct page *__pg = (pg); \ > int __sec = page_to_section(__pg); \ > (unsigned long)(__pg - __section_mem_map_addr(__nr_to_section(__sec))); \ > }) > > Where: > > static inline unsigned long page_to_section(const struct page *page) > { > return (page->flags >> SECTIONS_PGSHIFT) & SECTIONS_MASK; > } > > So if page_to_section() returns something that is, e.g. zero for an > invalid page in a non-zero section, you're not going to end up with > the right pfn from page_to_pfn(). Right, I emphasised "should" in an attempt to imply "in the absence of serious bugs that have further-reaching consequences anyway". > As I've said now a couple of times, trying to determine of a struct > page pointer is valid is the wrong question to be asking. And doing so in one single place, on the justification of avoiding an incredibly niche symptom, is even more so. Not to mention that an address size fault is one of the best possible outcomes anyway, vs. the untold damage that may stem from accesses actually going through to random parts of the physical memory map. Robin. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel