All of lore.kernel.org
 help / color / mirror / Atom feed
From: Isaac Manjarres <isaacmanjarres@google.com>
To: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
Cc: gregkh@linuxfoundation.org, aliceryhl@google.com,
	surenb@google.com, stable@vger.kernel.org,
	kernel-team@android.com
Subject: Re: [PATCH 5.10.y 0/4] Backport series: "permit write-sealed memfd read-only shared mappings"
Date: Wed, 30 Jul 2025 10:23:54 -0700	[thread overview]
Message-ID: <aIpVKpqXmfuITxf-@google.com> (raw)
In-Reply-To: <c99af418-946d-40c4-9594-36943d8c72bf@lucifer.local>

On Wed, Jul 30, 2025 at 03:21:48PM +0100, Lorenzo Stoakes wrote:
> Hi Isaac,
> 
> Thanks very much for all your hard work on this!
> 
> I'll respond to this one, but this is a general comment for all the
> backports.
> 
> I just wonder if this is what backporting is for - really this is a new
> feature, yes the documentation is incorrect, which is why I made the
> change, but it's sort of debatable if that's a bug or a new feature.

Hi Lorenzo,

Thanks for your feedback on this. That's a good question. The rationale
that I had when backporting these fixes was: The original intent of
F_SEAL_WRITE was to just prevent any writes to region after it had
been write-sealed, and that the existing behavior on older kernels
may have been a result of oversight or just an accident, making it a
bug. So fixing it would be fixing a bug that has been around for a
while. I hadn't really thought of it as a new feature.

I also learned recently that, at least for Android, there were attempts
in the past to map write-sealed memfds as read-only and shared, which
failed. This was surprising to developers, and they ended up working
around it. I'm not sure why it wasn't reported then, but this being
a surprise to multiple developers makes it feel like more of a bug
to me on older kernels.

>
> Having said that, I'm not against you doing this, just wondering about
> that.
> 
> Also - what kind of testing have you do on these series?
I did the following tests:

1. I have a unit test that tries to map write-sealed memfds as
read-only and shared. I verified that this works for each kernel version
that this series is being applied to.

2. Android devices do use memfds as well, so I did try these patches out
on a device running each kernel version, and tried boot testing, using
several apps/games. I was looking for functional failures in these
scenarios but didn't encounter any.

Do you have any other recommendations of what I should test?

Thanks,
Isaac

> Cheers, Lorenzo
> 
> On Tue, Jul 29, 2025 at 06:53:58PM -0700, Isaac J. Manjarres wrote:
> > Hello,
> >
> > Until kernel version 6.7, a write-sealed memfd could not be mapped as
> > shared and read-only. This was clearly a bug, and was not inline with
> > the description of F_SEAL_WRITE in the man page for fcntl()[1].
> >
> > Lorenzo's series [2] fixed that issue and was merged in kernel version
> > 6.7, but was not backported to older kernels. So, this issue is still
> > present on kernels 5.4, 5.10, 5.15, 6.1, and 6.6.
> >
> > This series consists of backports of two of Lorenzo's series [2] and
> > [3].
> >
> > Note: for [2], I dropped the last patch in that series, since it
> > wouldn't make sense to apply it due to [4] being part of this tree. In
> > lieu of that, I backported [3] to ultimately allow write-sealed memfds
> > to be mapped as read-only.
> >
> > [1] https://man7.org/linux/man-pages/man2/fcntl.2.html
> > [2] https://lore.kernel.org/all/913628168ce6cce77df7d13a63970bae06a526e0.1697116581.git.lstoakes@gmail.com/T/#m28fbfb0d5727e5693e54a7fb2e0c9ac30e95eca5
> > [3] https://lkml.kernel.org/r/99fc35d2c62bd2e05571cf60d9f8b843c56069e0.1732804776.git.lorenzo.stoakes@oracle.com
> > [4] https://lore.kernel.org/all/6e0becb36d2f5472053ac5d544c0edfe9b899e25.1730224667.git.lorenzo.stoakes@oracle.com/T/#u
> >
> > Lorenzo Stoakes (4):
> >   mm: drop the assumption that VM_SHARED always implies writable
> >   mm: update memfd seal write check to include F_SEAL_WRITE
> >   mm: reinstate ability to map write-sealed memfd mappings read-only
> >   selftests/memfd: add test for mapping write-sealed memfd read-only
> >
> >  fs/hugetlbfs/inode.c                       |  2 +-
> >  include/linux/fs.h                         |  4 +-
> >  include/linux/memfd.h                      | 14 ++++
> >  include/linux/mm.h                         | 80 +++++++++++++++-------
> >  kernel/fork.c                              |  2 +-
> >  mm/filemap.c                               |  2 +-
> >  mm/madvise.c                               |  2 +-
> >  mm/memfd.c                                 |  2 +-
> >  mm/mmap.c                                  | 10 ++-
> >  mm/shmem.c                                 |  2 +-
> >  tools/testing/selftests/memfd/memfd_test.c | 43 ++++++++++++
> >  11 files changed, 129 insertions(+), 34 deletions(-)
> >
> > --
> > 2.50.1.552.g942d659e1b-goog
> >

  reply	other threads:[~2025-07-30 17:24 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-07-30  1:53 [PATCH 5.10.y 0/4] Backport series: "permit write-sealed memfd read-only shared mappings" Isaac J. Manjarres
2025-07-30  1:53 ` [PATCH 5.10.y 1/4] mm: drop the assumption that VM_SHARED always implies writable Isaac J. Manjarres
2025-07-30 16:29   ` Sasha Levin
2025-08-24  7:57   ` Patch "mm: drop the assumption that VM_SHARED always implies writable" has been added to the 5.10-stable tree gregkh
2025-07-30  1:54 ` [PATCH 5.10.y 2/4] mm: update memfd seal write check to include F_SEAL_WRITE Isaac J. Manjarres
2025-07-30 16:29   ` Sasha Levin
2025-08-24  7:57   ` Patch "mm: update memfd seal write check to include F_SEAL_WRITE" has been added to the 5.10-stable tree gregkh
2025-07-30  1:54 ` [PATCH 5.10.y 3/4] mm: reinstate ability to map write-sealed memfd mappings read-only Isaac J. Manjarres
2025-07-30 16:29   ` Sasha Levin
2025-08-24  7:57   ` Patch "mm: reinstate ability to map write-sealed memfd mappings read-only" has been added to the 5.10-stable tree gregkh
2025-07-30  1:54 ` [PATCH 5.10.y 4/4] selftests/memfd: add test for mapping write-sealed memfd read-only Isaac J. Manjarres
2025-07-30 16:29   ` Sasha Levin
2025-07-30 14:21 ` [PATCH 5.10.y 0/4] Backport series: "permit write-sealed memfd read-only shared mappings" Lorenzo Stoakes
2025-07-30 17:23   ` Isaac Manjarres [this message]
2025-07-30 19:34     ` Lorenzo Stoakes
2025-07-30 22:26       ` Isaac Manjarres
2025-07-31  4:40         ` Lorenzo Stoakes
2025-07-31  4:58           ` Greg KH
2025-07-31  5:02             ` Lorenzo Stoakes
2025-08-06 16:54             ` Isaac Manjarres

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=aIpVKpqXmfuITxf-@google.com \
    --to=isaacmanjarres@google.com \
    --cc=aliceryhl@google.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=kernel-team@android.com \
    --cc=lorenzo.stoakes@oracle.com \
    --cc=stable@vger.kernel.org \
    --cc=surenb@google.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.