* Re: [Bug 217238] New: Creating shared read-only map is denied after add write seal to a memfd
[not found] <bug-217238-27@https.bugzilla.kernel.org/>
@ 2023-03-24 20:36 ` Andrew Morton
2023-03-25 14:51 ` Lorenzo Stoakes
0 siblings, 1 reply; 5+ messages in thread
From: Andrew Morton @ 2023-03-24 20:36 UTC (permalink / raw)
To: linux-mm, David Herrmann, yshuiv7; +Cc: bugzilla-daemon
(switched to email. Please respond via emailed reply-to-all, not via the
bugzilla web interface).
On Fri, 24 Mar 2023 03:34:23 +0000 bugzilla-daemon@kernel.org wrote:
> https://bugzilla.kernel.org/show_bug.cgi?id=217238
>
> Bug ID: 217238
> Summary: Creating shared read-only map is denied after add
> write seal to a memfd
> Product: Memory Management
> Version: 2.5
> Kernel Version: 6.2.8
> Hardware: All
> OS: Linux
> Tree: Mainline
> Status: NEW
> Severity: normal
> Priority: P1
> Component: Other
> Assignee: akpm@linux-foundation.org
> Reporter: yshuiv7@gmail.com
> Regression: No
>
> Test case:
>
> int main() {
> int fd = memfd_create("test", MFD_ALLOW_SEALING);
> write(fd, "test", 4);
> fcntl(fd, F_ADD_SEALS, F_SEAL_WRITE);
>
> void *ret = mmap(NULL, 4, PROT_READ, MAP_SHARED, fd, 0);
> }
>
> This fails with EPERM. This is in contradiction with what's described in the
> documentation of F_SEAL_WRITE.
>
> --
> You may reply to this email to add a comment.
>
> You are receiving this mail because:
> You are the assignee for the bug.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Bug 217238] New: Creating shared read-only map is denied after add write seal to a memfd
2023-03-24 20:36 ` [Bug 217238] New: Creating shared read-only map is denied after add write seal to a memfd Andrew Morton
@ 2023-03-25 14:51 ` Lorenzo Stoakes
2023-03-30 19:24 ` Lorenzo Stoakes
0 siblings, 1 reply; 5+ messages in thread
From: Lorenzo Stoakes @ 2023-03-25 14:51 UTC (permalink / raw)
To: Andrew Morton; +Cc: linux-mm, David Herrmann, yshuiv7, bugzilla-daemon
On Fri, Mar 24, 2023 at 01:36:46PM -0700, Andrew Morton wrote:
> (switched to email. Please respond via emailed reply-to-all, not via the
> bugzilla web interface).
>
> On Fri, 24 Mar 2023 03:34:23 +0000 bugzilla-daemon@kernel.org wrote:
>
> > https://bugzilla.kernel.org/show_bug.cgi?id=217238
> >
> > Bug ID: 217238
> > Summary: Creating shared read-only map is denied after add
> > write seal to a memfd
> > Product: Memory Management
> > Version: 2.5
> > Kernel Version: 6.2.8
> > Hardware: All
> > OS: Linux
> > Tree: Mainline
> > Status: NEW
> > Severity: normal
> > Priority: P1
> > Component: Other
> > Assignee: akpm@linux-foundation.org
> > Reporter: yshuiv7@gmail.com
> > Regression: No
> >
> > Test case:
> >
> > int main() {
> > int fd = memfd_create("test", MFD_ALLOW_SEALING);
> > write(fd, "test", 4);
> > fcntl(fd, F_ADD_SEALS, F_SEAL_WRITE);
> >
> > void *ret = mmap(NULL, 4, PROT_READ, MAP_SHARED, fd, 0);
> > }
> >
> > This fails with EPERM. This is in contradiction with what's described in the
> > documentation of F_SEAL_WRITE.
> >
> > --
> > You may reply to this email to add a comment.
> >
> > You are receiving this mail because:
> > You are the assignee for the bug.
>
This issue seems to be the result of the use of the memfd's shmem region's
page cache object (struct address_space)'s i_mmap_writable field to denote
whether it is write-sealed.
The kernel assumes that a VM_SHARED mapping might become writable at any
time via mprotect(), therefore treats VM_SHARED mappings as if they were
writable as far as i_mmap_writable is concerned (this field's primary use
is to determine whether, for architectures that require it, flushing must
occur if this is set to avoid aliasing, see filemap_read() for example).
In theory we could convert all such checks to VM_SHARED | VM_WRITE
(importantly including on fork) and then update mprotect() to check
mapping_map_writable() if a user tries to make unwritable memory
writable.
I suspect however there are reasons relating to locking that make it
unreasonable to try to do this, but I may be mistaken (others might have
some insight on this). I also see some complexity around this in the
security checks on marking shared writable mappings executable (e.g. in
mmap_violation_check()).
In any case, it doesn't really make much sense to have a write-sealed
shared mapping, since you're essentially saying 'nothing _at all_ can write
to this' so it may as well be private. The semantics are unfortunate here,
the memory will still be shared read-only by MAP_PRIVATE mappings.
A better choice here might be F_SEAL_FUTURE_WRITE (available from kernel
>=5.1) which does permit shared read-only mappings as this is explicitly
checked for in seal_check_future_write() invoked from shmem_mmap().
Regardless, I think the conclusion is that this is not a bug, but rather
that the documentation needs to be updated.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Bug 217238] New: Creating shared read-only map is denied after add write seal to a memfd
2023-03-25 14:51 ` Lorenzo Stoakes
@ 2023-03-30 19:24 ` Lorenzo Stoakes
2023-03-30 20:47 ` Andy Lutomirski
0 siblings, 1 reply; 5+ messages in thread
From: Lorenzo Stoakes @ 2023-03-30 19:24 UTC (permalink / raw)
To: Andrew Morton
Cc: linux-mm, David Herrmann, yshuiv7, bugzilla-daemon, linux-api,
linux-man, Michael Kerrisk
On Sat, Mar 25, 2023 at 02:51:05PM +0000, Lorenzo Stoakes wrote:
> On Fri, Mar 24, 2023 at 01:36:46PM -0700, Andrew Morton wrote:
> > (switched to email. Please respond via emailed reply-to-all, not via the
> > bugzilla web interface).
> >
> > On Fri, 24 Mar 2023 03:34:23 +0000 bugzilla-daemon@kernel.org wrote:
> >
> > > https://bugzilla.kernel.org/show_bug.cgi?id=217238
> > >
> > > Bug ID: 217238
> > > Summary: Creating shared read-only map is denied after add
> > > write seal to a memfd
> > > Product: Memory Management
> > > Version: 2.5
> > > Kernel Version: 6.2.8
> > > Hardware: All
> > > OS: Linux
> > > Tree: Mainline
> > > Status: NEW
> > > Severity: normal
> > > Priority: P1
> > > Component: Other
> > > Assignee: akpm@linux-foundation.org
> > > Reporter: yshuiv7@gmail.com
> > > Regression: No
> > >
> > > Test case:
> > >
> > > int main() {
> > > int fd = memfd_create("test", MFD_ALLOW_SEALING);
> > > write(fd, "test", 4);
> > > fcntl(fd, F_ADD_SEALS, F_SEAL_WRITE);
> > >
> > > void *ret = mmap(NULL, 4, PROT_READ, MAP_SHARED, fd, 0);
> > > }
> > >
> > > This fails with EPERM. This is in contradiction with what's described in the
> > > documentation of F_SEAL_WRITE.
> > >
> > > --
> > > You may reply to this email to add a comment.
> > >
> > > You are receiving this mail because:
> > > You are the assignee for the bug.
> >
>
> This issue seems to be the result of the use of the memfd's shmem region's
> page cache object (struct address_space)'s i_mmap_writable field to denote
> whether it is write-sealed.
>
> The kernel assumes that a VM_SHARED mapping might become writable at any
> time via mprotect(), therefore treats VM_SHARED mappings as if they were
> writable as far as i_mmap_writable is concerned (this field's primary use
> is to determine whether, for architectures that require it, flushing must
> occur if this is set to avoid aliasing, see filemap_read() for example).
>
> In theory we could convert all such checks to VM_SHARED | VM_WRITE
> (importantly including on fork) and then update mprotect() to check
> mapping_map_writable() if a user tries to make unwritable memory
> writable.
>
> I suspect however there are reasons relating to locking that make it
> unreasonable to try to do this, but I may be mistaken (others might have
> some insight on this). I also see some complexity around this in the
> security checks on marking shared writable mappings executable (e.g. in
> mmap_violation_check()).
>
> In any case, it doesn't really make much sense to have a write-sealed
> shared mapping, since you're essentially saying 'nothing _at all_ can write
> to this' so it may as well be private. The semantics are unfortunate here,
> the memory will still be shared read-only by MAP_PRIVATE mappings.
>
> A better choice here might be F_SEAL_FUTURE_WRITE (available from kernel
> >=5.1) which does permit shared read-only mappings as this is explicitly
> checked for in seal_check_future_write() invoked from shmem_mmap().
>
> Regardless, I think the conclusion is that this is not a bug, but rather
> that the documentation needs to be updated.
>
Adding docs people to cc list (sorry didn't think to do this in first
reply).
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Bug 217238] New: Creating shared read-only map is denied after add write seal to a memfd
2023-03-30 19:24 ` Lorenzo Stoakes
@ 2023-03-30 20:47 ` Andy Lutomirski
2023-03-30 21:46 ` Lorenzo Stoakes
0 siblings, 1 reply; 5+ messages in thread
From: Andy Lutomirski @ 2023-03-30 20:47 UTC (permalink / raw)
To: Lorenzo Stoakes
Cc: Andrew Morton, linux-mm, David Herrmann, yshuiv7,
bugzilla-daemon, linux-api, linux-man, Michael Kerrisk,
Andy Lutomirski
> On Mar 30, 2023, at 12:25 PM, Lorenzo Stoakes <lstoakes@gmail.com> wrote:
>
> On Sat, Mar 25, 2023 at 02:51:05PM +0000, Lorenzo Stoakes wrote:
>>> On Fri, Mar 24, 2023 at 01:36:46PM -0700, Andrew Morton wrote:
>>> (switched to email. Please respond via emailed reply-to-all, not via the
>>> bugzilla web interface).
>>>
>>>> On Fri, 24 Mar 2023 03:34:23 +0000 bugzilla-daemon@kernel.org wrote:
>>>
>>>> https://bugzilla.kernel.org/show_bug.cgi?id=217238
>>>>
>>>> Bug ID: 217238
>>>> Summary: Creating shared read-only map is denied after add
>>>> write seal to a memfd
>>>> Product: Memory Management
>>>> Version: 2.5
>>>> Kernel Version: 6.2.8
>>>> Hardware: All
>>>> OS: Linux
>>>> Tree: Mainline
>>>> Status: NEW
>>>> Severity: normal
>>>> Priority: P1
>>>> Component: Other
>>>> Assignee: akpm@linux-foundation.org
>>>> Reporter: yshuiv7@gmail.com
>>>> Regression: No
>>>>
>>>> Test case:
>>>>
>>>> int main() {
>>>> int fd = memfd_create("test", MFD_ALLOW_SEALING);
>>>> write(fd, "test", 4);
>>>> fcntl(fd, F_ADD_SEALS, F_SEAL_WRITE);
>>>>
>>>> void *ret = mmap(NULL, 4, PROT_READ, MAP_SHARED, fd, 0);
>>>> }
>>>>
>>>> This fails with EPERM. This is in contradiction with what's described in the
>>>> documentation of F_SEAL_WRITE.
>>>>
>>>> --
>>>> You may reply to this email to add a comment.
>>>>
>>>> You are receiving this mail because:
>>>> You are the assignee for the bug.
>>>
>>
>> This issue seems to be the result of the use of the memfd's shmem region's
>> page cache object (struct address_space)'s i_mmap_writable field to denote
>> whether it is write-sealed.
>>
>> The kernel assumes that a VM_SHARED mapping might become writable at any
>> time via mprotect(), therefore treats VM_SHARED mappings as if they were
>> writable as far as i_mmap_writable is concerned (this field's primary use
>> is to determine whether, for architectures that require it, flushing must
>> occur if this is set to avoid aliasing, see filemap_read() for example).
>>
>> In theory we could convert all such checks to VM_SHARED | VM_WRITE
>> (importantly including on fork) and then update mprotect() to check
>> mapping_map_writable() if a user tries to make unwritable memory
>> writable.
>>
Unless I’m missing something, we have VM_MAYWRITE for almost exactly this purpose. Can’t we just make a shared mapping with both of these bits clear?
>> I suspect however there are reasons relating to locking that make it
>> unreasonable to try to do this, but I may be mistaken (others might have
>> some insight on this). I also see some complexity around this in the
>> security checks on marking shared writable mappings executable (e.g. in
>> mmap_violation_check()).
>>
>> In any case, it doesn't really make much sense to have a write-sealed
>> shared mapping, since you're essentially saying 'nothing _at all_ can write
>> to this' so it may as well be private. The semantics are unfortunate here,
>> the memory will still be shared read-only by MAP_PRIVATE mappings.
>>
>> A better choice here might be F_SEAL_FUTURE_WRITE (available from kernel
>>> =5.1) which does permit shared read-only mappings as this is explicitly
>> checked for in seal_check_future_write() invoked from shmem_mmap().
>>
>> Regardless, I think the conclusion is that this is not a bug, but rather
>> that the documentation needs to be updated.
>>
>
> Adding docs people to cc list (sorry didn't think to do this in first
> reply).
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Bug 217238] New: Creating shared read-only map is denied after add write seal to a memfd
2023-03-30 20:47 ` Andy Lutomirski
@ 2023-03-30 21:46 ` Lorenzo Stoakes
0 siblings, 0 replies; 5+ messages in thread
From: Lorenzo Stoakes @ 2023-03-30 21:46 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Andrew Morton, linux-mm, David Herrmann, yshuiv7,
bugzilla-daemon, linux-api, linux-man, Michael Kerrisk,
Andy Lutomirski
On Thu, Mar 30, 2023 at 01:47:48PM -0700, Andy Lutomirski wrote:
>
>
> > On Mar 30, 2023, at 12:25 PM, Lorenzo Stoakes <lstoakes@gmail.com> wrote:
> >
> > On Sat, Mar 25, 2023 at 02:51:05PM +0000, Lorenzo Stoakes wrote:
> >>> On Fri, Mar 24, 2023 at 01:36:46PM -0700, Andrew Morton wrote:
> >>> (switched to email. Please respond via emailed reply-to-all, not via the
> >>> bugzilla web interface).
> >>>
> >>>> On Fri, 24 Mar 2023 03:34:23 +0000 bugzilla-daemon@kernel.org wrote:
> >>>
> >>>> https://bugzilla.kernel.org/show_bug.cgi?id=217238
> >>>>
> >>>> Bug ID: 217238
> >>>> Summary: Creating shared read-only map is denied after add
> >>>> write seal to a memfd
> >>>> Product: Memory Management
> >>>> Version: 2.5
> >>>> Kernel Version: 6.2.8
> >>>> Hardware: All
> >>>> OS: Linux
> >>>> Tree: Mainline
> >>>> Status: NEW
> >>>> Severity: normal
> >>>> Priority: P1
> >>>> Component: Other
> >>>> Assignee: akpm@linux-foundation.org
> >>>> Reporter: yshuiv7@gmail.com
> >>>> Regression: No
> >>>>
> >>>> Test case:
> >>>>
> >>>> int main() {
> >>>> int fd = memfd_create("test", MFD_ALLOW_SEALING);
> >>>> write(fd, "test", 4);
> >>>> fcntl(fd, F_ADD_SEALS, F_SEAL_WRITE);
> >>>>
> >>>> void *ret = mmap(NULL, 4, PROT_READ, MAP_SHARED, fd, 0);
> >>>> }
> >>>>
> >>>> This fails with EPERM. This is in contradiction with what's described in the
> >>>> documentation of F_SEAL_WRITE.
> >>>>
> >>>> --
> >>>> You may reply to this email to add a comment.
> >>>>
> >>>> You are receiving this mail because:
> >>>> You are the assignee for the bug.
> >>>
> >>
> >> This issue seems to be the result of the use of the memfd's shmem region's
> >> page cache object (struct address_space)'s i_mmap_writable field to denote
> >> whether it is write-sealed.
> >>
> >> The kernel assumes that a VM_SHARED mapping might become writable at any
> >> time via mprotect(), therefore treats VM_SHARED mappings as if they were
> >> writable as far as i_mmap_writable is concerned (this field's primary use
> >> is to determine whether, for architectures that require it, flushing must
> >> occur if this is set to avoid aliasing, see filemap_read() for example).
> >>
> >> In theory we could convert all such checks to VM_SHARED | VM_WRITE
> >> (importantly including on fork) and then update mprotect() to check
> >> mapping_map_writable() if a user tries to make unwritable memory
> >> writable.
> >>
>
> Unless I’m missing something, we have VM_MAYWRITE for almost exactly this purpose. Can’t we just make a shared mapping with both of these bits clear?
>
That's a good point, and there's definitely quite a few places where
VM_SHARED is simply taken to imply writable which is a little irksome,
however sprinkling some VM_MAYWRITE checks in these places would resolve
that.
Let me take a look into this and perhaps spin up a RFC to iron out the
details if this is indeed viable.
> >> I suspect however there are reasons relating to locking that make it
> >> unreasonable to try to do this, but I may be mistaken (others might have
> >> some insight on this). I also see some complexity around this in the
> >> security checks on marking shared writable mappings executable (e.g. in
> >> mmap_violation_check()).
> >>
> >> In any case, it doesn't really make much sense to have a write-sealed
> >> shared mapping, since you're essentially saying 'nothing _at all_ can write
> >> to this' so it may as well be private. The semantics are unfortunate here,
> >> the memory will still be shared read-only by MAP_PRIVATE mappings.
> >>
> >> A better choice here might be F_SEAL_FUTURE_WRITE (available from kernel
> >>> =5.1) which does permit shared read-only mappings as this is explicitly
> >> checked for in seal_check_future_write() invoked from shmem_mmap().
> >>
> >> Regardless, I think the conclusion is that this is not a bug, but rather
> >> that the documentation needs to be updated.
> >>
> >
> > Adding docs people to cc list (sorry didn't think to do this in first
> > reply).
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-03-30 21:46 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <bug-217238-27@https.bugzilla.kernel.org/>
2023-03-24 20:36 ` [Bug 217238] New: Creating shared read-only map is denied after add write seal to a memfd Andrew Morton
2023-03-25 14:51 ` Lorenzo Stoakes
2023-03-30 19:24 ` Lorenzo Stoakes
2023-03-30 20:47 ` Andy Lutomirski
2023-03-30 21:46 ` Lorenzo Stoakes
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.