linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Lorenzo Stoakes <lstoakes@gmail.com>
To: linux-mm@kvack.org, linux-kernel@vger.kernel.org,
	Andrew Morton <akpm@linux-foundation.org>
Cc: Matthew Wilcox <willy@infradead.org>,
	Mike Kravetz <mike.kravetz@oracle.com>,
	Muchun Song <muchun.song@linux.dev>,
	Alexander Viro <viro@zeniv.linux.org.uk>,
	Christian Brauner <brauner@kernel.org>,
	Andy Lutomirski <luto@amacapital.net>,
	linux-fsdevel@vger.kernel.org, Jan Kara <jack@suse.cz>,
	Hugh Dickins <hughd@google.com>,
	Lorenzo Stoakes <lstoakes@gmail.com>
Subject: [PATCH v2 0/3] permit write-sealed memfd read-only shared mappings
Date: Sun, 30 Apr 2023 23:26:04 +0100	[thread overview]
Message-ID: <cover.1682890156.git.lstoakes@gmail.com> (raw)

The man page for fcntl() describing memfd file seals states the following
about F_SEAL_WRITE:-

    Furthermore, trying to create new shared, writable memory-mappings via
    mmap(2) will also fail with EPERM.

With emphasis on _writable_. In turns out in fact that currently the kernel
simply disallows _all_ new shared memory mappings for a memfd with
F_SEAL_WRITE applied, rendering this documentation inaccurate.

This matters because users are therefore unable to obtain a shared mapping
to a memfd after write sealing altogether, which limits their
usefulness. This was reported in the discussion thread [1] originating from
a bug report [2].

This is a product of both using the struct address_space->i_mmap_writable
atomic counter to determine whether writing may be permitted, and the
kernel adjusting this counter when _any_ VM_SHARED mapping is performed.

It seems sensible that we should only update this mapping if VM_MAYWRITE is
specified, i.e. whether it is possible that this mapping could at any point
be written to.

If we do so then all we need to do to permit write seals to function as
documented is to clear VM_MAYWRITE when mapping read-only. It turns out
this functionality already exists for F_SEAL_FUTURE_WRITE - we can
therefore simply adapt this logic to do the same for F_SEAL_WRITE.

The final change required is to invoke call_mmap() before
mapping_map_writable() - doing so ensures that the memfd-relevant
shmem_mmap() or hugetlbfs_file_mmap() custom mmap handlers will be called
before the writable test, enabling us to clear VM_MAYWRITE first.

Thanks to Andy Lutomirski for the suggestion!

[1]:https://lore.kernel.org/all/20230324133646.16101dfa666f253c4715d965@linux-foundation.org/
[2]:https://bugzilla.kernel.org/show_bug.cgi?id=217238

v2:
- Removed RFC tag.
- Correct incorrect goto pointed out by Jan.
- Reworded cover letter as suggested by Jan.

v1:
https://lore.kernel.org/all/cover.1680560277.git.lstoakes@gmail.com/

Lorenzo Stoakes (3):
  mm: drop the assumption that VM_SHARED always implies writable
  mm: update seal_check_[future_]write() to include F_SEAL_WRITE as well
  mm: perform the mapping_map_writable() check after call_mmap()

 fs/hugetlbfs/inode.c |  2 +-
 include/linux/fs.h   |  4 ++--
 include/linux/mm.h   | 24 ++++++++++++++++++------
 kernel/fork.c        |  2 +-
 mm/filemap.c         |  2 +-
 mm/madvise.c         |  2 +-
 mm/mmap.c            | 22 +++++++++++-----------
 mm/shmem.c           |  2 +-
 8 files changed, 36 insertions(+), 24 deletions(-)

--
2.40.1


             reply	other threads:[~2023-04-30 22:26 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-04-30 22:26 Lorenzo Stoakes [this message]
2023-04-30 22:26 ` [PATCH v2 1/3] mm: drop the assumption that VM_SHARED always implies writable Lorenzo Stoakes
2023-04-30 22:26 ` [PATCH v2 2/3] mm: update seal_check_[future_]write() to include F_SEAL_WRITE as well Lorenzo Stoakes
2023-04-30 22:26 ` [PATCH v2 3/3] mm: perform the mapping_map_writable() check after call_mmap() Lorenzo Stoakes
2023-05-01 19:02   ` Andy Lutomirski
2023-05-02  7:57     ` Lorenzo Stoakes
2023-05-16  5:52   ` kernel test robot
2023-10-07 20:07     ` Lorenzo Stoakes

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=cover.1682890156.git.lstoakes@gmail.com \
    --to=lstoakes@gmail.com \
    --cc=akpm@linux-foundation.org \
    --cc=brauner@kernel.org \
    --cc=hughd@google.com \
    --cc=jack@suse.cz \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=luto@amacapital.net \
    --cc=mike.kravetz@oracle.com \
    --cc=muchun.song@linux.dev \
    --cc=viro@zeniv.linux.org.uk \
    --cc=willy@infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).