mm/memory.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Page mapping with "VM_READ|VM_WRITE|VM_MAYREAD|VM_MAYWRITE|VM_SHARED",
the first time accessing this address through a write operation will
trigger a do_shared_fault, if mapping is anonymous, it can lead to a
null pointer dereference.
[ 23.232336][ T195] Call trace:
[ 23.232542][ T195] file_update_time+0x2c/0xd8
[ 23.232801][ T195] fault_dirty_shared_page+0x1a0/0x220
[ 23.233099][ T195] do_shared_fault+0xe8/0x240
[ 23.233374][ T195] do_fault+0x78/0x240
[ 23.233629][ T195] handle_pte_fault+0x1f0/0x3f0
[ 23.233905][ T195] __handle_mm_fault+0x2b0/0x548
[ 23.234186][ T195] handle_mm_fault+0xd4/0x2f8
[ 23.234462][ T195] do_page_fault+0x2f0/0x5f8
[ 23.234727][ T195] do_translation_fault+0x8c/0xc8
[ 23.235021][ T195] do_mem_abort+0x68/0x100
[ 23.235283][ T195] el0_da+0x4c/0x1a8
[ 23.235551][ T195] el0t_64_sync_handler+0xe4/0x158
[ 23.235861][ T195] el0t_64_sync+0x37c/0x380
Signed-off-by: Yuntao Liu <liuyuntao12@huawei.com>
---
mm/memory.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/mm/memory.c b/mm/memory.c
index eaf98d518289..8106ef8a5036 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -3412,7 +3412,7 @@ static vm_fault_t fault_dirty_shared_page(struct vm_fault *vmf)
mapping = folio_raw_mapping(folio);
folio_unlock(folio);
- if (!page_mkwrite)
+ if (!page_mkwrite && vma->vm_file)
file_update_time(vma->vm_file);
/*
--
2.34.1On Mon, Jul 07, 2025 at 10:51:18AM +0000, Yuntao Liu wrote: > Page mapping with "VM_READ|VM_WRITE|VM_MAYREAD|VM_MAYWRITE|VM_SHARED", > the first time accessing this address through a write operation will > trigger a do_shared_fault, if mapping is anonymous, it can lead to a > null pointer dereference. How can it be anonymous with VM_SHARED set? This would be a far, far bigger bug. > > [ 23.232336][ T195] Call trace: > [ 23.232542][ T195] file_update_time+0x2c/0xd8 > [ 23.232801][ T195] fault_dirty_shared_page+0x1a0/0x220 > [ 23.233099][ T195] do_shared_fault+0xe8/0x240 > [ 23.233374][ T195] do_fault+0x78/0x240 > [ 23.233629][ T195] handle_pte_fault+0x1f0/0x3f0 > [ 23.233905][ T195] __handle_mm_fault+0x2b0/0x548 > [ 23.234186][ T195] handle_mm_fault+0xd4/0x2f8 > [ 23.234462][ T195] do_page_fault+0x2f0/0x5f8 > [ 23.234727][ T195] do_translation_fault+0x8c/0xc8 > [ 23.235021][ T195] do_mem_abort+0x68/0x100 > [ 23.235283][ T195] el0_da+0x4c/0x1a8 > [ 23.235551][ T195] el0t_64_sync_handler+0xe4/0x158 > [ 23.235861][ T195] el0t_64_sync+0x37c/0x380 How have you obtained this? Are you somehow injecting invalid state here? > > Signed-off-by: Yuntao Liu <liuyuntao12@huawei.com> > --- > mm/memory.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/mm/memory.c b/mm/memory.c > index eaf98d518289..8106ef8a5036 100644 > --- a/mm/memory.c > +++ b/mm/memory.c > @@ -3412,7 +3412,7 @@ static vm_fault_t fault_dirty_shared_page(struct vm_fault *vmf) > mapping = folio_raw_mapping(folio); > folio_unlock(folio); > > - if (!page_mkwrite) > + if (!page_mkwrite && vma->vm_file) The function is ltierally fault_dirty_shared_page(), how are we arriving here with !vma->vm_file? > file_update_time(vma->vm_file); > > /* > -- > 2.34.1 >
On 07.07.25 13:14, Lorenzo Stoakes wrote: > On Mon, Jul 07, 2025 at 10:51:18AM +0000, Yuntao Liu wrote: >> Page mapping with "VM_READ|VM_WRITE|VM_MAYREAD|VM_MAYWRITE|VM_SHARED", >> the first time accessing this address through a write operation will >> trigger a do_shared_fault, if mapping is anonymous, it can lead to a >> null pointer dereference. > > How can it be anonymous with VM_SHARED set? This would be a far, far bigger > bug. > >> >> [ 23.232336][ T195] Call trace: >> [ 23.232542][ T195] file_update_time+0x2c/0xd8 >> [ 23.232801][ T195] fault_dirty_shared_page+0x1a0/0x220 >> [ 23.233099][ T195] do_shared_fault+0xe8/0x240 >> [ 23.233374][ T195] do_fault+0x78/0x240 >> [ 23.233629][ T195] handle_pte_fault+0x1f0/0x3f0 >> [ 23.233905][ T195] __handle_mm_fault+0x2b0/0x548 >> [ 23.234186][ T195] handle_mm_fault+0xd4/0x2f8 >> [ 23.234462][ T195] do_page_fault+0x2f0/0x5f8 >> [ 23.234727][ T195] do_translation_fault+0x8c/0xc8 >> [ 23.235021][ T195] do_mem_abort+0x68/0x100 >> [ 23.235283][ T195] el0_da+0x4c/0x1a8 >> [ 23.235551][ T195] el0t_64_sync_handler+0xe4/0x158 >> [ 23.235861][ T195] el0t_64_sync+0x37c/0x380 > > How have you obtained this? Are you somehow injecting invalid state here? > >> >> Signed-off-by: Yuntao Liu <liuyuntao12@huawei.com> >> --- >> mm/memory.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/mm/memory.c b/mm/memory.c >> index eaf98d518289..8106ef8a5036 100644 >> --- a/mm/memory.c >> +++ b/mm/memory.c >> @@ -3412,7 +3412,7 @@ static vm_fault_t fault_dirty_shared_page(struct vm_fault *vmf) >> mapping = folio_raw_mapping(folio); >> folio_unlock(folio); >> >> - if (!page_mkwrite) >> + if (!page_mkwrite && vma->vm_file) > > The function is ltierally fault_dirty_shared_page(), how are we arriving > here with !vma->vm_file? IIRC, MAP_ANON |MAP_SHARED would have done a shmem_zero_setup(). mm/mmap.c still has the comment "mmap_region() will call shmem_zero_setup() to create a file". I think this was moved to __mmap_new_vma(). Is there any (error) path where we could not call that by accident? -- Cheers, David / dhildenb
On Mon, Jul 07, 2025 at 02:27:01PM +0200, David Hildenbrand wrote: > IIRC, MAP_ANON |MAP_SHARED would have done a shmem_zero_setup(). > > mm/mmap.c still has the comment "mmap_region() will call shmem_zero_setup() > to create a file". Correct. > > I think this was moved to __mmap_new_vma(). > > Is there any (error) path where we could not call that by accident? No. Shared means there's a file. This also would mean we've been kernel NULL pointer dereferencing since 2019 btw when this change was made. I don't recall the reports... :>) I think people get confused because MAP_SHARED | MAP_ANON makes people think there won't be a file. Our terminology sucks... > > -- > Cheers, > > David / dhildenb >
© 2016 - 2025 Red Hat, Inc.