Skip to content
Snippets Groups Projects
  • Lorenzo Stoakes's avatar
    464770df
    mm: reinstate ability to map write-sealed memfd mappings read-only · 464770df
    Lorenzo Stoakes authored
    commit 8ec396d05d1b737c87311fb7311f753b02c2a6b1 upstream.
    
    Patch series "mm: reinstate ability to map write-sealed memfd mappings
    read-only".
    
    In commit 15897894 ("mm: perform the mapping_map_writable() check
    after call_mmap()") (and preceding changes in the same series) it became
    possible to mmap() F_SEAL_WRITE sealed memfd mappings read-only.
    
    Commit 5de19506 ("mm: resolve faulty mmap_region() error path
    behaviour") unintentionally undid this logic by moving the
    mapping_map_writable() check before the shmem_mmap() hook is invoked,
    thereby regressing this change.
    
    This series reworks how we both permit write-sealed mappings being mapped
    read-only and disallow mprotect() from undoing the write-seal, fixing this
    regression.
    
    We also add a regression test to ensure that we do not accidentally
    regress this in future.
    
    Thanks to Julian Orth for reporting this regression.
    
    
    This patch (of 2):
    
    In commit 15897894 ("mm: perform the mapping_map_writable() check
    after call_mmap()") (and preceding changes in the same series) it became
    possible to mmap() F_SEAL_WRITE sealed memfd mappings read-only.
    
    This was previously unnecessarily disallowed, despite the man page
    documentation indicating that it would be, thereby limiting the usefulness
    of F_SEAL_WRITE logic.
    
    We fixed this by adapting logic that existed for the F_SEAL_FUTURE_WRITE
    seal (one which disallows future writes to the memfd) to also be used for
    F_SEAL_WRITE.
    
    For background - the F_SEAL_FUTURE_WRITE seal clears VM_MAYWRITE for a
    read-only mapping to disallow mprotect() from overriding the seal - an
    operation performed by seal_check_write(), invoked from shmem_mmap(), the
    f_op->mmap() hook used by shmem mappings.
    
    By extending this to F_SEAL_WRITE and critically - checking
    mapping_map_writable() to determine if we may map the memfd AFTER we
    invoke shmem_mmap() - the desired logic becomes possible.  This is because
    mapping_map_writable() explicitly checks for VM_MAYWRITE, which we will
    have cleared.
    
    Commit 5de19506 ("mm: resolve faulty mmap_region() error path
    behaviour") unintentionally undid this logic by moving the
    mapping_map_writable() check before the shmem_mmap() hook is invoked,
    thereby regressing this change.
    
    We reinstate this functionality by moving the check out of shmem_mmap()
    and instead performing it in do_mmap() at the point at which VMA flags are
    being determined, which seems in any case to be a more appropriate place
    in which to make this determination.
    
    In order to achieve this we rework memfd seal logic to allow us access to
    this information using existing logic and eliminate the clearing of
    VM_MAYWRITE from seal_check_write() which we are performing in do_mmap()
    instead.
    
    Link: https://lkml.kernel.org/r/99fc35d2c62bd2e05571cf60d9f8b843c56069e0.1732804776.git.lorenzo.stoakes@oracle.com
    
    
    Fixes: 5de19506 ("mm: resolve faulty mmap_region() error path behaviour")
    Signed-off-by: default avatarLorenzo Stoakes <lorenzo.stoakes@oracle.com>
    Reported-by: default avatarJulian Orth <ju.orth@gmail.com>
    Closes: https://lore.kernel.org/all/CAHijbEUMhvJTN9Xw1GmbM266FXXv=U7s4L_Jem5x3AaPZxrYpQ@mail.gmail.com/
    
    
    Cc: Jann Horn <jannh@google.com>
    Cc: Liam R. Howlett <Liam.Howlett@Oracle.com>
    Cc: Linus Torvalds <torvalds@linux-foundation.org>
    Cc: Shuah Khan <shuah@kernel.org>
    Cc: Vlastimil Babka <vbabka@suse.cz>
    Cc: <stable@vger.kernel.org>
    Signed-off-by: default avatarAndrew Morton <akpm@linux-foundation.org>
    Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
    464770df
    History
    mm: reinstate ability to map write-sealed memfd mappings read-only
    Lorenzo Stoakes authored
    commit 8ec396d05d1b737c87311fb7311f753b02c2a6b1 upstream.
    
    Patch series "mm: reinstate ability to map write-sealed memfd mappings
    read-only".
    
    In commit 15897894 ("mm: perform the mapping_map_writable() check
    after call_mmap()") (and preceding changes in the same series) it became
    possible to mmap() F_SEAL_WRITE sealed memfd mappings read-only.
    
    Commit 5de19506 ("mm: resolve faulty mmap_region() error path
    behaviour") unintentionally undid this logic by moving the
    mapping_map_writable() check before the shmem_mmap() hook is invoked,
    thereby regressing this change.
    
    This series reworks how we both permit write-sealed mappings being mapped
    read-only and disallow mprotect() from undoing the write-seal, fixing this
    regression.
    
    We also add a regression test to ensure that we do not accidentally
    regress this in future.
    
    Thanks to Julian Orth for reporting this regression.
    
    
    This patch (of 2):
    
    In commit 15897894 ("mm: perform the mapping_map_writable() check
    after call_mmap()") (and preceding changes in the same series) it became
    possible to mmap() F_SEAL_WRITE sealed memfd mappings read-only.
    
    This was previously unnecessarily disallowed, despite the man page
    documentation indicating that it would be, thereby limiting the usefulness
    of F_SEAL_WRITE logic.
    
    We fixed this by adapting logic that existed for the F_SEAL_FUTURE_WRITE
    seal (one which disallows future writes to the memfd) to also be used for
    F_SEAL_WRITE.
    
    For background - the F_SEAL_FUTURE_WRITE seal clears VM_MAYWRITE for a
    read-only mapping to disallow mprotect() from overriding the seal - an
    operation performed by seal_check_write(), invoked from shmem_mmap(), the
    f_op->mmap() hook used by shmem mappings.
    
    By extending this to F_SEAL_WRITE and critically - checking
    mapping_map_writable() to determine if we may map the memfd AFTER we
    invoke shmem_mmap() - the desired logic becomes possible.  This is because
    mapping_map_writable() explicitly checks for VM_MAYWRITE, which we will
    have cleared.
    
    Commit 5de19506 ("mm: resolve faulty mmap_region() error path
    behaviour") unintentionally undid this logic by moving the
    mapping_map_writable() check before the shmem_mmap() hook is invoked,
    thereby regressing this change.
    
    We reinstate this functionality by moving the check out of shmem_mmap()
    and instead performing it in do_mmap() at the point at which VMA flags are
    being determined, which seems in any case to be a more appropriate place
    in which to make this determination.
    
    In order to achieve this we rework memfd seal logic to allow us access to
    this information using existing logic and eliminate the clearing of
    VM_MAYWRITE from seal_check_write() which we are performing in do_mmap()
    instead.
    
    Link: https://lkml.kernel.org/r/99fc35d2c62bd2e05571cf60d9f8b843c56069e0.1732804776.git.lorenzo.stoakes@oracle.com
    
    
    Fixes: 5de19506 ("mm: resolve faulty mmap_region() error path behaviour")
    Signed-off-by: default avatarLorenzo Stoakes <lorenzo.stoakes@oracle.com>
    Reported-by: default avatarJulian Orth <ju.orth@gmail.com>
    Closes: https://lore.kernel.org/all/CAHijbEUMhvJTN9Xw1GmbM266FXXv=U7s4L_Jem5x3AaPZxrYpQ@mail.gmail.com/
    
    
    Cc: Jann Horn <jannh@google.com>
    Cc: Liam R. Howlett <Liam.Howlett@Oracle.com>
    Cc: Linus Torvalds <torvalds@linux-foundation.org>
    Cc: Shuah Khan <shuah@kernel.org>
    Cc: Vlastimil Babka <vbabka@suse.cz>
    Cc: <stable@vger.kernel.org>
    Signed-off-by: default avatarAndrew Morton <akpm@linux-foundation.org>
    Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>