From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.remote-ip=100.103.45.18 ARC-Seal: i=1; d=subspace.kernel.org; s=arc20261015; a=rsa-sha256; cv=none; t=1793362023; b=slP/5LkntpSl/WFBRtNQIdqVHSQ5yQtvkKsndwNN3s9btE5ER4qzUQgCPxb3U2vACwFu 8qPHxuRVWk4wMMifYSfW5E8rTku9hTZZ6m6zwOSPhV+4b8PT6vU2EE6eUG4DaI+VNdzLq elJ2kOn2pDBeO6gWnNc66egeRA983NazU9gaBqdzzgfTnEPOnkeGBZ8utsoavXiQALfvg KLSir1kzcqjn/EYCAWI8fOJV8XfvGLtJ8UXPm3kkRBmZDJZoFk+obi7x7PBtnXeMHvNPz JMedK6VlmTmWqXbv5P8dWjaKelJJrogJmqdbywWsxULJXistiUNxvvUr55ZIbQgS+WQ== ARC-Message-Signature: i=1; d=subspace.kernel.org; s=arc20261015; a=rsa-sha256; c=relaxed/relaxed; t=1793362023; h=Received:DKIM-Signature:Message-ID:Date:MIME-Version:User-Agent: Subject:To:Cc:References:From:Content-Language:Autocrypt:In-Reply-To: Content-Type:Content-Transfer-Encoding; bh=MoHT6WHyffUX/dGh5q8tXlh6lkd1ZuWufnzc0VjLK4Q=; b=XM9pHPL+QkwjBm7ShV3sGH5V3nn4A+MOOEsM782rdkyHfs8LZH8cCTwxtFrGQgtbAI5K plzPYVtYWLGRvOuE6UAVXZLv/D/c2+2aOKlXg4qahGq0u74FVyXDxpmJNpiLAGVJav/rS ooBoFFBnln2Qr7ssmdyWGK24Y9LUo+F8Zonsx0hfi5quF8qVX1Cz4obAzCUZqGKSshctb REfiLLyOFnlRq7uew05kn+ZFSmmewmQgpR3myJmacmNdNAsaO6ZyEeC3vsfxCXOZVKXdQ B+auw6vUt97v0NPZYZuFW6hx19ZJTfO+87fnykbDFafT4Jc0ruYfwbHVquHHueo1tSw== ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dkim=pass header.d=kernel.org header.i=@kernel.org header.a=rsa-sha256 header.s=k20260515 header.b=gHG0SPEp; arc=none smtp.remote-ip=100.103.45.18 Received: from smtp.kernel.org (quasi.space.kernel.org [100.103.45.18]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (prime256v1)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id DB228606A31A; Fri, 30 Oct 2026 12:07:02 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key, unprotected) header.d=kernel.org header.i=@kernel.org header.a=rsa-sha256 header.s=k20260515 header.b=gHG0SPEp Received: by smtp.kernel.org (Postfix) with ESMTPSA id 64BD21F000C1; Fri, 30 Oct 2026 12:06:48 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=kernel.org; s=k20260515; t=1793362022; bh=MoHT6WHyffUX/dGh5q8tXlh6lkd1ZuWufnzc0VjLK4Q=; h=Date:Subject:To:Cc:References:From:In-Reply-To; b=gHG0SPEprF9GqAtCjmA/2Rew1TV5CEtn7NJkzBh24jY0GRkkosdgs5df/0e2zVz1A EJkUJNifA5AYwttFZJRdEcjqqiwVtxiUq1k/1Wb3QSW697JdOK7VRU4/dIYKUyrgr2 DF4wBoCpoXJ503MSXy0H0jzbZ2I13WgmxNXUQFcRn2OZxzQh2V6RGZXNvuWUHHSG2I zeMYdjdUs5wKChneiKiJYQRW0NFc3652bEbk+rSr2fIZoQ5zpnLX4z1SLL2sZOpuZJ qIea0Ja2a9YNX/iqTGFM6cX+5pWz0RGaMd3mvSDueXB2FQrRZIeEbK2WGPEtUO7SRt VdHZkY9DsKidw== Message-ID: <297adf4c-1792-4dff-a7c8-2694dc2b1446@kernel.org> Date: Fri, 30 Oct 2026 13:06:45 +0100 X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Post: List-Help: List-Subscribe: List-Unsubscribe: List-Owner: Precedence: list MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v2 03/47] mm/memfd: check for real shmem inodes before accessing shmem state To: Pratyush Yadav Cc: Andrew Morton , Johannes Weiner , Michal Hocko , Qi Zheng , Shakeel Butt , Lorenzo Stoakes , Kairui Song , Barry Song , Axel Rasmussen , Yuanchu Xie , Wei Xu , Baoquan He , Baolin Wang , "Liam R. Howlett" , Vlastimil Babka , Mike Rapoport , Suren Baghdasaryan , Michal Hocko , Kees Cook , Peter Zijlstra , Jann Horn , Pedro Falcato , Hugh Dickins , Pasha Tatashin , "Matthew Wilcox (Oracle)" , Jan Kara , Zi Yan , Lance Yang , Rik van Riel , Harry Yoo , Alistair Popple , Nico Pache , Ryan Roberts , Dev Jain , Usama Arif , Kiryl Shutsemau , Matthew Brost , John Hubbard , Peter Xu , Will Deacon , "Aneesh Kumar K.V" , Nick Piggin , Oscar Salvador , Minchan Kim , Muchun Song , Xu Xin , Chengming Zhou , Chris Li , Kemeng Shi , Nhat Pham , Youngjun Park , Gregory Price , Joshua Hahn , Rakie Kim , Ying Huang , Jason Gunthorpe , linux-mm@kvack.org, linux-kernel@vger.kernel.org, kexec@lists.infradead.org, linux-fsdevel@vger.kernel.org, linux-arch@vger.kernel.org, linux-cxl@vger.kernel.org, stable@vger.kernel.org, Chris Mason References: <20261029-fixes_core-v2-0-a64ce7f83abb@kernel.org> <20261029-fixes_core-v2-3-a64ce7f83abb@kernel.org> <2vxzmrrvmnil.fsf@kernel.org> From: "David Hildenbrand (Arm)" Content-Language: en-US Autocrypt: addr=david@kernel.org; keydata= xsFNBFXLn5EBEAC+zYvAFJxCBY9Tr1xZgcESmxVNI/0ffzE/ZQOiHJl6mGkmA1R7/uUpiCjJ dBrn+lhhOYjjNefFQou6478faXE6o2AhmebqT4KiQoUQFV4R7y1KMEKoSyy8hQaK1umALTdL QZLQMzNE74ap+GDK0wnacPQFpcG1AE9RMq3aeErY5tujekBS32jfC/7AnH7I0v1v1TbbK3Gp XNeiN4QroO+5qaSr0ID2sz5jtBLRb15RMre27E1ImpaIv2Jw8NJgW0k/D1RyKCwaTsgRdwuK Kx/Y91XuSBdz0uOyU/S8kM1+ag0wvsGlpBVxRR/xw/E8M7TEwuCZQArqqTCmkG6HGcXFT0V9 PXFNNgV5jXMQRwU0O/ztJIQqsE5LsUomE//bLwzj9IVsaQpKDqW6TAPjcdBDPLHvriq7kGjt WhVhdl0qEYB8lkBEU7V2Yb+SYhmhpDrti9Fq1EsmhiHSkxJcGREoMK/63r9WLZYI3+4W2rAc UucZa4OT27U5ZISjNg3Ev0rxU5UH2/pT4wJCfxwocmqaRr6UYmrtZmND89X0KigoFD/XSeVv jwBRNjPAubK9/k5NoRrYqztM9W6sJqrH8+UWZ1Idd/DdmogJh0gNC0+N42Za9yBRURfIdKSb B3JfpUqcWwE7vUaYrHG1nw54pLUoPG6sAA7Mehl3nd4pZUALHwARAQABzS5EYXZpZCBIaWxk ZW5icmFuZCAoQ3VycmVudCkgPGRhdmlkQGtlcm5lbC5vcmc+wsGQBBMBCAA6AhsDBQkmWAik AgsJBBUKCQgCFgICHgUCF4AWIQQb2cqtc1xMOkYN/MpN3hD3AP+DWgUCaYJt/AIZAQAKCRBN 3hD3AP+DWriiD/9BLGEKG+N8L2AXhikJg6YmXom9ytRwPqDgpHpVg2xdhopoWdMRXjzOrIKD g4LSnFaKneQD0hZhoArEeamG5tyo32xoRsPwkbpIzL0OKSZ8G6mVbFGpjmyDLQCAxteXCLXz ZI0VbsuJKelYnKcXWOIndOrNRvE5eoOfTt2XfBnAapxMYY2IsV+qaUXlO63GgfIOg8RBaj7x 3NxkI3rV0SHhI4GU9K6jCvGghxeS1QX6L/XI9mfAYaIwGy5B68kF26piAVYv/QZDEVIpo3t7 /fjSpxKT8plJH6rhhR0epy8dWRHk3qT5tk2P85twasdloWtkMZ7FsCJRKWscm1BLpsDn6EQ4 jeMHECiY9kGKKi8dQpv3FRyo2QApZ49NNDbwcR0ZndK0XFo15iH708H5Qja/8TuXCwnPWAcJ DQoNIDFyaxe26Rx3ZwUkRALa3iPcVjE0//TrQ4KnFf+lMBSrS33xDDBfevW9+Dk6IISmDH1R HFq2jpkN+FX/PE8eVhV68B2DsAPZ5rUwyCKUXPTJ/irrCCmAAb5Jpv11S7hUSpqtM/6oVESC 3z/7CzrVtRODzLtNgV4r5EI+wAv/3PgJLlMwgJM90Fb3CB2IgbxhjvmB1WNdvXACVydx55V7 LPPKodSTF29rlnQAf9HLgCphuuSrrPn5VQDaYZl4N/7zc2wcWM7BTQRVy5+RARAA59fefSDR 9nMGCb9LbMX+TFAoIQo/wgP5XPyzLYakO+94GrgfZjfhdaxPXMsl2+o8jhp/hlIzG56taNdt VZtPp3ih1AgbR8rHgXw1xwOpuAd5lE1qNd54ndHuADO9a9A0vPimIes78Hi1/yy+ZEEvRkHk /kDa6F3AtTc1m4rbbOk2fiKzzsE9YXweFjQvl9p+AMw6qd/iC4lUk9g0+FQXNdRs+o4o6Qvy iOQJfGQ4UcBuOy1IrkJrd8qq5jet1fcM2j4QvsW8CLDWZS1L7kZ5gT5EycMKxUWb8LuRjxzZ 3QY1aQH2kkzn6acigU3HLtgFyV1gBNV44ehjgvJpRY2cC8VhanTx0dZ9mj1YKIky5N+C0f21 zvntBqcxV0+3p8MrxRRcgEtDZNav+xAoT3G0W4SahAaUTWXpsZoOecwtxi74CyneQNPTDjNg azHmvpdBVEfj7k3p4dmJp5i0U66Onmf6mMFpArvBRSMOKU9DlAzMi4IvhiNWjKVaIE2Se9BY FdKVAJaZq85P2y20ZBd08ILnKcj7XKZkLU5FkoA0udEBvQ0f9QLNyyy3DZMCQWcwRuj1m73D sq8DEFBdZ5eEkj1dCyx+t/ga6x2rHyc8Sl86oK1tvAkwBNsfKou3v+jP/l14a7DGBvrmlYjO 59o3t6inu6H7pt7OL6u6BQj7DoMAEQEAAcLBfAQYAQgAJgIbDBYhBBvZyq1zXEw6Rg38yk3e EPcA/4NaBQJonNqrBQkmWAihAAoJEE3eEPcA/4NaKtMQALAJ8PzprBEXbXcEXwDKQu+P/vts IfUb1UNMfMV76BicGa5NCZnJNQASDP/+bFg6O3gx5NbhHHPeaWz/VxlOmYHokHodOvtL0WCC 8A5PEP8tOk6029Z+J+xUcMrJClNVFpzVvOpb1lCbhjwAV465Hy+NUSbbUiRxdzNQtLtgZzOV Zw7jxUCs4UUZLQTCuBpFgb15bBxYZ/BL9MbzxPxvfUQIPbnzQMcqtpUs21CMK2PdfCh5c4gS sDci6D5/ZIBw94UQWmGpM/O1ilGXde2ZzzGYl64glmccD8e87OnEgKnH3FbnJnT4iJchtSvx yJNi1+t0+qDti4m88+/9IuPqCKb6Stl+s2dnLtJNrjXBGJtsQG/sRpqsJz5x1/2nPJSRMsx9 5YfqbdrJSOFXDzZ8/r82HgQEtUvlSXNaXCa95ez0UkOG7+bDm2b3s0XahBQeLVCH0mw3RAQg r7xDAYKIrAwfHHmMTnBQDPJwVqxJjVNr7yBic4yfzVWGCGNE4DnOW0vcIeoyhy9vnIa3w1uZ 3iyY2Nsd7JxfKu1PRhCGwXzRw5TlfEsoRI7V9A8isUCoqE2Dzh3FvYHVeX4Us+bRL/oqareJ CIFqgYMyvHj7Q06kTKmauOe4Nf0l0qEkIuIzfoLJ3qr5UyXc2hLtWyT9Ir+lYlX9efqh7mOY qIws/H2t In-Reply-To: <2vxzmrrvmnil.fsf@kernel.org> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit On 10/30/26 12:33, Pratyush Yadav wrote: > Hi David, > > On Thu, Oct 29 2026, David Hildenbrand (Arm) wrote: > >> shmem_file() only checks a file's backing mapping. That mapping can belong >> to another file, as with a Coda file backed by a tmpfs container after >> mmap(). It does not establish that file_inode(file) is a shmem inode. >> >> Seal handling and memfd preservation use this check before accessing >> shmem inode state, so they can access an inode of the wrong type. >> >> Add file_inode_is_shmem() to check the inode's file operations in both >> paths. This identifies regular shmem inodes, including those used by >> memfds and ordinary tmpfs files, independently of wrapper file operations. >> >> Fixes: c49fcfcda8b5 ("mm/shmem: update file sealing comments and file checking") >> Fixes: b3749f174d68 ("mm: memfd_luo: allow preserving memfd") >> Cc: stable@vger.kernel.org >> Reported-by: Chris Mason >> Reviewed-by: Rik van Riel >> Reviewed-by: Mike Rapoport (Microsoft) >> Signed-off-by: David Hildenbrand (Arm) > > Maybe you missed my email on v1 [0], so here's another try: I did miss it! Thanks for pointing that out again. > > Why not update shmem_file() to do this check instead? Having a > shmem_file() that might not always return the right result on a file > seems like a landmine. The current fix focuses on the sealing check indeed. > > For example, you missed check_memfd_seals() in udmabuf or check_memfd_seals() calls memfd_fcntl() where memfd_get_seals() would now properly reject it. > memfd_pin_folios() in gup. memfd_pin_folios() with sealing is a head-scratcher in general. I don't think it would be a problem here, though. The bigger question is if that path should just reject coda files as you point out below. I think yes. > Both want to only look at "proper" shmem > files, and should not do any work on a Coda file with a shmem mapping. > Plus, someone later down the line might use shmem_file() without knowing > this subtle problem, and we probably would miss it in review. > > For callers that only care about the mapping should then use > shmem_mapping(). I am not sure if there are any. Perhaps that would be > collapse_file()? But TBH I don't really understand khugepaged all that > much. All other callers of shmem_file() seem to care about the actual > inode. Looks like collapse_file() would then use shmem_mapping() for the time being. God this is confusing. What about the following replacement: >From 6e3c4f92a651e36ed1ae8d7659d2ca08543fa430 Mon Sep 17 00:00:00 2001 From: "David Hildenbrand (Arm)" Date: Sat, 24 Oct 2026 21:08:33 +0200 Subject: [PATCH] mm/shmem: make shmem_file() check the file's inode shmem_file() only checks if file->f_mapping is a shmem mapping. This mapping can belong to another file, however, as with a Coda file backed by a tmpfs container after mmap(). So it does not guarantee that the file inode is an actual shmem inode. Seal handling and memfd preservation use this check before accessing shmem inode state, so they can access an inode of the wrong type. Let's make shmem_file() check the inode's file operations instead, to identify shmem inodes reliably. This will identify regular shmem inodes, including memfds and ordinary tmpfs files, independently of wrapper file operations. Use shmem_mapping() in collapse_file() instead to not change the behavior of collapse_file(): it operates on the page cache and uses mapping->host for shmem inode operations. Focusing on the file does not seem relevant. Fixes: c49fcfcda8b5 ("mm/shmem: update file sealing comments and file checking") Fixes: b3749f174d68 ("mm: memfd_luo: allow preserving memfd") Cc: stable@vger.kernel.org Reported-by: Chris Mason Suggested-by: Pratyush Yadav Signed-off-by: David Hildenbrand (Arm) --- include/linux/shmem_fs.h | 15 ++++++--------- mm/khugepaged.c | 2 +- mm/shmem.c | 5 +++++ 3 files changed, 12 insertions(+), 10 deletions(-) diff --git a/include/linux/shmem_fs.h b/include/linux/shmem_fs.h index a7c7a96a7cbf..f89b4b151133 100644 --- a/include/linux/shmem_fs.h +++ b/include/linux/shmem_fs.h @@ -117,8 +117,14 @@ extern unsigned long shmem_get_unmapped_area(struct file *, unsigned long addr, unsigned long len, unsigned long pgoff, unsigned long flags); extern int shmem_lock(struct file *file, int lock, struct ucounts *ucounts); #ifdef CONFIG_SHMEM +bool shmem_file(struct file *file); bool shmem_mapping(const struct address_space *mapping); #else +static inline bool shmem_file(struct file *file) +{ + return false; +} + static inline bool shmem_mapping(const struct address_space *mapping) { return false; @@ -193,15 +199,6 @@ static inline struct page *shmem_read_mapping_page( mapping_gfp_mask(mapping)); } -static inline bool shmem_file(struct file *file) -{ - if (!IS_ENABLED(CONFIG_SHMEM)) - return false; - if (!file || !file->f_mapping) - return false; - return shmem_mapping(file->f_mapping); -} - /* Must be called with inode lock taken exclusive. */ static inline void shmem_freeze(struct inode *inode, bool freeze) { diff --git a/mm/khugepaged.c b/mm/khugepaged.c index 4b71d010b2af..3a6f4b7b2845 100644 --- a/mm/khugepaged.c +++ b/mm/khugepaged.c @@ -2230,6 +2230,7 @@ static enum scan_result collapse_file(struct mm_struct *mm, unsigned long addr, struct file *file, pgoff_t start, struct collapse_control *cc) { struct address_space *mapping = file->f_mapping; + const bool is_shmem = shmem_mapping(mapping); struct page *dst; struct folio *folio, *tmp, *new_folio; unsigned long new_pfn = -1; @@ -2238,7 +2239,6 @@ static enum scan_result collapse_file(struct mm_struct *mm, unsigned long addr, XA_STATE_ORDER(xas, &mapping->i_pages, start, HPAGE_PMD_ORDER); enum scan_result result = SCAN_SUCCEED; int nr_none = 0; - bool is_shmem = shmem_file(file); /* * MADV_COLLAPSE ignores shmem huge config, so do not check shmem diff --git a/mm/shmem.c b/mm/shmem.c index af21a89abb0f..421ba066995c 100644 --- a/mm/shmem.c +++ b/mm/shmem.c @@ -275,6 +275,11 @@ static const struct vm_operations_struct shmem_vm_ops; static const struct vm_operations_struct shmem_anon_vm_ops; static struct file_system_type shmem_fs_type; +bool shmem_file(struct file *file) +{ + return file && file_inode(file)->i_fop == &shmem_file_operations; +} + bool shmem_mapping(const struct address_space *mapping) { return mapping->a_ops == &shmem_aops; -- 2.43.0 -- Cheers, David