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

[David Hildenbrand (Arm)]

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@xxxxxxxxxxxxxxx
>> Reported-by: Chris Mason <mason@xxxxxxxxxx>
>> Reviewed-by: Rik van Riel <riel@xxxxxxxxxxx>
>> Reviewed-by: Mike Rapoport (Microsoft) <rppt@xxxxxxxxxx>
>> Signed-off-by: David Hildenbrand (Arm) <david@xxxxxxxxxx>
> 
> 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: