This is the first half of v3 of a series to improve the encapsulation of pte
entries by disallowing non-arch code from directly dereferencing pte_t pointers.
Based on earlier feedback, I've split the series in 2; this first part contains
fixes for existing bugs that were discovered during the work. The second part,
which contains the new changes I'm adding, will be posted separately.
These fixes have had a fair amount of review now so I hope they can be
considered for addition to the appropriate mm- branch as is. And I've cc'ed
stable since I believe they should be candidates for backport.
See the v1 cover letter at [1] for rationale and explanation of overall
objective (requires both parts of the split series to achieve).
The series is split up as follows:
patch 1-2: Fix bugs where code was _setting_ ptes directly, rather than using
set_pte_at() and friends. Data corruption was theoretically
possible.
patch 3: Minor refactoring requested in v2 review.
patch 4: Fix highmem unmapping issue I spotted while doing the work.
Patches are based on v6.4-rc4 and a branch is available at [3].
Changes since v2 [2]:
- patch 2: minor commit message rewording to fix review nits
- patch 3: Refactored damon code to use {pte|pmd}p_clear_young_notify()
- patch 1-4: applied Ack/Reviewed-by tags; thanks for those!
Changes since v1 [1]:
- patch 1: Refactored pfn to use local variable
- patch 1-2: Minor rewording of commit message: 'verify' -> 'check'
- patch 1-3: applied Ack/Reviewed-by tags; thanks for those!
[1] https://lore.kernel.org/linux-mm/[email protected]/
[2] https://lore.kernel.org/linux-mm/[email protected]/
[3] https://gitlab.arm.com/linux-arm/linux-rr/-/tree/bugs/pte_encapsulation_bypasses-lkml_v3
Thanks,
Ryan
Ryan Roberts (4):
mm: vmalloc must set pte via arch code
mm/damon/ops-common: atomically test and clear young on ptes and pmds
mm/damon/ops-common: Refactor to use {pte|pmd}p_clear_young_notify()
mm: Fix failure to unmap pte on highmem systems
mm/damon/ops-common.c | 30 ++++--------------------------
mm/damon/ops-common.h | 4 ++--
mm/damon/paddr.c | 4 ++--
mm/damon/vaddr.c | 4 ++--
mm/memory.c | 6 ++----
mm/vmalloc.c | 10 ++++++++--
6 files changed, 20 insertions(+), 38 deletions(-)
--
2.25.1
With the fix in place to atomically test and clear young on ptes and
pmds, simplify the code to handle the clearing for both the primary mmu
and the mmu notifier with a single API call.
Signed-off-by: Ryan Roberts <[email protected]>
---
mm/damon/ops-common.c | 22 ++--------------------
1 file changed, 2 insertions(+), 20 deletions(-)
diff --git a/mm/damon/ops-common.c b/mm/damon/ops-common.c
index acc264b97903..d4ab81229136 100644
--- a/mm/damon/ops-common.c
+++ b/mm/damon/ops-common.c
@@ -39,21 +39,12 @@ struct folio *damon_get_folio(unsigned long pfn)
void damon_ptep_mkold(pte_t *pte, struct vm_area_struct *vma, unsigned long addr)
{
- bool referenced = false;
struct folio *folio = damon_get_folio(pte_pfn(*pte));
if (!folio)
return;
- if (ptep_test_and_clear_young(vma, addr, pte))
- referenced = true;
-
-#ifdef CONFIG_MMU_NOTIFIER
- if (mmu_notifier_clear_young(vma->vm_mm, addr, addr + PAGE_SIZE))
- referenced = true;
-#endif /* CONFIG_MMU_NOTIFIER */
-
- if (referenced)
+ if (ptep_clear_young_notify(vma, addr, pte))
folio_set_young(folio);
folio_set_idle(folio);
@@ -63,21 +54,12 @@ void damon_ptep_mkold(pte_t *pte, struct vm_area_struct *vma, unsigned long addr
void damon_pmdp_mkold(pmd_t *pmd, struct vm_area_struct *vma, unsigned long addr)
{
#ifdef CONFIG_TRANSPARENT_HUGEPAGE
- bool referenced = false;
struct folio *folio = damon_get_folio(pmd_pfn(*pmd));
if (!folio)
return;
- if (pmdp_test_and_clear_young(vma, addr, pmd))
- referenced = true;
-
-#ifdef CONFIG_MMU_NOTIFIER
- if (mmu_notifier_clear_young(vma->vm_mm, addr, addr + HPAGE_PMD_SIZE))
- referenced = true;
-#endif /* CONFIG_MMU_NOTIFIER */
-
- if (referenced)
+ if (pmdp_clear_young_notify(vma, addr, pmd))
folio_set_young(folio);
folio_set_idle(folio);
--
2.25.1
On Fri, Jun 2, 2023 at 3:30 AM Ryan Roberts <[email protected]> wrote:
>
> With the fix in place to atomically test and clear young on ptes and
> pmds, simplify the code to handle the clearing for both the primary mmu
> and the mmu notifier with a single API call.
>
> Signed-off-by: Ryan Roberts <[email protected]>
Acked-by: Yu Zhao <[email protected]>
Hi Ryan,
On Fri, 2 Jun 2023 10:29:48 +0100 Ryan Roberts <[email protected]> wrote:
> With the fix in place to atomically test and clear young on ptes and
> pmds, simplify the code to handle the clearing for both the primary mmu
> and the mmu notifier with a single API call.
>
> Signed-off-by: Ryan Roberts <[email protected]>
Reviewed-by: SeongJae Park <[email protected]>
Thanks,
SJ
> ---
> mm/damon/ops-common.c | 22 ++--------------------
> 1 file changed, 2 insertions(+), 20 deletions(-)
>
> diff --git a/mm/damon/ops-common.c b/mm/damon/ops-common.c
> index acc264b97903..d4ab81229136 100644
> --- a/mm/damon/ops-common.c
> +++ b/mm/damon/ops-common.c
> @@ -39,21 +39,12 @@ struct folio *damon_get_folio(unsigned long pfn)
>
> void damon_ptep_mkold(pte_t *pte, struct vm_area_struct *vma, unsigned long addr)
> {
> - bool referenced = false;
> struct folio *folio = damon_get_folio(pte_pfn(*pte));
>
> if (!folio)
> return;
>
> - if (ptep_test_and_clear_young(vma, addr, pte))
> - referenced = true;
> -
> -#ifdef CONFIG_MMU_NOTIFIER
> - if (mmu_notifier_clear_young(vma->vm_mm, addr, addr + PAGE_SIZE))
> - referenced = true;
> -#endif /* CONFIG_MMU_NOTIFIER */
> -
> - if (referenced)
> + if (ptep_clear_young_notify(vma, addr, pte))
> folio_set_young(folio);
>
> folio_set_idle(folio);
> @@ -63,21 +54,12 @@ void damon_ptep_mkold(pte_t *pte, struct vm_area_struct *vma, unsigned long addr
> void damon_pmdp_mkold(pmd_t *pmd, struct vm_area_struct *vma, unsigned long addr)
> {
> #ifdef CONFIG_TRANSPARENT_HUGEPAGE
> - bool referenced = false;
> struct folio *folio = damon_get_folio(pmd_pfn(*pmd));
>
> if (!folio)
> return;
>
> - if (pmdp_test_and_clear_young(vma, addr, pmd))
> - referenced = true;
> -
> -#ifdef CONFIG_MMU_NOTIFIER
> - if (mmu_notifier_clear_young(vma->vm_mm, addr, addr + HPAGE_PMD_SIZE))
> - referenced = true;
> -#endif /* CONFIG_MMU_NOTIFIER */
> -
> - if (referenced)
> + if (pmdp_clear_young_notify(vma, addr, pmd))
> folio_set_young(folio);
>
> folio_set_idle(folio);
> --
> 2.25.1
>
>