* Re: [PATCH v2 2/2] arm64/mm: fix incorrect file_map_count for invalid pmd/pud
2022-11-17 7:56 ` Liu Shixin
@ 2022-11-17 7:49 ` David Hildenbrand
-1 siblings, 0 replies; 26+ messages in thread
From: David Hildenbrand @ 2022-11-17 7:49 UTC (permalink / raw)
To: Liu Shixin, Catalin Marinas, Will Deacon, Denys Vlasenko,
Kefeng Wang, Anshuman Khandual, David Hildenbrand, Rafael Aquini,
Pasha Tatashin
Cc: linux-arm-kernel, linux-kernel
On 17.11.22 08:56, Liu Shixin wrote:
> The page table check trigger BUG_ON() unexpectedly when split hugepage:
>
> ------------[ cut here ]------------
> kernel BUG at mm/page_table_check.c:119!
> Internal error: Oops - BUG: 00000000f2000800 [#1] SMP
> Dumping ftrace buffer:
> (ftrace buffer empty)
> Modules linked in:
> CPU: 7 PID: 210 Comm: transhuge-stres Not tainted 6.1.0-rc3+ #748
> Hardware name: linux,dummy-virt (DT)
> pstate: 20000005 (nzCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> pc : page_table_check_set.isra.0+0x398/0x468
> lr : page_table_check_set.isra.0+0x1c0/0x468
> [...]
> Call trace:
> page_table_check_set.isra.0+0x398/0x468
> __page_table_check_pte_set+0x160/0x1c0
> __split_huge_pmd_locked+0x900/0x1648
> __split_huge_pmd+0x28c/0x3b8
> unmap_page_range+0x428/0x858
> unmap_single_vma+0xf4/0x1c8
> zap_page_range+0x2b0/0x410
> madvise_vma_behavior+0xc44/0xe78
> do_madvise+0x280/0x698
> __arm64_sys_madvise+0x90/0xe8
> invoke_syscall.constprop.0+0xdc/0x1d8
> do_el0_svc+0xf4/0x3f8
> el0_svc+0x58/0x120
> el0t_64_sync_handler+0xb8/0xc0
> el0t_64_sync+0x19c/0x1a0
> [...]
>
> On arm64, pmd_leaf() will return true even if the pmd is invalid due to
> pmd_present_invalid() check. So in pmdp_invalidate() the file_map_count
> will not only decrease once but also increase once. Then in set_pte_at(),
> the file_map_count increase again, and so trigger BUG_ON() unexpectedly.
>
> Fix this problem by adding pmd_valid() in pmd_user_accessible_page().
> Moreover, add pud_valid() for pud_user_accessible_page() too.
>
> Fixes: 42b2547137f5 ("arm64/mm: enable ARCH_SUPPORTS_PAGE_TABLE_CHECK")
> Reported-by: Denys Vlasenko <dvlasenk@redhat.com>
> Signed-off-by: Liu Shixin <liushixin2@huawei.com>
> Acked-by: Pasha Tatashin <pasha.tatashin@soleen.com>
> ---
> arch/arm64/include/asm/pgtable.h | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h
> index edf6625ce965..3bc64199aa2e 100644
> --- a/arch/arm64/include/asm/pgtable.h
> +++ b/arch/arm64/include/asm/pgtable.h
> @@ -863,12 +863,12 @@ static inline bool pte_user_accessible_page(pte_t pte)
>
> static inline bool pmd_user_accessible_page(pmd_t pmd)
> {
> - return pmd_leaf(pmd) && (pmd_user(pmd) || pmd_user_exec(pmd));
> + return pmd_valid(pmd) && pmd_leaf(pmd) && (pmd_user(pmd) || pmd_user_exec(pmd));
> }
>
> static inline bool pud_user_accessible_page(pud_t pud)
> {
> - return pud_leaf(pud) && pud_user(pud);
> + return pud_valid(pud) && pud_leaf(pud) && pud_user(pud);
> }
> #endif
>
Acked-by: David Hildenbrand <david@redhat.com>
--
Thanks,
David / dhildenb
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v2 2/2] arm64/mm: fix incorrect file_map_count for invalid pmd/pud
@ 2022-11-17 7:49 ` David Hildenbrand
0 siblings, 0 replies; 26+ messages in thread
From: David Hildenbrand @ 2022-11-17 7:49 UTC (permalink / raw)
To: Liu Shixin, Catalin Marinas, Will Deacon, Denys Vlasenko,
Kefeng Wang, Anshuman Khandual, David Hildenbrand, Rafael Aquini,
Pasha Tatashin
Cc: linux-arm-kernel, linux-kernel
On 17.11.22 08:56, Liu Shixin wrote:
> The page table check trigger BUG_ON() unexpectedly when split hugepage:
>
> ------------[ cut here ]------------
> kernel BUG at mm/page_table_check.c:119!
> Internal error: Oops - BUG: 00000000f2000800 [#1] SMP
> Dumping ftrace buffer:
> (ftrace buffer empty)
> Modules linked in:
> CPU: 7 PID: 210 Comm: transhuge-stres Not tainted 6.1.0-rc3+ #748
> Hardware name: linux,dummy-virt (DT)
> pstate: 20000005 (nzCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> pc : page_table_check_set.isra.0+0x398/0x468
> lr : page_table_check_set.isra.0+0x1c0/0x468
> [...]
> Call trace:
> page_table_check_set.isra.0+0x398/0x468
> __page_table_check_pte_set+0x160/0x1c0
> __split_huge_pmd_locked+0x900/0x1648
> __split_huge_pmd+0x28c/0x3b8
> unmap_page_range+0x428/0x858
> unmap_single_vma+0xf4/0x1c8
> zap_page_range+0x2b0/0x410
> madvise_vma_behavior+0xc44/0xe78
> do_madvise+0x280/0x698
> __arm64_sys_madvise+0x90/0xe8
> invoke_syscall.constprop.0+0xdc/0x1d8
> do_el0_svc+0xf4/0x3f8
> el0_svc+0x58/0x120
> el0t_64_sync_handler+0xb8/0xc0
> el0t_64_sync+0x19c/0x1a0
> [...]
>
> On arm64, pmd_leaf() will return true even if the pmd is invalid due to
> pmd_present_invalid() check. So in pmdp_invalidate() the file_map_count
> will not only decrease once but also increase once. Then in set_pte_at(),
> the file_map_count increase again, and so trigger BUG_ON() unexpectedly.
>
> Fix this problem by adding pmd_valid() in pmd_user_accessible_page().
> Moreover, add pud_valid() for pud_user_accessible_page() too.
>
> Fixes: 42b2547137f5 ("arm64/mm: enable ARCH_SUPPORTS_PAGE_TABLE_CHECK")
> Reported-by: Denys Vlasenko <dvlasenk@redhat.com>
> Signed-off-by: Liu Shixin <liushixin2@huawei.com>
> Acked-by: Pasha Tatashin <pasha.tatashin@soleen.com>
> ---
> arch/arm64/include/asm/pgtable.h | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h
> index edf6625ce965..3bc64199aa2e 100644
> --- a/arch/arm64/include/asm/pgtable.h
> +++ b/arch/arm64/include/asm/pgtable.h
> @@ -863,12 +863,12 @@ static inline bool pte_user_accessible_page(pte_t pte)
>
> static inline bool pmd_user_accessible_page(pmd_t pmd)
> {
> - return pmd_leaf(pmd) && (pmd_user(pmd) || pmd_user_exec(pmd));
> + return pmd_valid(pmd) && pmd_leaf(pmd) && (pmd_user(pmd) || pmd_user_exec(pmd));
> }
>
> static inline bool pud_user_accessible_page(pud_t pud)
> {
> - return pud_leaf(pud) && pud_user(pud);
> + return pud_valid(pud) && pud_leaf(pud) && pud_user(pud);
> }
> #endif
>
Acked-by: David Hildenbrand <david@redhat.com>
--
Thanks,
David / dhildenb
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH v2 0/2] arm64: fix two bug about page table check
@ 2022-11-17 7:56 ` Liu Shixin
0 siblings, 0 replies; 26+ messages in thread
From: Liu Shixin @ 2022-11-17 7:56 UTC (permalink / raw)
To: Catalin Marinas, Will Deacon, Denys Vlasenko, Kefeng Wang,
Anshuman Khandual, David Hildenbrand, Rafael Aquini,
Pasha Tatashin
Cc: linux-arm-kernel, linux-kernel, Liu Shixin
Denys Vlasenko has reported two bug about page table check on arm64.
On arm64, pmd_present() contains non-leaf pmd and invalid pmd too.
When collapse hugepage, the pmd is non-leaf and should skip the check.
Use pmd_leaf() instead of pmd_present().
When split hugepage, the pmd will be marked as invalid and then populate.
So we should decrease file_map_count when invalid pmd and then increase
when populate the pmd. Add pmd_valid() check.
v1->v2: Update comment and optimize the code by moving p?d_valid() at
first place suggested by Mark.
Liu Shixin (2):
arm64/mm: fix incorrect file_map_count for non-leaf pmd/pud
arm64/mm: fix incorrect file_map_count for invalid pmd/pud
arch/arm64/include/asm/pgtable.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
--
2.25.1
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH v2 0/2] arm64: fix two bug about page table check
@ 2022-11-17 7:56 ` Liu Shixin
0 siblings, 0 replies; 26+ messages in thread
From: Liu Shixin @ 2022-11-17 7:56 UTC (permalink / raw)
To: Catalin Marinas, Will Deacon, Denys Vlasenko, Kefeng Wang,
Anshuman Khandual, David Hildenbrand, Rafael Aquini,
Pasha Tatashin
Cc: linux-arm-kernel, linux-kernel, Liu Shixin
Denys Vlasenko has reported two bug about page table check on arm64.
On arm64, pmd_present() contains non-leaf pmd and invalid pmd too.
When collapse hugepage, the pmd is non-leaf and should skip the check.
Use pmd_leaf() instead of pmd_present().
When split hugepage, the pmd will be marked as invalid and then populate.
So we should decrease file_map_count when invalid pmd and then increase
when populate the pmd. Add pmd_valid() check.
v1->v2: Update comment and optimize the code by moving p?d_valid() at
first place suggested by Mark.
Liu Shixin (2):
arm64/mm: fix incorrect file_map_count for non-leaf pmd/pud
arm64/mm: fix incorrect file_map_count for invalid pmd/pud
arch/arm64/include/asm/pgtable.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
--
2.25.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH v2 1/2] arm64/mm: fix incorrect file_map_count for non-leaf pmd/pud
2022-11-17 7:56 ` Liu Shixin
@ 2022-11-17 7:56 ` Liu Shixin
-1 siblings, 0 replies; 26+ messages in thread
From: Liu Shixin @ 2022-11-17 7:56 UTC (permalink / raw)
To: Catalin Marinas, Will Deacon, Denys Vlasenko, Kefeng Wang,
Anshuman Khandual, David Hildenbrand, Rafael Aquini,
Pasha Tatashin
Cc: linux-arm-kernel, linux-kernel, Liu Shixin
The page table check trigger BUG_ON() unexpectedly when collapse hugepage:
------------[ cut here ]------------
kernel BUG at mm/page_table_check.c:82!
Internal error: Oops - BUG: 00000000f2000800 [#1] SMP
Dumping ftrace buffer:
(ftrace buffer empty)
Modules linked in:
CPU: 6 PID: 68 Comm: khugepaged Not tainted 6.1.0-rc3+ #750
Hardware name: linux,dummy-virt (DT)
pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
pc : page_table_check_clear.isra.0+0x258/0x3f0
lr : page_table_check_clear.isra.0+0x240/0x3f0
[...]
Call trace:
page_table_check_clear.isra.0+0x258/0x3f0
__page_table_check_pmd_clear+0xbc/0x108
pmdp_collapse_flush+0xb0/0x160
collapse_huge_page+0xa08/0x1080
hpage_collapse_scan_pmd+0xf30/0x1590
khugepaged_scan_mm_slot.constprop.0+0x52c/0xac8
khugepaged+0x338/0x518
kthread+0x278/0x2f8
ret_from_fork+0x10/0x20
[...]
Since pmd_user_accessible_page() doesn't check if a pmd is leaf, it
decrease file_map_count for a non-leaf pmd comes from collapse_huge_page().
and so trigger BUG_ON() unexpectedly.
Fix this problem by using pmd_leaf() insteal of pmd_present() in
pmd_user_accessible_page(). Moreover, use pud_leaf() for
pud_user_accessible_page() too.
Fixes: 42b2547137f5 ("arm64/mm: enable ARCH_SUPPORTS_PAGE_TABLE_CHECK")
Reported-by: Denys Vlasenko <dvlasenk@redhat.com>
Signed-off-by: Liu Shixin <liushixin2@huawei.com>
Reviewed-by: David Hildenbrand <david@redhat.com>
Acked-by: Pasha Tatashin <pasha.tatashin@soleen.com>
---
arch/arm64/include/asm/pgtable.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h
index 71a1af42f0e8..edf6625ce965 100644
--- a/arch/arm64/include/asm/pgtable.h
+++ b/arch/arm64/include/asm/pgtable.h
@@ -863,12 +863,12 @@ static inline bool pte_user_accessible_page(pte_t pte)
static inline bool pmd_user_accessible_page(pmd_t pmd)
{
- return pmd_present(pmd) && (pmd_user(pmd) || pmd_user_exec(pmd));
+ return pmd_leaf(pmd) && (pmd_user(pmd) || pmd_user_exec(pmd));
}
static inline bool pud_user_accessible_page(pud_t pud)
{
- return pud_present(pud) && pud_user(pud);
+ return pud_leaf(pud) && pud_user(pud);
}
#endif
--
2.25.1
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH v2 1/2] arm64/mm: fix incorrect file_map_count for non-leaf pmd/pud
@ 2022-11-17 7:56 ` Liu Shixin
0 siblings, 0 replies; 26+ messages in thread
From: Liu Shixin @ 2022-11-17 7:56 UTC (permalink / raw)
To: Catalin Marinas, Will Deacon, Denys Vlasenko, Kefeng Wang,
Anshuman Khandual, David Hildenbrand, Rafael Aquini,
Pasha Tatashin
Cc: linux-arm-kernel, linux-kernel, Liu Shixin
The page table check trigger BUG_ON() unexpectedly when collapse hugepage:
------------[ cut here ]------------
kernel BUG at mm/page_table_check.c:82!
Internal error: Oops - BUG: 00000000f2000800 [#1] SMP
Dumping ftrace buffer:
(ftrace buffer empty)
Modules linked in:
CPU: 6 PID: 68 Comm: khugepaged Not tainted 6.1.0-rc3+ #750
Hardware name: linux,dummy-virt (DT)
pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
pc : page_table_check_clear.isra.0+0x258/0x3f0
lr : page_table_check_clear.isra.0+0x240/0x3f0
[...]
Call trace:
page_table_check_clear.isra.0+0x258/0x3f0
__page_table_check_pmd_clear+0xbc/0x108
pmdp_collapse_flush+0xb0/0x160
collapse_huge_page+0xa08/0x1080
hpage_collapse_scan_pmd+0xf30/0x1590
khugepaged_scan_mm_slot.constprop.0+0x52c/0xac8
khugepaged+0x338/0x518
kthread+0x278/0x2f8
ret_from_fork+0x10/0x20
[...]
Since pmd_user_accessible_page() doesn't check if a pmd is leaf, it
decrease file_map_count for a non-leaf pmd comes from collapse_huge_page().
and so trigger BUG_ON() unexpectedly.
Fix this problem by using pmd_leaf() insteal of pmd_present() in
pmd_user_accessible_page(). Moreover, use pud_leaf() for
pud_user_accessible_page() too.
Fixes: 42b2547137f5 ("arm64/mm: enable ARCH_SUPPORTS_PAGE_TABLE_CHECK")
Reported-by: Denys Vlasenko <dvlasenk@redhat.com>
Signed-off-by: Liu Shixin <liushixin2@huawei.com>
Reviewed-by: David Hildenbrand <david@redhat.com>
Acked-by: Pasha Tatashin <pasha.tatashin@soleen.com>
---
arch/arm64/include/asm/pgtable.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h
index 71a1af42f0e8..edf6625ce965 100644
--- a/arch/arm64/include/asm/pgtable.h
+++ b/arch/arm64/include/asm/pgtable.h
@@ -863,12 +863,12 @@ static inline bool pte_user_accessible_page(pte_t pte)
static inline bool pmd_user_accessible_page(pmd_t pmd)
{
- return pmd_present(pmd) && (pmd_user(pmd) || pmd_user_exec(pmd));
+ return pmd_leaf(pmd) && (pmd_user(pmd) || pmd_user_exec(pmd));
}
static inline bool pud_user_accessible_page(pud_t pud)
{
- return pud_present(pud) && pud_user(pud);
+ return pud_leaf(pud) && pud_user(pud);
}
#endif
--
2.25.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH v2 2/2] arm64/mm: fix incorrect file_map_count for invalid pmd/pud
2022-11-17 7:56 ` Liu Shixin
@ 2022-11-17 7:56 ` Liu Shixin
-1 siblings, 0 replies; 26+ messages in thread
From: Liu Shixin @ 2022-11-17 7:56 UTC (permalink / raw)
To: Catalin Marinas, Will Deacon, Denys Vlasenko, Kefeng Wang,
Anshuman Khandual, David Hildenbrand, Rafael Aquini,
Pasha Tatashin
Cc: linux-arm-kernel, linux-kernel, Liu Shixin
The page table check trigger BUG_ON() unexpectedly when split hugepage:
------------[ cut here ]------------
kernel BUG at mm/page_table_check.c:119!
Internal error: Oops - BUG: 00000000f2000800 [#1] SMP
Dumping ftrace buffer:
(ftrace buffer empty)
Modules linked in:
CPU: 7 PID: 210 Comm: transhuge-stres Not tainted 6.1.0-rc3+ #748
Hardware name: linux,dummy-virt (DT)
pstate: 20000005 (nzCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
pc : page_table_check_set.isra.0+0x398/0x468
lr : page_table_check_set.isra.0+0x1c0/0x468
[...]
Call trace:
page_table_check_set.isra.0+0x398/0x468
__page_table_check_pte_set+0x160/0x1c0
__split_huge_pmd_locked+0x900/0x1648
__split_huge_pmd+0x28c/0x3b8
unmap_page_range+0x428/0x858
unmap_single_vma+0xf4/0x1c8
zap_page_range+0x2b0/0x410
madvise_vma_behavior+0xc44/0xe78
do_madvise+0x280/0x698
__arm64_sys_madvise+0x90/0xe8
invoke_syscall.constprop.0+0xdc/0x1d8
do_el0_svc+0xf4/0x3f8
el0_svc+0x58/0x120
el0t_64_sync_handler+0xb8/0xc0
el0t_64_sync+0x19c/0x1a0
[...]
On arm64, pmd_leaf() will return true even if the pmd is invalid due to
pmd_present_invalid() check. So in pmdp_invalidate() the file_map_count
will not only decrease once but also increase once. Then in set_pte_at(),
the file_map_count increase again, and so trigger BUG_ON() unexpectedly.
Fix this problem by adding pmd_valid() in pmd_user_accessible_page().
Moreover, add pud_valid() for pud_user_accessible_page() too.
Fixes: 42b2547137f5 ("arm64/mm: enable ARCH_SUPPORTS_PAGE_TABLE_CHECK")
Reported-by: Denys Vlasenko <dvlasenk@redhat.com>
Signed-off-by: Liu Shixin <liushixin2@huawei.com>
Acked-by: Pasha Tatashin <pasha.tatashin@soleen.com>
---
arch/arm64/include/asm/pgtable.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h
index edf6625ce965..3bc64199aa2e 100644
--- a/arch/arm64/include/asm/pgtable.h
+++ b/arch/arm64/include/asm/pgtable.h
@@ -863,12 +863,12 @@ static inline bool pte_user_accessible_page(pte_t pte)
static inline bool pmd_user_accessible_page(pmd_t pmd)
{
- return pmd_leaf(pmd) && (pmd_user(pmd) || pmd_user_exec(pmd));
+ return pmd_valid(pmd) && pmd_leaf(pmd) && (pmd_user(pmd) || pmd_user_exec(pmd));
}
static inline bool pud_user_accessible_page(pud_t pud)
{
- return pud_leaf(pud) && pud_user(pud);
+ return pud_valid(pud) && pud_leaf(pud) && pud_user(pud);
}
#endif
--
2.25.1
^ permalink raw reply related [flat|nested] 26+ messages in thread
* [PATCH v2 2/2] arm64/mm: fix incorrect file_map_count for invalid pmd/pud
@ 2022-11-17 7:56 ` Liu Shixin
0 siblings, 0 replies; 26+ messages in thread
From: Liu Shixin @ 2022-11-17 7:56 UTC (permalink / raw)
To: Catalin Marinas, Will Deacon, Denys Vlasenko, Kefeng Wang,
Anshuman Khandual, David Hildenbrand, Rafael Aquini,
Pasha Tatashin
Cc: linux-arm-kernel, linux-kernel, Liu Shixin
The page table check trigger BUG_ON() unexpectedly when split hugepage:
------------[ cut here ]------------
kernel BUG at mm/page_table_check.c:119!
Internal error: Oops - BUG: 00000000f2000800 [#1] SMP
Dumping ftrace buffer:
(ftrace buffer empty)
Modules linked in:
CPU: 7 PID: 210 Comm: transhuge-stres Not tainted 6.1.0-rc3+ #748
Hardware name: linux,dummy-virt (DT)
pstate: 20000005 (nzCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
pc : page_table_check_set.isra.0+0x398/0x468
lr : page_table_check_set.isra.0+0x1c0/0x468
[...]
Call trace:
page_table_check_set.isra.0+0x398/0x468
__page_table_check_pte_set+0x160/0x1c0
__split_huge_pmd_locked+0x900/0x1648
__split_huge_pmd+0x28c/0x3b8
unmap_page_range+0x428/0x858
unmap_single_vma+0xf4/0x1c8
zap_page_range+0x2b0/0x410
madvise_vma_behavior+0xc44/0xe78
do_madvise+0x280/0x698
__arm64_sys_madvise+0x90/0xe8
invoke_syscall.constprop.0+0xdc/0x1d8
do_el0_svc+0xf4/0x3f8
el0_svc+0x58/0x120
el0t_64_sync_handler+0xb8/0xc0
el0t_64_sync+0x19c/0x1a0
[...]
On arm64, pmd_leaf() will return true even if the pmd is invalid due to
pmd_present_invalid() check. So in pmdp_invalidate() the file_map_count
will not only decrease once but also increase once. Then in set_pte_at(),
the file_map_count increase again, and so trigger BUG_ON() unexpectedly.
Fix this problem by adding pmd_valid() in pmd_user_accessible_page().
Moreover, add pud_valid() for pud_user_accessible_page() too.
Fixes: 42b2547137f5 ("arm64/mm: enable ARCH_SUPPORTS_PAGE_TABLE_CHECK")
Reported-by: Denys Vlasenko <dvlasenk@redhat.com>
Signed-off-by: Liu Shixin <liushixin2@huawei.com>
Acked-by: Pasha Tatashin <pasha.tatashin@soleen.com>
---
arch/arm64/include/asm/pgtable.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h
index edf6625ce965..3bc64199aa2e 100644
--- a/arch/arm64/include/asm/pgtable.h
+++ b/arch/arm64/include/asm/pgtable.h
@@ -863,12 +863,12 @@ static inline bool pte_user_accessible_page(pte_t pte)
static inline bool pmd_user_accessible_page(pmd_t pmd)
{
- return pmd_leaf(pmd) && (pmd_user(pmd) || pmd_user_exec(pmd));
+ return pmd_valid(pmd) && pmd_leaf(pmd) && (pmd_user(pmd) || pmd_user_exec(pmd));
}
static inline bool pud_user_accessible_page(pud_t pud)
{
- return pud_leaf(pud) && pud_user(pud);
+ return pud_valid(pud) && pud_leaf(pud) && pud_user(pud);
}
#endif
--
2.25.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 26+ messages in thread
* Re: [PATCH v2 1/2] arm64/mm: fix incorrect file_map_count for non-leaf pmd/pud
2022-11-17 7:56 ` Liu Shixin
@ 2022-11-17 7:56 ` Kefeng Wang
-1 siblings, 0 replies; 26+ messages in thread
From: Kefeng Wang @ 2022-11-17 7:56 UTC (permalink / raw)
To: Liu Shixin, Catalin Marinas, Will Deacon, Denys Vlasenko,
Anshuman Khandual, David Hildenbrand, Rafael Aquini,
Pasha Tatashin
Cc: linux-arm-kernel, linux-kernel
On 2022/11/17 15:56, Liu Shixin wrote:
> The page table check trigger BUG_ON() unexpectedly when collapse hugepage:
>
> ------------[ cut here ]------------
> kernel BUG at mm/page_table_check.c:82!
> Internal error: Oops - BUG: 00000000f2000800 [#1] SMP
> Dumping ftrace buffer:
> (ftrace buffer empty)
> Modules linked in:
> CPU: 6 PID: 68 Comm: khugepaged Not tainted 6.1.0-rc3+ #750
> Hardware name: linux,dummy-virt (DT)
> pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> pc : page_table_check_clear.isra.0+0x258/0x3f0
> lr : page_table_check_clear.isra.0+0x240/0x3f0
> [...]
> Call trace:
> page_table_check_clear.isra.0+0x258/0x3f0
> __page_table_check_pmd_clear+0xbc/0x108
> pmdp_collapse_flush+0xb0/0x160
> collapse_huge_page+0xa08/0x1080
> hpage_collapse_scan_pmd+0xf30/0x1590
> khugepaged_scan_mm_slot.constprop.0+0x52c/0xac8
> khugepaged+0x338/0x518
> kthread+0x278/0x2f8
> ret_from_fork+0x10/0x20
> [...]
>
> Since pmd_user_accessible_page() doesn't check if a pmd is leaf, it
> decrease file_map_count for a non-leaf pmd comes from collapse_huge_page().
> and so trigger BUG_ON() unexpectedly.
>
> Fix this problem by using pmd_leaf() insteal of pmd_present() in
> pmd_user_accessible_page(). Moreover, use pud_leaf() for
> pud_user_accessible_page() too.
>
> Fixes: 42b2547137f5 ("arm64/mm: enable ARCH_SUPPORTS_PAGE_TABLE_CHECK")
> Reported-by: Denys Vlasenko <dvlasenk@redhat.com>
> Signed-off-by: Liu Shixin <liushixin2@huawei.com>
> Reviewed-by: David Hildenbrand <david@redhat.com>
> Acked-by: Pasha Tatashin <pasha.tatashin@soleen.com>
Reviewed-by: Kefeng Wang <wangkefeng.wang@huawei.com>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v2 1/2] arm64/mm: fix incorrect file_map_count for non-leaf pmd/pud
@ 2022-11-17 7:56 ` Kefeng Wang
0 siblings, 0 replies; 26+ messages in thread
From: Kefeng Wang @ 2022-11-17 7:56 UTC (permalink / raw)
To: Liu Shixin, Catalin Marinas, Will Deacon, Denys Vlasenko,
Anshuman Khandual, David Hildenbrand, Rafael Aquini,
Pasha Tatashin
Cc: linux-arm-kernel, linux-kernel
On 2022/11/17 15:56, Liu Shixin wrote:
> The page table check trigger BUG_ON() unexpectedly when collapse hugepage:
>
> ------------[ cut here ]------------
> kernel BUG at mm/page_table_check.c:82!
> Internal error: Oops - BUG: 00000000f2000800 [#1] SMP
> Dumping ftrace buffer:
> (ftrace buffer empty)
> Modules linked in:
> CPU: 6 PID: 68 Comm: khugepaged Not tainted 6.1.0-rc3+ #750
> Hardware name: linux,dummy-virt (DT)
> pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> pc : page_table_check_clear.isra.0+0x258/0x3f0
> lr : page_table_check_clear.isra.0+0x240/0x3f0
> [...]
> Call trace:
> page_table_check_clear.isra.0+0x258/0x3f0
> __page_table_check_pmd_clear+0xbc/0x108
> pmdp_collapse_flush+0xb0/0x160
> collapse_huge_page+0xa08/0x1080
> hpage_collapse_scan_pmd+0xf30/0x1590
> khugepaged_scan_mm_slot.constprop.0+0x52c/0xac8
> khugepaged+0x338/0x518
> kthread+0x278/0x2f8
> ret_from_fork+0x10/0x20
> [...]
>
> Since pmd_user_accessible_page() doesn't check if a pmd is leaf, it
> decrease file_map_count for a non-leaf pmd comes from collapse_huge_page().
> and so trigger BUG_ON() unexpectedly.
>
> Fix this problem by using pmd_leaf() insteal of pmd_present() in
> pmd_user_accessible_page(). Moreover, use pud_leaf() for
> pud_user_accessible_page() too.
>
> Fixes: 42b2547137f5 ("arm64/mm: enable ARCH_SUPPORTS_PAGE_TABLE_CHECK")
> Reported-by: Denys Vlasenko <dvlasenk@redhat.com>
> Signed-off-by: Liu Shixin <liushixin2@huawei.com>
> Reviewed-by: David Hildenbrand <david@redhat.com>
> Acked-by: Pasha Tatashin <pasha.tatashin@soleen.com>
Reviewed-by: Kefeng Wang <wangkefeng.wang@huawei.com>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v2 2/2] arm64/mm: fix incorrect file_map_count for invalid pmd/pud
2022-11-17 7:56 ` Liu Shixin
@ 2022-11-17 7:56 ` Kefeng Wang
-1 siblings, 0 replies; 26+ messages in thread
From: Kefeng Wang @ 2022-11-17 7:56 UTC (permalink / raw)
To: Liu Shixin, Catalin Marinas, Will Deacon, Denys Vlasenko,
Anshuman Khandual, David Hildenbrand, Rafael Aquini,
Pasha Tatashin
Cc: linux-arm-kernel, linux-kernel
On 2022/11/17 15:56, Liu Shixin wrote:
> The page table check trigger BUG_ON() unexpectedly when split hugepage:
>
> ------------[ cut here ]------------
> kernel BUG at mm/page_table_check.c:119!
> Internal error: Oops - BUG: 00000000f2000800 [#1] SMP
> Dumping ftrace buffer:
> (ftrace buffer empty)
> Modules linked in:
> CPU: 7 PID: 210 Comm: transhuge-stres Not tainted 6.1.0-rc3+ #748
> Hardware name: linux,dummy-virt (DT)
> pstate: 20000005 (nzCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> pc : page_table_check_set.isra.0+0x398/0x468
> lr : page_table_check_set.isra.0+0x1c0/0x468
> [...]
> Call trace:
> page_table_check_set.isra.0+0x398/0x468
> __page_table_check_pte_set+0x160/0x1c0
> __split_huge_pmd_locked+0x900/0x1648
> __split_huge_pmd+0x28c/0x3b8
> unmap_page_range+0x428/0x858
> unmap_single_vma+0xf4/0x1c8
> zap_page_range+0x2b0/0x410
> madvise_vma_behavior+0xc44/0xe78
> do_madvise+0x280/0x698
> __arm64_sys_madvise+0x90/0xe8
> invoke_syscall.constprop.0+0xdc/0x1d8
> do_el0_svc+0xf4/0x3f8
> el0_svc+0x58/0x120
> el0t_64_sync_handler+0xb8/0xc0
> el0t_64_sync+0x19c/0x1a0
> [...]
>
> On arm64, pmd_leaf() will return true even if the pmd is invalid due to
> pmd_present_invalid() check. So in pmdp_invalidate() the file_map_count
> will not only decrease once but also increase once. Then in set_pte_at(),
> the file_map_count increase again, and so trigger BUG_ON() unexpectedly.
>
> Fix this problem by adding pmd_valid() in pmd_user_accessible_page().
> Moreover, add pud_valid() for pud_user_accessible_page() too.
>
> Fixes: 42b2547137f5 ("arm64/mm: enable ARCH_SUPPORTS_PAGE_TABLE_CHECK")
> Reported-by: Denys Vlasenko <dvlasenk@redhat.com>
> Signed-off-by: Liu Shixin <liushixin2@huawei.com>
> Acked-by: Pasha Tatashin <pasha.tatashin@soleen.com>
Reviewed-by: Kefeng Wang <wangkefeng.wang@huawei.com>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v2 2/2] arm64/mm: fix incorrect file_map_count for invalid pmd/pud
@ 2022-11-17 7:56 ` Kefeng Wang
0 siblings, 0 replies; 26+ messages in thread
From: Kefeng Wang @ 2022-11-17 7:56 UTC (permalink / raw)
To: Liu Shixin, Catalin Marinas, Will Deacon, Denys Vlasenko,
Anshuman Khandual, David Hildenbrand, Rafael Aquini,
Pasha Tatashin
Cc: linux-arm-kernel, linux-kernel
On 2022/11/17 15:56, Liu Shixin wrote:
> The page table check trigger BUG_ON() unexpectedly when split hugepage:
>
> ------------[ cut here ]------------
> kernel BUG at mm/page_table_check.c:119!
> Internal error: Oops - BUG: 00000000f2000800 [#1] SMP
> Dumping ftrace buffer:
> (ftrace buffer empty)
> Modules linked in:
> CPU: 7 PID: 210 Comm: transhuge-stres Not tainted 6.1.0-rc3+ #748
> Hardware name: linux,dummy-virt (DT)
> pstate: 20000005 (nzCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> pc : page_table_check_set.isra.0+0x398/0x468
> lr : page_table_check_set.isra.0+0x1c0/0x468
> [...]
> Call trace:
> page_table_check_set.isra.0+0x398/0x468
> __page_table_check_pte_set+0x160/0x1c0
> __split_huge_pmd_locked+0x900/0x1648
> __split_huge_pmd+0x28c/0x3b8
> unmap_page_range+0x428/0x858
> unmap_single_vma+0xf4/0x1c8
> zap_page_range+0x2b0/0x410
> madvise_vma_behavior+0xc44/0xe78
> do_madvise+0x280/0x698
> __arm64_sys_madvise+0x90/0xe8
> invoke_syscall.constprop.0+0xdc/0x1d8
> do_el0_svc+0xf4/0x3f8
> el0_svc+0x58/0x120
> el0t_64_sync_handler+0xb8/0xc0
> el0t_64_sync+0x19c/0x1a0
> [...]
>
> On arm64, pmd_leaf() will return true even if the pmd is invalid due to
> pmd_present_invalid() check. So in pmdp_invalidate() the file_map_count
> will not only decrease once but also increase once. Then in set_pte_at(),
> the file_map_count increase again, and so trigger BUG_ON() unexpectedly.
>
> Fix this problem by adding pmd_valid() in pmd_user_accessible_page().
> Moreover, add pud_valid() for pud_user_accessible_page() too.
>
> Fixes: 42b2547137f5 ("arm64/mm: enable ARCH_SUPPORTS_PAGE_TABLE_CHECK")
> Reported-by: Denys Vlasenko <dvlasenk@redhat.com>
> Signed-off-by: Liu Shixin <liushixin2@huawei.com>
> Acked-by: Pasha Tatashin <pasha.tatashin@soleen.com>
Reviewed-by: Kefeng Wang <wangkefeng.wang@huawei.com>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v2 2/2] arm64/mm: fix incorrect file_map_count for invalid pmd/pud
2022-11-17 7:56 ` Liu Shixin
@ 2022-11-18 14:34 ` Will Deacon
-1 siblings, 0 replies; 26+ messages in thread
From: Will Deacon @ 2022-11-18 14:34 UTC (permalink / raw)
To: Liu Shixin
Cc: Catalin Marinas, Denys Vlasenko, Kefeng Wang, Anshuman Khandual,
David Hildenbrand, Rafael Aquini, Pasha Tatashin,
linux-arm-kernel, linux-kernel
On Thu, Nov 17, 2022 at 03:56:02PM +0800, Liu Shixin wrote:
> The page table check trigger BUG_ON() unexpectedly when split hugepage:
>
> ------------[ cut here ]------------
> kernel BUG at mm/page_table_check.c:119!
> Internal error: Oops - BUG: 00000000f2000800 [#1] SMP
> Dumping ftrace buffer:
> (ftrace buffer empty)
> Modules linked in:
> CPU: 7 PID: 210 Comm: transhuge-stres Not tainted 6.1.0-rc3+ #748
> Hardware name: linux,dummy-virt (DT)
> pstate: 20000005 (nzCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> pc : page_table_check_set.isra.0+0x398/0x468
> lr : page_table_check_set.isra.0+0x1c0/0x468
> [...]
> Call trace:
> page_table_check_set.isra.0+0x398/0x468
> __page_table_check_pte_set+0x160/0x1c0
> __split_huge_pmd_locked+0x900/0x1648
> __split_huge_pmd+0x28c/0x3b8
> unmap_page_range+0x428/0x858
> unmap_single_vma+0xf4/0x1c8
> zap_page_range+0x2b0/0x410
> madvise_vma_behavior+0xc44/0xe78
> do_madvise+0x280/0x698
> __arm64_sys_madvise+0x90/0xe8
> invoke_syscall.constprop.0+0xdc/0x1d8
> do_el0_svc+0xf4/0x3f8
> el0_svc+0x58/0x120
> el0t_64_sync_handler+0xb8/0xc0
> el0t_64_sync+0x19c/0x1a0
> [...]
>
> On arm64, pmd_leaf() will return true even if the pmd is invalid due to
> pmd_present_invalid() check. So in pmdp_invalidate() the file_map_count
> will not only decrease once but also increase once. Then in set_pte_at(),
> the file_map_count increase again, and so trigger BUG_ON() unexpectedly.
>
> Fix this problem by adding pmd_valid() in pmd_user_accessible_page().
> Moreover, add pud_valid() for pud_user_accessible_page() too.
>
> Fixes: 42b2547137f5 ("arm64/mm: enable ARCH_SUPPORTS_PAGE_TABLE_CHECK")
> Reported-by: Denys Vlasenko <dvlasenk@redhat.com>
> Signed-off-by: Liu Shixin <liushixin2@huawei.com>
> Acked-by: Pasha Tatashin <pasha.tatashin@soleen.com>
> ---
> arch/arm64/include/asm/pgtable.h | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h
> index edf6625ce965..3bc64199aa2e 100644
> --- a/arch/arm64/include/asm/pgtable.h
> +++ b/arch/arm64/include/asm/pgtable.h
> @@ -863,12 +863,12 @@ static inline bool pte_user_accessible_page(pte_t pte)
>
> static inline bool pmd_user_accessible_page(pmd_t pmd)
> {
> - return pmd_leaf(pmd) && (pmd_user(pmd) || pmd_user_exec(pmd));
> + return pmd_valid(pmd) && pmd_leaf(pmd) && (pmd_user(pmd) || pmd_user_exec(pmd));
Hmm, doesn't this have a funny interaction with PROT_NONE where the pmd is
invalid but present? If you don't care about PROT_NONE, then you could just
do:
pmd_valid(pmd) && !pmd_table(pmd) && (pmd_user(pmd) || pmd_user_exec(pmd))
but if you do care then you could do:
pmd_leaf(pmd) && !pmd_present_invalid(pmd) && (pmd_user(pmd) || pmd_user_exec(pmd))
> static inline bool pud_user_accessible_page(pud_t pud)
> {
> - return pud_leaf(pud) && pud_user(pud);
> + return pud_valid(pud) && pud_leaf(pud) && pud_user(pud);
Not caused by this patch, but why don't we have something like a
pud_user_exec() check here like we do for the pte and pmd levels?
Will
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v2 2/2] arm64/mm: fix incorrect file_map_count for invalid pmd/pud
@ 2022-11-18 14:34 ` Will Deacon
0 siblings, 0 replies; 26+ messages in thread
From: Will Deacon @ 2022-11-18 14:34 UTC (permalink / raw)
To: Liu Shixin
Cc: Catalin Marinas, Denys Vlasenko, Kefeng Wang, Anshuman Khandual,
David Hildenbrand, Rafael Aquini, Pasha Tatashin,
linux-arm-kernel, linux-kernel
On Thu, Nov 17, 2022 at 03:56:02PM +0800, Liu Shixin wrote:
> The page table check trigger BUG_ON() unexpectedly when split hugepage:
>
> ------------[ cut here ]------------
> kernel BUG at mm/page_table_check.c:119!
> Internal error: Oops - BUG: 00000000f2000800 [#1] SMP
> Dumping ftrace buffer:
> (ftrace buffer empty)
> Modules linked in:
> CPU: 7 PID: 210 Comm: transhuge-stres Not tainted 6.1.0-rc3+ #748
> Hardware name: linux,dummy-virt (DT)
> pstate: 20000005 (nzCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> pc : page_table_check_set.isra.0+0x398/0x468
> lr : page_table_check_set.isra.0+0x1c0/0x468
> [...]
> Call trace:
> page_table_check_set.isra.0+0x398/0x468
> __page_table_check_pte_set+0x160/0x1c0
> __split_huge_pmd_locked+0x900/0x1648
> __split_huge_pmd+0x28c/0x3b8
> unmap_page_range+0x428/0x858
> unmap_single_vma+0xf4/0x1c8
> zap_page_range+0x2b0/0x410
> madvise_vma_behavior+0xc44/0xe78
> do_madvise+0x280/0x698
> __arm64_sys_madvise+0x90/0xe8
> invoke_syscall.constprop.0+0xdc/0x1d8
> do_el0_svc+0xf4/0x3f8
> el0_svc+0x58/0x120
> el0t_64_sync_handler+0xb8/0xc0
> el0t_64_sync+0x19c/0x1a0
> [...]
>
> On arm64, pmd_leaf() will return true even if the pmd is invalid due to
> pmd_present_invalid() check. So in pmdp_invalidate() the file_map_count
> will not only decrease once but also increase once. Then in set_pte_at(),
> the file_map_count increase again, and so trigger BUG_ON() unexpectedly.
>
> Fix this problem by adding pmd_valid() in pmd_user_accessible_page().
> Moreover, add pud_valid() for pud_user_accessible_page() too.
>
> Fixes: 42b2547137f5 ("arm64/mm: enable ARCH_SUPPORTS_PAGE_TABLE_CHECK")
> Reported-by: Denys Vlasenko <dvlasenk@redhat.com>
> Signed-off-by: Liu Shixin <liushixin2@huawei.com>
> Acked-by: Pasha Tatashin <pasha.tatashin@soleen.com>
> ---
> arch/arm64/include/asm/pgtable.h | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h
> index edf6625ce965..3bc64199aa2e 100644
> --- a/arch/arm64/include/asm/pgtable.h
> +++ b/arch/arm64/include/asm/pgtable.h
> @@ -863,12 +863,12 @@ static inline bool pte_user_accessible_page(pte_t pte)
>
> static inline bool pmd_user_accessible_page(pmd_t pmd)
> {
> - return pmd_leaf(pmd) && (pmd_user(pmd) || pmd_user_exec(pmd));
> + return pmd_valid(pmd) && pmd_leaf(pmd) && (pmd_user(pmd) || pmd_user_exec(pmd));
Hmm, doesn't this have a funny interaction with PROT_NONE where the pmd is
invalid but present? If you don't care about PROT_NONE, then you could just
do:
pmd_valid(pmd) && !pmd_table(pmd) && (pmd_user(pmd) || pmd_user_exec(pmd))
but if you do care then you could do:
pmd_leaf(pmd) && !pmd_present_invalid(pmd) && (pmd_user(pmd) || pmd_user_exec(pmd))
> static inline bool pud_user_accessible_page(pud_t pud)
> {
> - return pud_leaf(pud) && pud_user(pud);
> + return pud_valid(pud) && pud_leaf(pud) && pud_user(pud);
Not caused by this patch, but why don't we have something like a
pud_user_exec() check here like we do for the pte and pmd levels?
Will
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v2 1/2] arm64/mm: fix incorrect file_map_count for non-leaf pmd/pud
2022-11-17 7:56 ` Liu Shixin
@ 2022-11-18 14:35 ` Will Deacon
-1 siblings, 0 replies; 26+ messages in thread
From: Will Deacon @ 2022-11-18 14:35 UTC (permalink / raw)
To: Liu Shixin
Cc: Catalin Marinas, Denys Vlasenko, Kefeng Wang, Anshuman Khandual,
David Hildenbrand, Rafael Aquini, Pasha Tatashin,
linux-arm-kernel, linux-kernel
On Thu, Nov 17, 2022 at 03:56:01PM +0800, Liu Shixin wrote:
> The page table check trigger BUG_ON() unexpectedly when collapse hugepage:
>
> ------------[ cut here ]------------
> kernel BUG at mm/page_table_check.c:82!
> Internal error: Oops - BUG: 00000000f2000800 [#1] SMP
> Dumping ftrace buffer:
> (ftrace buffer empty)
> Modules linked in:
> CPU: 6 PID: 68 Comm: khugepaged Not tainted 6.1.0-rc3+ #750
> Hardware name: linux,dummy-virt (DT)
> pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> pc : page_table_check_clear.isra.0+0x258/0x3f0
> lr : page_table_check_clear.isra.0+0x240/0x3f0
> [...]
> Call trace:
> page_table_check_clear.isra.0+0x258/0x3f0
> __page_table_check_pmd_clear+0xbc/0x108
> pmdp_collapse_flush+0xb0/0x160
> collapse_huge_page+0xa08/0x1080
> hpage_collapse_scan_pmd+0xf30/0x1590
> khugepaged_scan_mm_slot.constprop.0+0x52c/0xac8
> khugepaged+0x338/0x518
> kthread+0x278/0x2f8
> ret_from_fork+0x10/0x20
> [...]
>
> Since pmd_user_accessible_page() doesn't check if a pmd is leaf, it
> decrease file_map_count for a non-leaf pmd comes from collapse_huge_page().
> and so trigger BUG_ON() unexpectedly.
>
> Fix this problem by using pmd_leaf() insteal of pmd_present() in
> pmd_user_accessible_page(). Moreover, use pud_leaf() for
> pud_user_accessible_page() too.
>
> Fixes: 42b2547137f5 ("arm64/mm: enable ARCH_SUPPORTS_PAGE_TABLE_CHECK")
> Reported-by: Denys Vlasenko <dvlasenk@redhat.com>
> Signed-off-by: Liu Shixin <liushixin2@huawei.com>
> Reviewed-by: David Hildenbrand <david@redhat.com>
> Acked-by: Pasha Tatashin <pasha.tatashin@soleen.com>
> ---
> arch/arm64/include/asm/pgtable.h | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h
> index 71a1af42f0e8..edf6625ce965 100644
> --- a/arch/arm64/include/asm/pgtable.h
> +++ b/arch/arm64/include/asm/pgtable.h
> @@ -863,12 +863,12 @@ static inline bool pte_user_accessible_page(pte_t pte)
>
> static inline bool pmd_user_accessible_page(pmd_t pmd)
> {
> - return pmd_present(pmd) && (pmd_user(pmd) || pmd_user_exec(pmd));
> + return pmd_leaf(pmd) && (pmd_user(pmd) || pmd_user_exec(pmd));
> }
>
> static inline bool pud_user_accessible_page(pud_t pud)
> {
> - return pud_present(pud) && pud_user(pud);
> + return pud_leaf(pud) && pud_user(pud);
Acked-by: Will Deacon <will@kernel.org>
Will
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v2 1/2] arm64/mm: fix incorrect file_map_count for non-leaf pmd/pud
@ 2022-11-18 14:35 ` Will Deacon
0 siblings, 0 replies; 26+ messages in thread
From: Will Deacon @ 2022-11-18 14:35 UTC (permalink / raw)
To: Liu Shixin
Cc: Catalin Marinas, Denys Vlasenko, Kefeng Wang, Anshuman Khandual,
David Hildenbrand, Rafael Aquini, Pasha Tatashin,
linux-arm-kernel, linux-kernel
On Thu, Nov 17, 2022 at 03:56:01PM +0800, Liu Shixin wrote:
> The page table check trigger BUG_ON() unexpectedly when collapse hugepage:
>
> ------------[ cut here ]------------
> kernel BUG at mm/page_table_check.c:82!
> Internal error: Oops - BUG: 00000000f2000800 [#1] SMP
> Dumping ftrace buffer:
> (ftrace buffer empty)
> Modules linked in:
> CPU: 6 PID: 68 Comm: khugepaged Not tainted 6.1.0-rc3+ #750
> Hardware name: linux,dummy-virt (DT)
> pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> pc : page_table_check_clear.isra.0+0x258/0x3f0
> lr : page_table_check_clear.isra.0+0x240/0x3f0
> [...]
> Call trace:
> page_table_check_clear.isra.0+0x258/0x3f0
> __page_table_check_pmd_clear+0xbc/0x108
> pmdp_collapse_flush+0xb0/0x160
> collapse_huge_page+0xa08/0x1080
> hpage_collapse_scan_pmd+0xf30/0x1590
> khugepaged_scan_mm_slot.constprop.0+0x52c/0xac8
> khugepaged+0x338/0x518
> kthread+0x278/0x2f8
> ret_from_fork+0x10/0x20
> [...]
>
> Since pmd_user_accessible_page() doesn't check if a pmd is leaf, it
> decrease file_map_count for a non-leaf pmd comes from collapse_huge_page().
> and so trigger BUG_ON() unexpectedly.
>
> Fix this problem by using pmd_leaf() insteal of pmd_present() in
> pmd_user_accessible_page(). Moreover, use pud_leaf() for
> pud_user_accessible_page() too.
>
> Fixes: 42b2547137f5 ("arm64/mm: enable ARCH_SUPPORTS_PAGE_TABLE_CHECK")
> Reported-by: Denys Vlasenko <dvlasenk@redhat.com>
> Signed-off-by: Liu Shixin <liushixin2@huawei.com>
> Reviewed-by: David Hildenbrand <david@redhat.com>
> Acked-by: Pasha Tatashin <pasha.tatashin@soleen.com>
> ---
> arch/arm64/include/asm/pgtable.h | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h
> index 71a1af42f0e8..edf6625ce965 100644
> --- a/arch/arm64/include/asm/pgtable.h
> +++ b/arch/arm64/include/asm/pgtable.h
> @@ -863,12 +863,12 @@ static inline bool pte_user_accessible_page(pte_t pte)
>
> static inline bool pmd_user_accessible_page(pmd_t pmd)
> {
> - return pmd_present(pmd) && (pmd_user(pmd) || pmd_user_exec(pmd));
> + return pmd_leaf(pmd) && (pmd_user(pmd) || pmd_user_exec(pmd));
> }
>
> static inline bool pud_user_accessible_page(pud_t pud)
> {
> - return pud_present(pud) && pud_user(pud);
> + return pud_leaf(pud) && pud_user(pud);
Acked-by: Will Deacon <will@kernel.org>
Will
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: (subset) [PATCH v2 0/2] arm64: fix two bug about page table check
2022-11-17 7:56 ` Liu Shixin
@ 2022-11-18 19:35 ` Catalin Marinas
-1 siblings, 0 replies; 26+ messages in thread
From: Catalin Marinas @ 2022-11-18 19:35 UTC (permalink / raw)
To: David Hildenbrand, Pasha Tatashin, Anshuman Khandual, Liu Shixin,
Rafael Aquini, Denys Vlasenko, Kefeng Wang, Will Deacon
Cc: linux-kernel, linux-arm-kernel
On Thu, 17 Nov 2022 15:56:00 +0800, Liu Shixin wrote:
> Denys Vlasenko has reported two bug about page table check on arm64.
> On arm64, pmd_present() contains non-leaf pmd and invalid pmd too.
>
> When collapse hugepage, the pmd is non-leaf and should skip the check.
> Use pmd_leaf() instead of pmd_present().
>
> When split hugepage, the pmd will be marked as invalid and then populate.
> So we should decrease file_map_count when invalid pmd and then increase
> when populate the pmd. Add pmd_valid() check.
>
> [...]
Applied to arm64 (for-next/fixes), thanks!
[1/2] arm64/mm: fix incorrect file_map_count for non-leaf pmd/pud
https://git.kernel.org/arm64/c/5b47348fc0b1
I only merged the first patch in this series as Will had some questions
on the second patch (it does seem weird that the pud and pmd functions
are different w.r.t. the p*d_user() checks).
--
Catalin
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: (subset) [PATCH v2 0/2] arm64: fix two bug about page table check
@ 2022-11-18 19:35 ` Catalin Marinas
0 siblings, 0 replies; 26+ messages in thread
From: Catalin Marinas @ 2022-11-18 19:35 UTC (permalink / raw)
To: David Hildenbrand, Pasha Tatashin, Anshuman Khandual, Liu Shixin,
Rafael Aquini, Denys Vlasenko, Kefeng Wang, Will Deacon
Cc: linux-kernel, linux-arm-kernel
On Thu, 17 Nov 2022 15:56:00 +0800, Liu Shixin wrote:
> Denys Vlasenko has reported two bug about page table check on arm64.
> On arm64, pmd_present() contains non-leaf pmd and invalid pmd too.
>
> When collapse hugepage, the pmd is non-leaf and should skip the check.
> Use pmd_leaf() instead of pmd_present().
>
> When split hugepage, the pmd will be marked as invalid and then populate.
> So we should decrease file_map_count when invalid pmd and then increase
> when populate the pmd. Add pmd_valid() check.
>
> [...]
Applied to arm64 (for-next/fixes), thanks!
[1/2] arm64/mm: fix incorrect file_map_count for non-leaf pmd/pud
https://git.kernel.org/arm64/c/5b47348fc0b1
I only merged the first patch in this series as Will had some questions
on the second patch (it does seem weird that the pud and pmd functions
are different w.r.t. the p*d_user() checks).
--
Catalin
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v2 2/2] arm64/mm: fix incorrect file_map_count for invalid pmd/pud
2022-11-18 14:34 ` Will Deacon
@ 2022-11-21 3:15 ` Liu Shixin
-1 siblings, 0 replies; 26+ messages in thread
From: Liu Shixin @ 2022-11-21 3:15 UTC (permalink / raw)
To: Will Deacon
Cc: Catalin Marinas, Denys Vlasenko, Kefeng Wang, Anshuman Khandual,
David Hildenbrand, Rafael Aquini, Pasha Tatashin,
linux-arm-kernel, linux-kernel
On 2022/11/18 22:34, Will Deacon wrote:
> On Thu, Nov 17, 2022 at 03:56:02PM +0800, Liu Shixin wrote:
>> The page table check trigger BUG_ON() unexpectedly when split hugepage:
>>
>> ------------[ cut here ]------------
>> kernel BUG at mm/page_table_check.c:119!
>> Internal error: Oops - BUG: 00000000f2000800 [#1] SMP
>> Dumping ftrace buffer:
>> (ftrace buffer empty)
>> Modules linked in:
>> CPU: 7 PID: 210 Comm: transhuge-stres Not tainted 6.1.0-rc3+ #748
>> Hardware name: linux,dummy-virt (DT)
>> pstate: 20000005 (nzCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
>> pc : page_table_check_set.isra.0+0x398/0x468
>> lr : page_table_check_set.isra.0+0x1c0/0x468
>> [...]
>> Call trace:
>> page_table_check_set.isra.0+0x398/0x468
>> __page_table_check_pte_set+0x160/0x1c0
>> __split_huge_pmd_locked+0x900/0x1648
>> __split_huge_pmd+0x28c/0x3b8
>> unmap_page_range+0x428/0x858
>> unmap_single_vma+0xf4/0x1c8
>> zap_page_range+0x2b0/0x410
>> madvise_vma_behavior+0xc44/0xe78
>> do_madvise+0x280/0x698
>> __arm64_sys_madvise+0x90/0xe8
>> invoke_syscall.constprop.0+0xdc/0x1d8
>> do_el0_svc+0xf4/0x3f8
>> el0_svc+0x58/0x120
>> el0t_64_sync_handler+0xb8/0xc0
>> el0t_64_sync+0x19c/0x1a0
>> [...]
>>
>> On arm64, pmd_leaf() will return true even if the pmd is invalid due to
>> pmd_present_invalid() check. So in pmdp_invalidate() the file_map_count
>> will not only decrease once but also increase once. Then in set_pte_at(),
>> the file_map_count increase again, and so trigger BUG_ON() unexpectedly.
>>
>> Fix this problem by adding pmd_valid() in pmd_user_accessible_page().
>> Moreover, add pud_valid() for pud_user_accessible_page() too.
>>
>> Fixes: 42b2547137f5 ("arm64/mm: enable ARCH_SUPPORTS_PAGE_TABLE_CHECK")
>> Reported-by: Denys Vlasenko <dvlasenk@redhat.com>
>> Signed-off-by: Liu Shixin <liushixin2@huawei.com>
>> Acked-by: Pasha Tatashin <pasha.tatashin@soleen.com>
>> ---
>> arch/arm64/include/asm/pgtable.h | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h
>> index edf6625ce965..3bc64199aa2e 100644
>> --- a/arch/arm64/include/asm/pgtable.h
>> +++ b/arch/arm64/include/asm/pgtable.h
>> @@ -863,12 +863,12 @@ static inline bool pte_user_accessible_page(pte_t pte)
>>
>> static inline bool pmd_user_accessible_page(pmd_t pmd)
>> {
>> - return pmd_leaf(pmd) && (pmd_user(pmd) || pmd_user_exec(pmd));
>> + return pmd_valid(pmd) && pmd_leaf(pmd) && (pmd_user(pmd) || pmd_user_exec(pmd));
> Hmm, doesn't this have a funny interaction with PROT_NONE where the pmd is
> invalid but present? If you don't care about PROT_NONE, then you could just
> do:
>
> pmd_valid(pmd) && !pmd_table(pmd) && (pmd_user(pmd) || pmd_user_exec(pmd))
>
> but if you do care then you could do:
>
> pmd_leaf(pmd) && !pmd_present_invalid(pmd) && (pmd_user(pmd) || pmd_user_exec(pmd))
I prefer the latter. I will fix and resend later.
>> static inline bool pud_user_accessible_page(pud_t pud)
>> {
>> - return pud_leaf(pud) && pud_user(pud);
>> + return pud_valid(pud) && pud_leaf(pud) && pud_user(pud);
> Not caused by this patch, but why don't we have something like a
> pud_user_exec() check here like we do for the pte and pmd levels?
As far as I know, there is no user use the user executable pud on arm64, so didn't define pud_user_exec().
Thanks,
>
> Will
> .
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v2 2/2] arm64/mm: fix incorrect file_map_count for invalid pmd/pud
@ 2022-11-21 3:15 ` Liu Shixin
0 siblings, 0 replies; 26+ messages in thread
From: Liu Shixin @ 2022-11-21 3:15 UTC (permalink / raw)
To: Will Deacon
Cc: Catalin Marinas, Denys Vlasenko, Kefeng Wang, Anshuman Khandual,
David Hildenbrand, Rafael Aquini, Pasha Tatashin,
linux-arm-kernel, linux-kernel
On 2022/11/18 22:34, Will Deacon wrote:
> On Thu, Nov 17, 2022 at 03:56:02PM +0800, Liu Shixin wrote:
>> The page table check trigger BUG_ON() unexpectedly when split hugepage:
>>
>> ------------[ cut here ]------------
>> kernel BUG at mm/page_table_check.c:119!
>> Internal error: Oops - BUG: 00000000f2000800 [#1] SMP
>> Dumping ftrace buffer:
>> (ftrace buffer empty)
>> Modules linked in:
>> CPU: 7 PID: 210 Comm: transhuge-stres Not tainted 6.1.0-rc3+ #748
>> Hardware name: linux,dummy-virt (DT)
>> pstate: 20000005 (nzCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
>> pc : page_table_check_set.isra.0+0x398/0x468
>> lr : page_table_check_set.isra.0+0x1c0/0x468
>> [...]
>> Call trace:
>> page_table_check_set.isra.0+0x398/0x468
>> __page_table_check_pte_set+0x160/0x1c0
>> __split_huge_pmd_locked+0x900/0x1648
>> __split_huge_pmd+0x28c/0x3b8
>> unmap_page_range+0x428/0x858
>> unmap_single_vma+0xf4/0x1c8
>> zap_page_range+0x2b0/0x410
>> madvise_vma_behavior+0xc44/0xe78
>> do_madvise+0x280/0x698
>> __arm64_sys_madvise+0x90/0xe8
>> invoke_syscall.constprop.0+0xdc/0x1d8
>> do_el0_svc+0xf4/0x3f8
>> el0_svc+0x58/0x120
>> el0t_64_sync_handler+0xb8/0xc0
>> el0t_64_sync+0x19c/0x1a0
>> [...]
>>
>> On arm64, pmd_leaf() will return true even if the pmd is invalid due to
>> pmd_present_invalid() check. So in pmdp_invalidate() the file_map_count
>> will not only decrease once but also increase once. Then in set_pte_at(),
>> the file_map_count increase again, and so trigger BUG_ON() unexpectedly.
>>
>> Fix this problem by adding pmd_valid() in pmd_user_accessible_page().
>> Moreover, add pud_valid() for pud_user_accessible_page() too.
>>
>> Fixes: 42b2547137f5 ("arm64/mm: enable ARCH_SUPPORTS_PAGE_TABLE_CHECK")
>> Reported-by: Denys Vlasenko <dvlasenk@redhat.com>
>> Signed-off-by: Liu Shixin <liushixin2@huawei.com>
>> Acked-by: Pasha Tatashin <pasha.tatashin@soleen.com>
>> ---
>> arch/arm64/include/asm/pgtable.h | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h
>> index edf6625ce965..3bc64199aa2e 100644
>> --- a/arch/arm64/include/asm/pgtable.h
>> +++ b/arch/arm64/include/asm/pgtable.h
>> @@ -863,12 +863,12 @@ static inline bool pte_user_accessible_page(pte_t pte)
>>
>> static inline bool pmd_user_accessible_page(pmd_t pmd)
>> {
>> - return pmd_leaf(pmd) && (pmd_user(pmd) || pmd_user_exec(pmd));
>> + return pmd_valid(pmd) && pmd_leaf(pmd) && (pmd_user(pmd) || pmd_user_exec(pmd));
> Hmm, doesn't this have a funny interaction with PROT_NONE where the pmd is
> invalid but present? If you don't care about PROT_NONE, then you could just
> do:
>
> pmd_valid(pmd) && !pmd_table(pmd) && (pmd_user(pmd) || pmd_user_exec(pmd))
>
> but if you do care then you could do:
>
> pmd_leaf(pmd) && !pmd_present_invalid(pmd) && (pmd_user(pmd) || pmd_user_exec(pmd))
I prefer the latter. I will fix and resend later.
>> static inline bool pud_user_accessible_page(pud_t pud)
>> {
>> - return pud_leaf(pud) && pud_user(pud);
>> + return pud_valid(pud) && pud_leaf(pud) && pud_user(pud);
> Not caused by this patch, but why don't we have something like a
> pud_user_exec() check here like we do for the pte and pmd levels?
As far as I know, there is no user use the user executable pud on arm64, so didn't define pud_user_exec().
Thanks,
>
> Will
> .
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v2 2/2] arm64/mm: fix incorrect file_map_count for invalid pmd/pud
2022-11-21 3:15 ` Liu Shixin
@ 2022-11-21 18:16 ` Will Deacon
-1 siblings, 0 replies; 26+ messages in thread
From: Will Deacon @ 2022-11-21 18:16 UTC (permalink / raw)
To: Liu Shixin
Cc: Catalin Marinas, Denys Vlasenko, Kefeng Wang, Anshuman Khandual,
David Hildenbrand, Rafael Aquini, Pasha Tatashin,
linux-arm-kernel, linux-kernel
On Mon, Nov 21, 2022 at 11:15:49AM +0800, Liu Shixin wrote:
> On 2022/11/18 22:34, Will Deacon wrote:
> > On Thu, Nov 17, 2022 at 03:56:02PM +0800, Liu Shixin wrote:
> >> The page table check trigger BUG_ON() unexpectedly when split hugepage:
> >>
> >> ------------[ cut here ]------------
> >> kernel BUG at mm/page_table_check.c:119!
> >> Internal error: Oops - BUG: 00000000f2000800 [#1] SMP
> >> Dumping ftrace buffer:
> >> (ftrace buffer empty)
> >> Modules linked in:
> >> CPU: 7 PID: 210 Comm: transhuge-stres Not tainted 6.1.0-rc3+ #748
> >> Hardware name: linux,dummy-virt (DT)
> >> pstate: 20000005 (nzCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> >> pc : page_table_check_set.isra.0+0x398/0x468
> >> lr : page_table_check_set.isra.0+0x1c0/0x468
> >> [...]
> >> Call trace:
> >> page_table_check_set.isra.0+0x398/0x468
> >> __page_table_check_pte_set+0x160/0x1c0
> >> __split_huge_pmd_locked+0x900/0x1648
> >> __split_huge_pmd+0x28c/0x3b8
> >> unmap_page_range+0x428/0x858
> >> unmap_single_vma+0xf4/0x1c8
> >> zap_page_range+0x2b0/0x410
> >> madvise_vma_behavior+0xc44/0xe78
> >> do_madvise+0x280/0x698
> >> __arm64_sys_madvise+0x90/0xe8
> >> invoke_syscall.constprop.0+0xdc/0x1d8
> >> do_el0_svc+0xf4/0x3f8
> >> el0_svc+0x58/0x120
> >> el0t_64_sync_handler+0xb8/0xc0
> >> el0t_64_sync+0x19c/0x1a0
> >> [...]
> >>
> >> On arm64, pmd_leaf() will return true even if the pmd is invalid due to
> >> pmd_present_invalid() check. So in pmdp_invalidate() the file_map_count
> >> will not only decrease once but also increase once. Then in set_pte_at(),
> >> the file_map_count increase again, and so trigger BUG_ON() unexpectedly.
> >>
> >> Fix this problem by adding pmd_valid() in pmd_user_accessible_page().
> >> Moreover, add pud_valid() for pud_user_accessible_page() too.
> >>
> >> Fixes: 42b2547137f5 ("arm64/mm: enable ARCH_SUPPORTS_PAGE_TABLE_CHECK")
> >> Reported-by: Denys Vlasenko <dvlasenk@redhat.com>
> >> Signed-off-by: Liu Shixin <liushixin2@huawei.com>
> >> Acked-by: Pasha Tatashin <pasha.tatashin@soleen.com>
> >> ---
> >> arch/arm64/include/asm/pgtable.h | 4 ++--
> >> 1 file changed, 2 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h
> >> index edf6625ce965..3bc64199aa2e 100644
> >> --- a/arch/arm64/include/asm/pgtable.h
> >> +++ b/arch/arm64/include/asm/pgtable.h
> >> @@ -863,12 +863,12 @@ static inline bool pte_user_accessible_page(pte_t pte)
> >>
> >> static inline bool pmd_user_accessible_page(pmd_t pmd)
> >> {
> >> - return pmd_leaf(pmd) && (pmd_user(pmd) || pmd_user_exec(pmd));
> >> + return pmd_valid(pmd) && pmd_leaf(pmd) && (pmd_user(pmd) || pmd_user_exec(pmd));
> > Hmm, doesn't this have a funny interaction with PROT_NONE where the pmd is
> > invalid but present? If you don't care about PROT_NONE, then you could just
> > do:
> >
> > pmd_valid(pmd) && !pmd_table(pmd) && (pmd_user(pmd) || pmd_user_exec(pmd))
> >
> > but if you do care then you could do:
> >
> > pmd_leaf(pmd) && !pmd_present_invalid(pmd) && (pmd_user(pmd) || pmd_user_exec(pmd))
> I prefer the latter. I will fix and resend later.
> >> static inline bool pud_user_accessible_page(pud_t pud)
> >> {
> >> - return pud_leaf(pud) && pud_user(pud);
> >> + return pud_valid(pud) && pud_leaf(pud) && pud_user(pud);
> > Not caused by this patch, but why don't we have something like a
> > pud_user_exec() check here like we do for the pte and pmd levels?
> As far as I know, there is no user use the user executable pud on arm64, so didn't define pud_user_exec().
I can believe they don't get exposed to userspace at all, but exposing only
as non-executable doesn't sound right. So I would have thought that either
pud_user_accessible_page() would always return false or it would need to
check for the executable case too.
Will
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v2 2/2] arm64/mm: fix incorrect file_map_count for invalid pmd/pud
@ 2022-11-21 18:16 ` Will Deacon
0 siblings, 0 replies; 26+ messages in thread
From: Will Deacon @ 2022-11-21 18:16 UTC (permalink / raw)
To: Liu Shixin
Cc: Catalin Marinas, Denys Vlasenko, Kefeng Wang, Anshuman Khandual,
David Hildenbrand, Rafael Aquini, Pasha Tatashin,
linux-arm-kernel, linux-kernel
On Mon, Nov 21, 2022 at 11:15:49AM +0800, Liu Shixin wrote:
> On 2022/11/18 22:34, Will Deacon wrote:
> > On Thu, Nov 17, 2022 at 03:56:02PM +0800, Liu Shixin wrote:
> >> The page table check trigger BUG_ON() unexpectedly when split hugepage:
> >>
> >> ------------[ cut here ]------------
> >> kernel BUG at mm/page_table_check.c:119!
> >> Internal error: Oops - BUG: 00000000f2000800 [#1] SMP
> >> Dumping ftrace buffer:
> >> (ftrace buffer empty)
> >> Modules linked in:
> >> CPU: 7 PID: 210 Comm: transhuge-stres Not tainted 6.1.0-rc3+ #748
> >> Hardware name: linux,dummy-virt (DT)
> >> pstate: 20000005 (nzCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> >> pc : page_table_check_set.isra.0+0x398/0x468
> >> lr : page_table_check_set.isra.0+0x1c0/0x468
> >> [...]
> >> Call trace:
> >> page_table_check_set.isra.0+0x398/0x468
> >> __page_table_check_pte_set+0x160/0x1c0
> >> __split_huge_pmd_locked+0x900/0x1648
> >> __split_huge_pmd+0x28c/0x3b8
> >> unmap_page_range+0x428/0x858
> >> unmap_single_vma+0xf4/0x1c8
> >> zap_page_range+0x2b0/0x410
> >> madvise_vma_behavior+0xc44/0xe78
> >> do_madvise+0x280/0x698
> >> __arm64_sys_madvise+0x90/0xe8
> >> invoke_syscall.constprop.0+0xdc/0x1d8
> >> do_el0_svc+0xf4/0x3f8
> >> el0_svc+0x58/0x120
> >> el0t_64_sync_handler+0xb8/0xc0
> >> el0t_64_sync+0x19c/0x1a0
> >> [...]
> >>
> >> On arm64, pmd_leaf() will return true even if the pmd is invalid due to
> >> pmd_present_invalid() check. So in pmdp_invalidate() the file_map_count
> >> will not only decrease once but also increase once. Then in set_pte_at(),
> >> the file_map_count increase again, and so trigger BUG_ON() unexpectedly.
> >>
> >> Fix this problem by adding pmd_valid() in pmd_user_accessible_page().
> >> Moreover, add pud_valid() for pud_user_accessible_page() too.
> >>
> >> Fixes: 42b2547137f5 ("arm64/mm: enable ARCH_SUPPORTS_PAGE_TABLE_CHECK")
> >> Reported-by: Denys Vlasenko <dvlasenk@redhat.com>
> >> Signed-off-by: Liu Shixin <liushixin2@huawei.com>
> >> Acked-by: Pasha Tatashin <pasha.tatashin@soleen.com>
> >> ---
> >> arch/arm64/include/asm/pgtable.h | 4 ++--
> >> 1 file changed, 2 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h
> >> index edf6625ce965..3bc64199aa2e 100644
> >> --- a/arch/arm64/include/asm/pgtable.h
> >> +++ b/arch/arm64/include/asm/pgtable.h
> >> @@ -863,12 +863,12 @@ static inline bool pte_user_accessible_page(pte_t pte)
> >>
> >> static inline bool pmd_user_accessible_page(pmd_t pmd)
> >> {
> >> - return pmd_leaf(pmd) && (pmd_user(pmd) || pmd_user_exec(pmd));
> >> + return pmd_valid(pmd) && pmd_leaf(pmd) && (pmd_user(pmd) || pmd_user_exec(pmd));
> > Hmm, doesn't this have a funny interaction with PROT_NONE where the pmd is
> > invalid but present? If you don't care about PROT_NONE, then you could just
> > do:
> >
> > pmd_valid(pmd) && !pmd_table(pmd) && (pmd_user(pmd) || pmd_user_exec(pmd))
> >
> > but if you do care then you could do:
> >
> > pmd_leaf(pmd) && !pmd_present_invalid(pmd) && (pmd_user(pmd) || pmd_user_exec(pmd))
> I prefer the latter. I will fix and resend later.
> >> static inline bool pud_user_accessible_page(pud_t pud)
> >> {
> >> - return pud_leaf(pud) && pud_user(pud);
> >> + return pud_valid(pud) && pud_leaf(pud) && pud_user(pud);
> > Not caused by this patch, but why don't we have something like a
> > pud_user_exec() check here like we do for the pte and pmd levels?
> As far as I know, there is no user use the user executable pud on arm64, so didn't define pud_user_exec().
I can believe they don't get exposed to userspace at all, but exposing only
as non-executable doesn't sound right. So I would have thought that either
pud_user_accessible_page() would always return false or it would need to
check for the executable case too.
Will
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v2 2/2] arm64/mm: fix incorrect file_map_count for invalid pmd/pud
2022-11-21 18:16 ` Will Deacon
@ 2022-11-22 1:24 ` Liu Shixin
-1 siblings, 0 replies; 26+ messages in thread
From: Liu Shixin @ 2022-11-22 1:24 UTC (permalink / raw)
To: Will Deacon
Cc: Catalin Marinas, Denys Vlasenko, Kefeng Wang, Anshuman Khandual,
David Hildenbrand, Rafael Aquini, Pasha Tatashin,
linux-arm-kernel, linux-kernel
On 2022/11/22 2:16, Will Deacon wrote:
> On Mon, Nov 21, 2022 at 11:15:49AM +0800, Liu Shixin wrote:
>> On 2022/11/18 22:34, Will Deacon wrote:
>>> On Thu, Nov 17, 2022 at 03:56:02PM +0800, Liu Shixin wrote:
>>>> static inline bool pud_user_accessible_page(pud_t pud)
>>>> {
>>>> - return pud_leaf(pud) && pud_user(pud);
>>>> + return pud_valid(pud) && pud_leaf(pud) && pud_user(pud);
>>> Not caused by this patch, but why don't we have something like a
>>> pud_user_exec() check here like we do for the pte and pmd levels?
>> As far as I know, there is no user use the user executable pud on arm64, so didn't define pud_user_exec().
> I can believe they don't get exposed to userspace at all, but exposing only
> as non-executable doesn't sound right. So I would have thought that either
> pud_user_accessible_page() would always return false or it would need to
> check for the executable case too.
Thanks for your advice, I will add the check for the executable case too.
>
> Will
> .
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v2 2/2] arm64/mm: fix incorrect file_map_count for invalid pmd/pud
@ 2022-11-22 1:24 ` Liu Shixin
0 siblings, 0 replies; 26+ messages in thread
From: Liu Shixin @ 2022-11-22 1:24 UTC (permalink / raw)
To: Will Deacon
Cc: Catalin Marinas, Denys Vlasenko, Kefeng Wang, Anshuman Khandual,
David Hildenbrand, Rafael Aquini, Pasha Tatashin,
linux-arm-kernel, linux-kernel
On 2022/11/22 2:16, Will Deacon wrote:
> On Mon, Nov 21, 2022 at 11:15:49AM +0800, Liu Shixin wrote:
>> On 2022/11/18 22:34, Will Deacon wrote:
>>> On Thu, Nov 17, 2022 at 03:56:02PM +0800, Liu Shixin wrote:
>>>> static inline bool pud_user_accessible_page(pud_t pud)
>>>> {
>>>> - return pud_leaf(pud) && pud_user(pud);
>>>> + return pud_valid(pud) && pud_leaf(pud) && pud_user(pud);
>>> Not caused by this patch, but why don't we have something like a
>>> pud_user_exec() check here like we do for the pte and pmd levels?
>> As far as I know, there is no user use the user executable pud on arm64, so didn't define pud_user_exec().
> I can believe they don't get exposed to userspace at all, but exposing only
> as non-executable doesn't sound right. So I would have thought that either
> pud_user_accessible_page() would always return false or it would need to
> check for the executable case too.
Thanks for your advice, I will add the check for the executable case too.
>
> Will
> .
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v2 1/2] arm64/mm: fix incorrect file_map_count for non-leaf pmd/pud
2022-11-17 7:56 ` Liu Shixin
@ 2022-11-22 3:21 ` Anshuman Khandual
-1 siblings, 0 replies; 26+ messages in thread
From: Anshuman Khandual @ 2022-11-22 3:21 UTC (permalink / raw)
To: Liu Shixin, Catalin Marinas, Will Deacon, Denys Vlasenko,
Kefeng Wang, David Hildenbrand, Rafael Aquini, Pasha Tatashin
Cc: linux-arm-kernel, linux-kernel
On 11/17/22 13:26, Liu Shixin wrote:
> The page table check trigger BUG_ON() unexpectedly when collapse hugepage:
>
> ------------[ cut here ]------------
> kernel BUG at mm/page_table_check.c:82!
> Internal error: Oops - BUG: 00000000f2000800 [#1] SMP
> Dumping ftrace buffer:
> (ftrace buffer empty)
> Modules linked in:
> CPU: 6 PID: 68 Comm: khugepaged Not tainted 6.1.0-rc3+ #750
> Hardware name: linux,dummy-virt (DT)
> pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> pc : page_table_check_clear.isra.0+0x258/0x3f0
> lr : page_table_check_clear.isra.0+0x240/0x3f0
> [...]
> Call trace:
> page_table_check_clear.isra.0+0x258/0x3f0
> __page_table_check_pmd_clear+0xbc/0x108
> pmdp_collapse_flush+0xb0/0x160
> collapse_huge_page+0xa08/0x1080
> hpage_collapse_scan_pmd+0xf30/0x1590
> khugepaged_scan_mm_slot.constprop.0+0x52c/0xac8
> khugepaged+0x338/0x518
> kthread+0x278/0x2f8
> ret_from_fork+0x10/0x20
> [...]
>
> Since pmd_user_accessible_page() doesn't check if a pmd is leaf, it
> decrease file_map_count for a non-leaf pmd comes from collapse_huge_page().
> and so trigger BUG_ON() unexpectedly.
>
> Fix this problem by using pmd_leaf() insteal of pmd_present() in
> pmd_user_accessible_page(). Moreover, use pud_leaf() for
> pud_user_accessible_page() too.
>
> Fixes: 42b2547137f5 ("arm64/mm: enable ARCH_SUPPORTS_PAGE_TABLE_CHECK")
> Reported-by: Denys Vlasenko <dvlasenk@redhat.com>
> Signed-off-by: Liu Shixin <liushixin2@huawei.com>
> Reviewed-by: David Hildenbrand <david@redhat.com>
> Acked-by: Pasha Tatashin <pasha.tatashin@soleen.com>
> ---
Reviewed-by: Anshuman Khandual <anshuman.khandual@arm.com>
> arch/arm64/include/asm/pgtable.h | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h
> index 71a1af42f0e8..edf6625ce965 100644
> --- a/arch/arm64/include/asm/pgtable.h
> +++ b/arch/arm64/include/asm/pgtable.h
> @@ -863,12 +863,12 @@ static inline bool pte_user_accessible_page(pte_t pte)
>
> static inline bool pmd_user_accessible_page(pmd_t pmd)
> {
> - return pmd_present(pmd) && (pmd_user(pmd) || pmd_user_exec(pmd));
> + return pmd_leaf(pmd) && (pmd_user(pmd) || pmd_user_exec(pmd));
> }
>
> static inline bool pud_user_accessible_page(pud_t pud)
> {
> - return pud_present(pud) && pud_user(pud);
> + return pud_leaf(pud) && pud_user(pud);
> }
> #endif
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v2 1/2] arm64/mm: fix incorrect file_map_count for non-leaf pmd/pud
@ 2022-11-22 3:21 ` Anshuman Khandual
0 siblings, 0 replies; 26+ messages in thread
From: Anshuman Khandual @ 2022-11-22 3:21 UTC (permalink / raw)
To: Liu Shixin, Catalin Marinas, Will Deacon, Denys Vlasenko,
Kefeng Wang, David Hildenbrand, Rafael Aquini, Pasha Tatashin
Cc: linux-arm-kernel, linux-kernel
On 11/17/22 13:26, Liu Shixin wrote:
> The page table check trigger BUG_ON() unexpectedly when collapse hugepage:
>
> ------------[ cut here ]------------
> kernel BUG at mm/page_table_check.c:82!
> Internal error: Oops - BUG: 00000000f2000800 [#1] SMP
> Dumping ftrace buffer:
> (ftrace buffer empty)
> Modules linked in:
> CPU: 6 PID: 68 Comm: khugepaged Not tainted 6.1.0-rc3+ #750
> Hardware name: linux,dummy-virt (DT)
> pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> pc : page_table_check_clear.isra.0+0x258/0x3f0
> lr : page_table_check_clear.isra.0+0x240/0x3f0
> [...]
> Call trace:
> page_table_check_clear.isra.0+0x258/0x3f0
> __page_table_check_pmd_clear+0xbc/0x108
> pmdp_collapse_flush+0xb0/0x160
> collapse_huge_page+0xa08/0x1080
> hpage_collapse_scan_pmd+0xf30/0x1590
> khugepaged_scan_mm_slot.constprop.0+0x52c/0xac8
> khugepaged+0x338/0x518
> kthread+0x278/0x2f8
> ret_from_fork+0x10/0x20
> [...]
>
> Since pmd_user_accessible_page() doesn't check if a pmd is leaf, it
> decrease file_map_count for a non-leaf pmd comes from collapse_huge_page().
> and so trigger BUG_ON() unexpectedly.
>
> Fix this problem by using pmd_leaf() insteal of pmd_present() in
> pmd_user_accessible_page(). Moreover, use pud_leaf() for
> pud_user_accessible_page() too.
>
> Fixes: 42b2547137f5 ("arm64/mm: enable ARCH_SUPPORTS_PAGE_TABLE_CHECK")
> Reported-by: Denys Vlasenko <dvlasenk@redhat.com>
> Signed-off-by: Liu Shixin <liushixin2@huawei.com>
> Reviewed-by: David Hildenbrand <david@redhat.com>
> Acked-by: Pasha Tatashin <pasha.tatashin@soleen.com>
> ---
Reviewed-by: Anshuman Khandual <anshuman.khandual@arm.com>
> arch/arm64/include/asm/pgtable.h | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h
> index 71a1af42f0e8..edf6625ce965 100644
> --- a/arch/arm64/include/asm/pgtable.h
> +++ b/arch/arm64/include/asm/pgtable.h
> @@ -863,12 +863,12 @@ static inline bool pte_user_accessible_page(pte_t pte)
>
> static inline bool pmd_user_accessible_page(pmd_t pmd)
> {
> - return pmd_present(pmd) && (pmd_user(pmd) || pmd_user_exec(pmd));
> + return pmd_leaf(pmd) && (pmd_user(pmd) || pmd_user_exec(pmd));
> }
>
> static inline bool pud_user_accessible_page(pud_t pud)
> {
> - return pud_present(pud) && pud_user(pud);
> + return pud_leaf(pud) && pud_user(pud);
> }
> #endif
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 26+ messages in thread
end of thread, other threads:[~2022-11-22 3:23 UTC | newest]
Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-17 7:56 [PATCH v2 0/2] arm64: fix two bug about page table check Liu Shixin
2022-11-17 7:56 ` Liu Shixin
2022-11-17 7:56 ` [PATCH v2 1/2] arm64/mm: fix incorrect file_map_count for non-leaf pmd/pud Liu Shixin
2022-11-17 7:56 ` Liu Shixin
2022-11-17 7:56 ` Kefeng Wang
2022-11-17 7:56 ` Kefeng Wang
2022-11-18 14:35 ` Will Deacon
2022-11-18 14:35 ` Will Deacon
2022-11-22 3:21 ` Anshuman Khandual
2022-11-22 3:21 ` Anshuman Khandual
2022-11-17 7:56 ` [PATCH v2 2/2] arm64/mm: fix incorrect file_map_count for invalid pmd/pud Liu Shixin
2022-11-17 7:56 ` Liu Shixin
2022-11-17 7:49 ` David Hildenbrand
2022-11-17 7:49 ` David Hildenbrand
2022-11-17 7:56 ` Kefeng Wang
2022-11-17 7:56 ` Kefeng Wang
2022-11-18 14:34 ` Will Deacon
2022-11-18 14:34 ` Will Deacon
2022-11-21 3:15 ` Liu Shixin
2022-11-21 3:15 ` Liu Shixin
2022-11-21 18:16 ` Will Deacon
2022-11-21 18:16 ` Will Deacon
2022-11-22 1:24 ` Liu Shixin
2022-11-22 1:24 ` Liu Shixin
2022-11-18 19:35 ` (subset) [PATCH v2 0/2] arm64: fix two bug about page table check Catalin Marinas
2022-11-18 19:35 ` Catalin Marinas
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.