* [PATCH] drivers/char/mem: mmap readonly MAP_SHARED-/dev/zero correctly
@ 2026-09-24 14:48 Lorenzo Stoakes (ARM)
2026-09-24 15:08 ` Greg Kroah-Hartman
2026-09-24 15:37 ` David Hildenbrand (Arm)
0 siblings, 2 replies; 8+ messages in thread
From: Lorenzo Stoakes (ARM) @ 2026-09-24 14:48 UTC (permalink / raw)
To: Andrew Morton, Liam R. Howlett, Vlastimil Babka, Jann Horn,
Pedro Falcato, David Hildenbrand, Mike Rapoport,
Suren Baghdasaryan, Michal Hocko, Arnd Bergmann,
Greg Kroah-Hartman
Cc: linux-mm, linux-kernel, Lance Yang, syzbot+c181d3198e98f8aef8b9,
Lorenzo Stoakes (ARM)
Rather surprisingly, opening /dev/zero read-only then mmap()'ing it
MAP_SHARED gets you true anonymous memory (albeit in a VMA with
non-NULL vma->vm_file).
This is a by-product of MAP_PRIVATE-/dev/zero being how anonymous memory
was mapped in Linux's distant past.
It happens because mmap_zero_prepare() gates on VMA_SHARED_BIT and when
mapping a read-only file MAP_SHARED, do_mmap() clears VMA_SHARED_BIT and
VMA_MAYWRITE_BIT.
The gating is incorrect - the (poorly named) VMA_MAYSHARE_BIT flag exists
explicitly to tell you if something was originally mapped MAP_SHARED.
So the fix is simple - gate on this instead.
This isn't exactly a common use case, but it's unexpected behaviour which
now causes an assert if CONFIG_DEBUG_VM is set.
While this bug has existed since the dawn of time for linux (or at least
since 2.6.12), it hasn't caused issues in the past, so while it's incorrect
behaviour, it doesn't seem necessary to backport that far.
The mapping is now accounted at mmap time and can fail with -ENOMEM under
strict overcommit, and read faults allocate folios. However this is normal
behaviour for a read-only shmem mapping.
Commit 93c0c8dc87f6 ("mm/rmap: use anon pgoff to track MAP_PRIVATE
file-backed anon folios") is the first patch at which the debug assert
fires, so target that instead.
Fixes: 93c0c8dc87f6 ("mm/rmap: use anon pgoff to track MAP_PRIVATE file-backed anon folios")
Reported-by: syzbot+c181d3198e98f8aef8b9@syzkaller.appspotmail.com
Closes: https://lore.kernel.org/linux-mm/6ab4ae75.80e1c6cc.1e8e5f.000d.GAE@google.com/
Signed-off-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>
---
drivers/char/mem.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/char/mem.c b/drivers/char/mem.c
index 63253d1de5d7..5b93c92c2cf1 100644
--- a/drivers/char/mem.c
+++ b/drivers/char/mem.c
@@ -503,7 +503,7 @@ static int mmap_zero_prepare(struct vm_area_desc *desc)
#ifndef CONFIG_MMU
return -ENOSYS;
#endif
- if (vma_desc_test(desc, VMA_SHARED_BIT))
+ if (vma_desc_test(desc, VMA_MAYSHARE_BIT))
return shmem_zero_setup_desc(desc);
/*
---
base-commit: 62f4c998b297cf233997a2b4cd6fc2d2df0319c9
change-id: 20260924-fix-dev-zero-readonly-shared-f2d46c156ce2
Best regards,
--
Lorenzo Stoakes (ARM) <ljs@kernel.org>
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] drivers/char/mem: mmap readonly MAP_SHARED-/dev/zero correctly 2026-09-24 14:48 [PATCH] drivers/char/mem: mmap readonly MAP_SHARED-/dev/zero correctly Lorenzo Stoakes (ARM) @ 2026-09-24 15:08 ` Greg Kroah-Hartman 2026-09-24 15:29 ` Lorenzo Stoakes (ARM) 2026-09-24 15:37 ` David Hildenbrand (Arm) 1 sibling, 1 reply; 8+ messages in thread From: Greg Kroah-Hartman @ 2026-09-24 15:08 UTC (permalink / raw) To: Lorenzo Stoakes (ARM) Cc: Andrew Morton, Liam R. Howlett, Vlastimil Babka, Jann Horn, Pedro Falcato, David Hildenbrand, Mike Rapoport, Suren Baghdasaryan, Michal Hocko, Arnd Bergmann, linux-mm, linux-kernel, Lance Yang, syzbot+c181d3198e98f8aef8b9 On Thu, Sep 24, 2026 at 03:48:24PM +0100, Lorenzo Stoakes (ARM) wrote: > Rather surprisingly, opening /dev/zero read-only then mmap()'ing it > MAP_SHARED gets you true anonymous memory (albeit in a VMA with > non-NULL vma->vm_file). > > This is a by-product of MAP_PRIVATE-/dev/zero being how anonymous memory > was mapped in Linux's distant past. > > It happens because mmap_zero_prepare() gates on VMA_SHARED_BIT and when > mapping a read-only file MAP_SHARED, do_mmap() clears VMA_SHARED_BIT and > VMA_MAYWRITE_BIT. > > The gating is incorrect - the (poorly named) VMA_MAYSHARE_BIT flag exists > explicitly to tell you if something was originally mapped MAP_SHARED. > > So the fix is simple - gate on this instead. > > This isn't exactly a common use case, but it's unexpected behaviour which > now causes an assert if CONFIG_DEBUG_VM is set. > > While this bug has existed since the dawn of time for linux (or at least > since 2.6.12), it hasn't caused issues in the past, so while it's incorrect > behaviour, it doesn't seem necessary to backport that far. > > The mapping is now accounted at mmap time and can fail with -ENOMEM under > strict overcommit, and read faults allocate folios. However this is normal > behaviour for a read-only shmem mapping. > > Commit 93c0c8dc87f6 ("mm/rmap: use anon pgoff to track MAP_PRIVATE > file-backed anon folios") is the first patch at which the debug assert > fires, so target that instead. > > Fixes: 93c0c8dc87f6 ("mm/rmap: use anon pgoff to track MAP_PRIVATE file-backed anon folios") > Reported-by: syzbot+c181d3198e98f8aef8b9@syzkaller.appspotmail.com > Closes: https://lore.kernel.org/linux-mm/6ab4ae75.80e1c6cc.1e8e5f.000d.GAE@google.com/ > Signed-off-by: Lorenzo Stoakes (ARM) <ljs@kernel.org> > --- > drivers/char/mem.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/char/mem.c b/drivers/char/mem.c > index 63253d1de5d7..5b93c92c2cf1 100644 > --- a/drivers/char/mem.c > +++ b/drivers/char/mem.c > @@ -503,7 +503,7 @@ static int mmap_zero_prepare(struct vm_area_desc *desc) > #ifndef CONFIG_MMU > return -ENOSYS; > #endif > - if (vma_desc_test(desc, VMA_SHARED_BIT)) > + if (vma_desc_test(desc, VMA_MAYSHARE_BIT)) > return shmem_zero_setup_desc(desc); What's the odds that something "big" actually relies on this? I'm all for the change, but be prepared in about 5 years for people to start complaining :) Want me to take it through my tree? thanks, greg k-h ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] drivers/char/mem: mmap readonly MAP_SHARED-/dev/zero correctly 2026-09-24 15:08 ` Greg Kroah-Hartman @ 2026-09-24 15:29 ` Lorenzo Stoakes (ARM) 2026-09-24 15:42 ` Greg Kroah-Hartman 0 siblings, 1 reply; 8+ messages in thread From: Lorenzo Stoakes (ARM) @ 2026-09-24 15:29 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: Andrew Morton, Liam R. Howlett, Vlastimil Babka, Jann Horn, Pedro Falcato, David Hildenbrand, Mike Rapoport, Suren Baghdasaryan, Michal Hocko, Arnd Bergmann, linux-mm, linux-kernel, Lance Yang, syzbot+c181d3198e98f8aef8b9 On Thu, Sep 24, 2026 at 05:08:49PM +0200, Greg Kroah-Hartman wrote: > On Thu, Sep 24, 2026 at 03:48:24PM +0100, Lorenzo Stoakes (ARM) wrote: > > Rather surprisingly, opening /dev/zero read-only then mmap()'ing it > > MAP_SHARED gets you true anonymous memory (albeit in a VMA with > > non-NULL vma->vm_file). > > > > This is a by-product of MAP_PRIVATE-/dev/zero being how anonymous memory > > was mapped in Linux's distant past. > > > > It happens because mmap_zero_prepare() gates on VMA_SHARED_BIT and when > > mapping a read-only file MAP_SHARED, do_mmap() clears VMA_SHARED_BIT and > > VMA_MAYWRITE_BIT. > > > > The gating is incorrect - the (poorly named) VMA_MAYSHARE_BIT flag exists > > explicitly to tell you if something was originally mapped MAP_SHARED. > > > > So the fix is simple - gate on this instead. > > > > This isn't exactly a common use case, but it's unexpected behaviour which > > now causes an assert if CONFIG_DEBUG_VM is set. > > > > While this bug has existed since the dawn of time for linux (or at least > > since 2.6.12), it hasn't caused issues in the past, so while it's incorrect > > behaviour, it doesn't seem necessary to backport that far. > > > > The mapping is now accounted at mmap time and can fail with -ENOMEM under > > strict overcommit, and read faults allocate folios. However this is normal > > behaviour for a read-only shmem mapping. > > > > Commit 93c0c8dc87f6 ("mm/rmap: use anon pgoff to track MAP_PRIVATE > > file-backed anon folios") is the first patch at which the debug assert > > fires, so target that instead. > > > > Fixes: 93c0c8dc87f6 ("mm/rmap: use anon pgoff to track MAP_PRIVATE file-backed anon folios") > > Reported-by: syzbot+c181d3198e98f8aef8b9@syzkaller.appspotmail.com > > Closes: https://lore.kernel.org/linux-mm/6ab4ae75.80e1c6cc.1e8e5f.000d.GAE@google.com/ > > Signed-off-by: Lorenzo Stoakes (ARM) <ljs@kernel.org> > > --- > > drivers/char/mem.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/char/mem.c b/drivers/char/mem.c > > index 63253d1de5d7..5b93c92c2cf1 100644 > > --- a/drivers/char/mem.c > > +++ b/drivers/char/mem.c > > @@ -503,7 +503,7 @@ static int mmap_zero_prepare(struct vm_area_desc *desc) > > #ifndef CONFIG_MMU > > return -ENOSYS; > > #endif > > - if (vma_desc_test(desc, VMA_SHARED_BIT)) > > + if (vma_desc_test(desc, VMA_MAYSHARE_BIT)) > > return shmem_zero_setup_desc(desc); > > What's the odds that something "big" actually relies on this? I think it'd be fairly crazy to, unless people really care about read-faulting a lot of zero pages which they want to somehow not account properly. Anything's possible but I think on this, right to take a (v. small) risk! > > I'm all for the change, but be prepared in about 5 years for people to > start complaining :) > > Want me to take it through my tree? Sure, thanks, though there will be a conflict against a patch in mm. Though it's really that the file is moved (to mm/), so actually should be an easy resolution? > > thanks, > > greg k-h -- Cheers, Lorenzo ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] drivers/char/mem: mmap readonly MAP_SHARED-/dev/zero correctly 2026-09-24 15:29 ` Lorenzo Stoakes (ARM) @ 2026-09-24 15:42 ` Greg Kroah-Hartman 0 siblings, 0 replies; 8+ messages in thread From: Greg Kroah-Hartman @ 2026-09-24 15:42 UTC (permalink / raw) To: Lorenzo Stoakes (ARM) Cc: Andrew Morton, Liam R. Howlett, Vlastimil Babka, Jann Horn, Pedro Falcato, David Hildenbrand, Mike Rapoport, Suren Baghdasaryan, Michal Hocko, Arnd Bergmann, linux-mm, linux-kernel, Lance Yang, syzbot+c181d3198e98f8aef8b9 On Thu, Sep 24, 2026 at 04:29:23PM +0100, Lorenzo Stoakes (ARM) wrote: > On Thu, Sep 24, 2026 at 05:08:49PM +0200, Greg Kroah-Hartman wrote: > > On Thu, Sep 24, 2026 at 03:48:24PM +0100, Lorenzo Stoakes (ARM) wrote: > > > Rather surprisingly, opening /dev/zero read-only then mmap()'ing it > > > MAP_SHARED gets you true anonymous memory (albeit in a VMA with > > > non-NULL vma->vm_file). > > > > > > This is a by-product of MAP_PRIVATE-/dev/zero being how anonymous memory > > > was mapped in Linux's distant past. > > > > > > It happens because mmap_zero_prepare() gates on VMA_SHARED_BIT and when > > > mapping a read-only file MAP_SHARED, do_mmap() clears VMA_SHARED_BIT and > > > VMA_MAYWRITE_BIT. > > > > > > The gating is incorrect - the (poorly named) VMA_MAYSHARE_BIT flag exists > > > explicitly to tell you if something was originally mapped MAP_SHARED. > > > > > > So the fix is simple - gate on this instead. > > > > > > This isn't exactly a common use case, but it's unexpected behaviour which > > > now causes an assert if CONFIG_DEBUG_VM is set. > > > > > > While this bug has existed since the dawn of time for linux (or at least > > > since 2.6.12), it hasn't caused issues in the past, so while it's incorrect > > > behaviour, it doesn't seem necessary to backport that far. > > > > > > The mapping is now accounted at mmap time and can fail with -ENOMEM under > > > strict overcommit, and read faults allocate folios. However this is normal > > > behaviour for a read-only shmem mapping. > > > > > > Commit 93c0c8dc87f6 ("mm/rmap: use anon pgoff to track MAP_PRIVATE > > > file-backed anon folios") is the first patch at which the debug assert > > > fires, so target that instead. > > > > > > Fixes: 93c0c8dc87f6 ("mm/rmap: use anon pgoff to track MAP_PRIVATE file-backed anon folios") > > > Reported-by: syzbot+c181d3198e98f8aef8b9@syzkaller.appspotmail.com > > > Closes: https://lore.kernel.org/linux-mm/6ab4ae75.80e1c6cc.1e8e5f.000d.GAE@google.com/ > > > Signed-off-by: Lorenzo Stoakes (ARM) <ljs@kernel.org> > > > --- > > > drivers/char/mem.c | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/drivers/char/mem.c b/drivers/char/mem.c > > > index 63253d1de5d7..5b93c92c2cf1 100644 > > > --- a/drivers/char/mem.c > > > +++ b/drivers/char/mem.c > > > @@ -503,7 +503,7 @@ static int mmap_zero_prepare(struct vm_area_desc *desc) > > > #ifndef CONFIG_MMU > > > return -ENOSYS; > > > #endif > > > - if (vma_desc_test(desc, VMA_SHARED_BIT)) > > > + if (vma_desc_test(desc, VMA_MAYSHARE_BIT)) > > > return shmem_zero_setup_desc(desc); > > > > What's the odds that something "big" actually relies on this? > > I think it'd be fairly crazy to, unless people really care about read-faulting a > lot of zero pages which they want to somehow not account properly. > > Anything's possible but I think on this, right to take a (v. small) risk! > > > > > I'm all for the change, but be prepared in about 5 years for people to > > start complaining :) > > > > Want me to take it through my tree? > > Sure, thanks, though there will be a conflict against a patch in mm. > > Though it's really that the file is moved (to mm/), so actually should be an > easy resolution? No problem, you can take it if that makes it easier: Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] drivers/char/mem: mmap readonly MAP_SHARED-/dev/zero correctly 2026-09-24 14:48 [PATCH] drivers/char/mem: mmap readonly MAP_SHARED-/dev/zero correctly Lorenzo Stoakes (ARM) 2026-09-24 15:08 ` Greg Kroah-Hartman @ 2026-09-24 15:37 ` David Hildenbrand (Arm) 2026-09-25 2:58 ` Andrew Morton 1 sibling, 1 reply; 8+ messages in thread From: David Hildenbrand (Arm) @ 2026-09-24 15:37 UTC (permalink / raw) To: Lorenzo Stoakes (ARM), Andrew Morton, Liam R. Howlett, Vlastimil Babka, Jann Horn, Pedro Falcato, Mike Rapoport, Suren Baghdasaryan, Michal Hocko, Arnd Bergmann, Greg Kroah-Hartman Cc: linux-mm, linux-kernel, Lance Yang, syzbot+c181d3198e98f8aef8b9 On 9/24/26 16:48, Lorenzo Stoakes (ARM) wrote: > Rather surprisingly, opening /dev/zero read-only then mmap()'ing it > MAP_SHARED gets you true anonymous memory (albeit in a VMA with > non-NULL vma->vm_file). > > This is a by-product of MAP_PRIVATE-/dev/zero being how anonymous memory > was mapped in Linux's distant past. > > It happens because mmap_zero_prepare() gates on VMA_SHARED_BIT and when > mapping a read-only file MAP_SHARED, do_mmap() clears VMA_SHARED_BIT and > VMA_MAYWRITE_BIT. > > The gating is incorrect - the (poorly named) VMA_MAYSHARE_BIT flag exists > explicitly to tell you if something was originally mapped MAP_SHARED. > > So the fix is simple - gate on this instead. > > This isn't exactly a common use case, but it's unexpected behaviour which > now causes an assert if CONFIG_DEBUG_VM is set. > > While this bug has existed since the dawn of time for linux (or at least > since 2.6.12), it hasn't caused issues in the past, so while it's incorrect > behaviour, it doesn't seem necessary to backport that far. > > The mapping is now accounted at mmap time and can fail with -ENOMEM under > strict overcommit, and read faults allocate folios. However this is normal > behaviour for a read-only shmem mapping. > > Commit 93c0c8dc87f6 ("mm/rmap: use anon pgoff to track MAP_PRIVATE > file-backed anon folios") is the first patch at which the debug assert > fires, so target that instead. > > Fixes: 93c0c8dc87f6 ("mm/rmap: use anon pgoff to track MAP_PRIVATE file-backed anon folios") > Reported-by: syzbot+c181d3198e98f8aef8b9@syzkaller.appspotmail.com > Closes: https://lore.kernel.org/linux-mm/6ab4ae75.80e1c6cc.1e8e5f.000d.GAE@google.com/ > Signed-off-by: Lorenzo Stoakes (ARM) <ljs@kernel.org> > --- > drivers/char/mem.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/char/mem.c b/drivers/char/mem.c > index 63253d1de5d7..5b93c92c2cf1 100644 > --- a/drivers/char/mem.c > +++ b/drivers/char/mem.c > @@ -503,7 +503,7 @@ static int mmap_zero_prepare(struct vm_area_desc *desc) > #ifndef CONFIG_MMU > return -ENOSYS; > #endif > - if (vma_desc_test(desc, VMA_SHARED_BIT)) > + if (vma_desc_test(desc, VMA_MAYSHARE_BIT)) > return shmem_zero_setup_desc(desc); > So instead of shared zeropages we'd now get zero-filled shmem pages. The alternative would be to just convert it to a proper read-only COW mapping in mmap code: * Not clearing VM_MAYWRITE, but keeping VM_WRITE clear * Clearing VMA_SHARED and VMA_MAYSHARE Sure, someone could then mprotect(PROT_WRITE that thing) or FOLL_FORCE|FOLL_WRITE to get anonymous memory. Just raising that as an alternative. -- Cheers, David ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] drivers/char/mem: mmap readonly MAP_SHARED-/dev/zero correctly 2026-09-24 15:37 ` David Hildenbrand (Arm) @ 2026-09-25 2:58 ` Andrew Morton 2026-09-25 7:29 ` David Hildenbrand (Arm) 0 siblings, 1 reply; 8+ messages in thread From: Andrew Morton @ 2026-09-25 2:58 UTC (permalink / raw) To: David Hildenbrand (Arm) Cc: Lorenzo Stoakes (ARM), Liam R. Howlett, Vlastimil Babka, Jann Horn, Pedro Falcato, Mike Rapoport, Suren Baghdasaryan, Michal Hocko, Arnd Bergmann, Greg Kroah-Hartman, linux-mm, linux-kernel, Lance Yang, syzbot+c181d3198e98f8aef8b9 On Thu, 24 Sep 2026 17:37:29 +0200 "David Hildenbrand (Arm)" <david@kernel.org> wrote: > On 9/24/26 16:48, Lorenzo Stoakes (ARM) wrote: > > Rather surprisingly, opening /dev/zero read-only then mmap()'ing it > > MAP_SHARED gets you true anonymous memory (albeit in a VMA with > > non-NULL vma->vm_file). > > > > ... > > > --- a/drivers/char/mem.c > > +++ b/drivers/char/mem.c > > @@ -503,7 +503,7 @@ static int mmap_zero_prepare(struct vm_area_desc *desc) > > #ifndef CONFIG_MMU > > return -ENOSYS; > > #endif > > - if (vma_desc_test(desc, VMA_SHARED_BIT)) > > + if (vma_desc_test(desc, VMA_MAYSHARE_BIT)) > > return shmem_zero_setup_desc(desc); > > > > So instead of shared zeropages we'd now get zero-filled shmem pages. "zeropage". Singular. Used to be! The accounting differences, possible changes in reclaim, memcg charging, maybe swap behavior. Switching to a different fault handler. It's hard to foresee all the effects of this. > The alternative would be to just convert it to a proper read-only COW mapping in > mmap code: > * Not clearing VM_MAYWRITE, but keeping VM_WRITE clear > * Clearing VMA_SHARED and VMA_MAYSHARE > > Sure, someone could then mprotect(PROT_WRITE that thing) or > FOLL_FORCE|FOLL_WRITE to get anonymous memory. Just raising that as an alternative. I dunno, the whole thing feels imprudent. To alter such longstanding core(ish) behavior. And why? Because a shiny new assertion said "hey, that isn't quite right". Wouldn't it be better to squish the warning somehow and to set about this change in a very careful way? ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] drivers/char/mem: mmap readonly MAP_SHARED-/dev/zero correctly 2026-09-25 2:58 ` Andrew Morton @ 2026-09-25 7:29 ` David Hildenbrand (Arm) 2026-09-25 8:38 ` Lorenzo Stoakes (ARM) 0 siblings, 1 reply; 8+ messages in thread From: David Hildenbrand (Arm) @ 2026-09-25 7:29 UTC (permalink / raw) To: Andrew Morton Cc: Lorenzo Stoakes (ARM), Liam R. Howlett, Vlastimil Babka, Jann Horn, Pedro Falcato, Mike Rapoport, Suren Baghdasaryan, Michal Hocko, Arnd Bergmann, Greg Kroah-Hartman, linux-mm, linux-kernel, Lance Yang, syzbot+c181d3198e98f8aef8b9 On 9/25/26 04:58, Andrew Morton wrote: > On Thu, 24 Sep 2026 17:37:29 +0200 "David Hildenbrand (Arm)" <david@kernel.org> wrote: > >> On 9/24/26 16:48, Lorenzo Stoakes (ARM) wrote: >>> Rather surprisingly, opening /dev/zero read-only then mmap()'ing it >>> MAP_SHARED gets you true anonymous memory (albeit in a VMA with >>> non-NULL vma->vm_file). >>> >> >> ... >> >>> --- a/drivers/char/mem.c >>> +++ b/drivers/char/mem.c >>> @@ -503,7 +503,7 @@ static int mmap_zero_prepare(struct vm_area_desc *desc) >>> #ifndef CONFIG_MMU >>> return -ENOSYS; >>> #endif >>> - if (vma_desc_test(desc, VMA_SHARED_BIT)) >>> + if (vma_desc_test(desc, VMA_MAYSHARE_BIT)) >>> return shmem_zero_setup_desc(desc); >>> >> >> So instead of shared zeropages we'd now get zero-filled shmem pages. > > "zeropage". Singular. Used to be! Hey, leave that German native speaker alone! :P Yes, you'd get the shared zeropage multiple times. (some architectures like s390x do have multiple ones .... likely you could even get the huge zero folio here) ... unless the MM has the shared zeropage disabled, and fallback to anonymous memory: -> mm_forbids_zeropage() ... we end up using THPs and the huge zero folio is disallowed, so we fallback to a anonymous THPs -> transparent_hugepage_use_zero_page() > > The accounting differences, possible changes in reclaim, memcg > charging, maybe swap behavior. Switching to a different fault handler. > It's hard to foresee all the effects of this. > >> The alternative would be to just convert it to a proper read-only COW mapping in >> mmap code: >> * Not clearing VM_MAYWRITE, but keeping VM_WRITE clear >> * Clearing VMA_SHARED and VMA_MAYSHARE >> >> Sure, someone could then mprotect(PROT_WRITE that thing) or >> FOLL_FORCE|FOLL_WRITE to get anonymous memory. Just raising that as an alternative. > > I dunno, the whole thing feels imprudent. To alter such longstanding > core(ish) behavior. And why? Because a shiny new assertion said "hey, > that isn't quite right". Wouldn't it be better to squish the warning > somehow and to set about this change in a very careful way? We really shouldn't allow anonymous pages in non-cow mappings. We can a) Disallow allocating an anon_vma and fail gracefully. So only a shared zeropage could ever get mapped there. Might break the s390x mm_forbids_zeropage(). But given that's only used in hypervisors like QEMU, unlikely. b) Do what Lorenzo proposes. This will allocate real memory. Someone decided to use MAP_SHARED, for unknown reasons, so I'd assume it's unlikely that something breaks, but you have a point. c) Convert them to proper COW mappings. After all, having the file read-only is absolutely irrelevant, because we will never ever use that file. It's anonymous memory. -- Cheers, David ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] drivers/char/mem: mmap readonly MAP_SHARED-/dev/zero correctly 2026-09-25 7:29 ` David Hildenbrand (Arm) @ 2026-09-25 8:38 ` Lorenzo Stoakes (ARM) 0 siblings, 0 replies; 8+ messages in thread From: Lorenzo Stoakes (ARM) @ 2026-09-25 8:38 UTC (permalink / raw) To: David Hildenbrand (Arm) Cc: Andrew Morton, Liam R. Howlett, Vlastimil Babka, Jann Horn, Pedro Falcato, Mike Rapoport, Suren Baghdasaryan, Michal Hocko, Arnd Bergmann, Greg Kroah-Hartman, linux-mm, linux-kernel, Lance Yang, syzbot+c181d3198e98f8aef8b9 On Fri, Sep 25, 2026 at 09:29:40AM +0200, David Hildenbrand (Arm) wrote: > On 9/25/26 04:58, Andrew Morton wrote: > > On Thu, 24 Sep 2026 17:37:29 +0200 "David Hildenbrand (Arm)" <david@kernel.org> wrote: > > > >> On 9/24/26 16:48, Lorenzo Stoakes (ARM) wrote: > >>> Rather surprisingly, opening /dev/zero read-only then mmap()'ing it > >>> MAP_SHARED gets you true anonymous memory (albeit in a VMA with > >>> non-NULL vma->vm_file). > >>> > >> > >> ... > >> > >>> --- a/drivers/char/mem.c > >>> +++ b/drivers/char/mem.c > >>> @@ -503,7 +503,7 @@ static int mmap_zero_prepare(struct vm_area_desc *desc) > >>> #ifndef CONFIG_MMU > >>> return -ENOSYS; > >>> #endif > >>> - if (vma_desc_test(desc, VMA_SHARED_BIT)) > >>> + if (vma_desc_test(desc, VMA_MAYSHARE_BIT)) > >>> return shmem_zero_setup_desc(desc); > >>> > >> > >> So instead of shared zeropages we'd now get zero-filled shmem pages. > > > > "zeropage". Singular. Used to be! > > Hey, leave that German native speaker alone! :P > > Yes, you'd get the shared zeropage multiple times. (some architectures like > s390x do have multiple ones .... likely you could even get the huge zero folio here) > > ... unless the MM has the shared zeropage disabled, and fallback to anonymous > memory: > > -> mm_forbids_zeropage() > > ... we end up using THPs and the huge zero folio is disallowed, so we fallback > to a anonymous THPs > > -> transparent_hugepage_use_zero_page() > > > > > The accounting differences, possible changes in reclaim, memcg > > charging, maybe swap behavior. Switching to a different fault handler. > > It's hard to foresee all the effects of this. > > > >> The alternative would be to just convert it to a proper read-only COW mapping in > >> mmap code: > >> * Not clearing VM_MAYWRITE, but keeping VM_WRITE clear > >> * Clearing VMA_SHARED and VMA_MAYSHARE > >> > >> Sure, someone could then mprotect(PROT_WRITE that thing) or > >> FOLL_FORCE|FOLL_WRITE to get anonymous memory. Just raising that as an alternative. > > > > I dunno, the whole thing feels imprudent. To alter such longstanding > > core(ish) behavior. And why? Because a shiny new assertion said "hey, > > that isn't quite right". Wouldn't it be better to squish the warning > > somehow and to set about this change in a very careful way? > > We really shouldn't allow anonymous pages in non-cow mappings. Yes agreed entirely. It then becomes a question about how to get readonly memory. > > We can > > a) Disallow allocating an anon_vma and fail gracefully. So only a shared > zeropage could ever get mapped there. Might break the s390x > mm_forbids_zeropage(). But given that's only used in hypervisors like QEMU, > unlikely. Something like: /* about to maybe prep anon_vma */ if (!vma_cow_mapping(vma) && vma_test(vma, VMA_MAYSHARE_BIT)) { /* don't prep anon give zero page */ } ? I think though it's surely the only case (I hope!) where you can possibly be both anon (as in missing vm_ops) and !CoW? I hope? :) So it feels better to fix it at the source. OTOH maybe it's worth special-casing so we don't allocate on read. But that brings me to c)... > > b) Do what Lorenzo proposes. This will allocate real memory. Someone decided to > use MAP_SHARED, for unknown reasons, so I'd assume it's unlikely that > something breaks, but you have a point. I would say this patch is the right fix for the moment to fix the assert, and we can chase up with other approaches afterwards. > > c) Convert them to proper COW mappings. After all, having the file read-only is > absolutely irrelevant, because we will never ever use that file. It's > anonymous memory. > ...My idea for the next step for /dev/zero is to remove the mmap handler and have some specific code in the mmap logic for it solely. Like we already have: if (map->vm_file) error = __mmap_new_file_vma(map, vma); else if (!is_anon) error = shmem_zero_setup(vma); And there's already specific file_is_dev_zero() code, so there you could simply decide: CoW /dev/zero -> R/W anon shared readonly /dev/zero -> R/O CoW (i.e. with VMA_MAYWRITE_BIT set) As a special case because somebody really probably does want that. But for the purposes of a 7.3 fix I think let's go with b) [i.e. this patch] and follow up if that makes sense to you? > -- > Cheers, > > David -- Cheers, Lorenzo ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2026-09-25 8:38 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2026-09-24 14:48 [PATCH] drivers/char/mem: mmap readonly MAP_SHARED-/dev/zero correctly Lorenzo Stoakes (ARM) 2026-09-24 15:08 ` Greg Kroah-Hartman 2026-09-24 15:29 ` Lorenzo Stoakes (ARM) 2026-09-24 15:42 ` Greg Kroah-Hartman 2026-09-24 15:37 ` David Hildenbrand (Arm) 2026-09-25 2:58 ` Andrew Morton 2026-09-25 7:29 ` David Hildenbrand (Arm) 2026-09-25 8:38 ` Lorenzo Stoakes (ARM)
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®