Re: [PATCH v2 03/47] mm/memfd: check for real shmem inodes before accessing shmem state

[David Hildenbrand (Arm)]

On 10/30/26 13:29, Pratyush Yadav wrote:
> On Fri, Oct 30 2026, David Hildenbrand (Arm) wrote:
> 
>> On 10/30/26 12:33, Pratyush Yadav wrote:
>>> Hi David,
>>>
> [...]
>>> 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.
> 
> Yes...
> 
>>
>> What about the following replacement:
>>
>>
>> From 6e3c4f92a651e36ed1ae8d7659d2ca08543fa430 Mon Sep 17 00:00:00 2001
>> From: "David Hildenbrand (Arm)" <david@xxxxxxxxxx>
>> 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@xxxxxxxxxxxxxxx
>> Reported-by: Chris Mason <mason@xxxxxxxxxx>
>> Suggested-by: Pratyush Yadav <pratyush@xxxxxxxxxx>
>> Signed-off-by: David Hildenbrand (Arm) <david@xxxxxxxxxx>
> 
> Looks great, thanks!
> 
> Reviewed-by: Pratyush Yadav (Google) <pratyush@xxxxxxxxxx>

Thanks!

@Mike, @Rik, Ill drop your RB, so please re-review.

-- 
Cheers,

David