From: sashiko-bot@kernel.org
To: "Yunhui Cui" <cuiyunhui@bytedance.com>
Cc: kvm@vger.kernel.org, linux-perf-users@vger.kernel.org
Subject: Re: [PATCH v2 02/15] riscv: switch arch page-table users to raw PTE helpers
Date: Thu, 16 Jul 2026 13:05:21 +0000 [thread overview]
Message-ID: <20260716130521.D9D071F000E9@smtp.kernel.org> (raw)
In-Reply-To: <304b13bffd2f5299da4ae80c37803e527fb1719d.1784201104.git.cuiyunhui@bytedance.com>
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
New issues:
- [High] Svnapot huge page set and clear paths have mismatched page_table_check PFN tracking and use unaligned addresses, leading to immediate BUG_ON panics.
- [Medium] The conversion to raw PTE helpers incorrectly bypasses the page_table_check framework for non-NAPOT hugetlb pages.
Pre-existing issues:
- [Critical] KVM stage-2 page table pages are freed before the stage-2 TLB is flushed, causing a use-after-free race.
- [High] huge_pte_clear() and huge_ptep_get_and_clear() fail to clear all contiguous PTEs for non-present entries like migration or hwpoison entries.
--
--- Patch [2]: [PATCH v2 02/15] riscv: switch arch page-table users to raw PTE helpers ---
commit 3f0c1de9839b5c9daaa76b96222e6c57e5096eda
Author: Yunhui Cui <cuiyunhui@bytedance.com>
Subject: riscv: switch arch page-table users to raw PTE helpers
This patch converts RISC-V internal page-table walkers and mutators to
use raw PTE helpers. It also adds a raw get-and-clear helper and updates
the hugetlb Svnapot paths to operate on contiguous blocks using the raw
helper model.
> diff --git a/arch/riscv/kvm/gstage.c b/arch/riscv/kvm/gstage.c
> index c4c3b79567f10..67b2c75f5cec1 100644
> --- a/arch/riscv/kvm/gstage.c
> +++ b/arch/riscv/kvm/gstage.c
[ ... ]
> @@ -371,18 +371,18 @@ bool kvm_riscv_gstage_op_pte(struct kvm_gstage *gstage, gpa_t addr,
>
> WARN_ON(addr & (page_size - 1));
>
> - if (!pte_val(ptep_get(ptep)))
> + if (!pte_val(__ptep_get(ptep)))
> return false;
>
> if (ptep_level && !gstage_pte_leaf(ptep)) {
> - next_ptep = (pte_t *)gstage_pte_page_vaddr(ptep_get(ptep));
> + next_ptep = (pte_t *)gstage_pte_page_vaddr(__ptep_get(ptep));
> next_ptep_level = ptep_level - 1;
> ret = gstage_level_to_page_size(gstage, next_ptep_level, &next_page_size);
> if (ret)
> return false;
>
> if (op == GSTAGE_OP_CLEAR)
> - set_pte(ptep, __pte(0));
> + __set_pte(ptep, __pte(0));
> for (i = 0; i < PTRS_PER_PTE; i++)
> flush |= kvm_riscv_gstage_op_pte(gstage, addr + i * next_page_size,
> &next_ptep[i], next_ptep_level, op);
> if (op == GSTAGE_OP_CLEAR)
> put_page(virt_to_page(next_ptep));
[Severity: Critical]
This is a pre-existing issue, but does this unconditionally free the child
page table page before the stage-2 TLB is invalidated?
The page is freed here by calling put_page() before returning the flush
boolean to the caller. This allows a concurrent hardware page table walk
to use the cached TLB directory entry to read from the freed page.
> diff --git a/arch/riscv/mm/hugetlbpage.c b/arch/riscv/mm/hugetlbpage.c
> index a6d217112cf46..5ededac02161d 100644
> --- a/arch/riscv/mm/hugetlbpage.c
> +++ b/arch/riscv/mm/hugetlbpage.c
[ ... ]
> @@ -145,28 +145,52 @@ unsigned long hugetlb_mask_last_page(struct hstate *h)
> return 0UL;
> }
>
> +static unsigned long napot_hugetlb_block_addr(pte_t pte, unsigned long addr)
> +{
> + unsigned long order;
> +
> + if (!pte_napot(pte))
> + return addr;
> +
> + order = napot_cont_order(pte);
> +
> + return addr & napot_cont_mask(order);
> +}
> +
> +static pte_t napot_hugetlb_get_and_clear(struct mm_struct *mm,
> + unsigned long addr, pte_t *ptep)
> +{
> + pte_t pte;
> +
> + pte = __ptep_get_and_clear_noptc(ptep);
> + page_table_check_pte_clear(mm, addr, pte_mknonnapot(pte, addr));
> +
> + return pte;
> +}
[Severity: High]
Does this cause a refcount underflow and trigger a BUG_ON in the
page_table_check logic?
When a SVNAPOT huge page is unmapped, get_clear_contig() iteratively calls
napot_hugetlb_get_and_clear() 16 times for a 64KB block. Each iteration calls
page_table_check_pte_clear() with the exact same encoded PTE (base_PFN + 8).
Since set_huge_napot_ptes() tracks the block consecutively, untracking the
same base PFN + 8 sixteen times will underflow the counter and panic.
[ ... ]
> @@ -207,14 +234,31 @@ static void clear_flush(struct mm_struct *mm,
> unsigned long ncontig)
> {
> struct vm_area_struct vma = TLB_FLUSH_VMA(mm, 0);
> + pte_t pte = __ptep_get(ptep);
> unsigned long i, saddr = addr;
>
> + addr = napot_hugetlb_block_addr(pte, addr);
> + if (pte_napot(pte))
> + ptep = huge_pte_offset(mm, addr,
> + napot_cont_size(napot_cont_order(pte)));
> + saddr = addr;
> +
> for (i = 0; i < ncontig; i++, addr += pgsize, ptep++)
> - ptep_get_and_clear(mm, addr, ptep);
> + napot_hugetlb_get_and_clear(mm, addr, ptep);
>
> flush_tlb_range(&vma, saddr, addr);
> }
>
> +static void set_huge_napot_ptes(struct mm_struct *mm, unsigned long addr,
> + pte_t *ptep, pte_t pte, unsigned long pte_num)
> +{
> + unsigned long i;
> +
> + page_table_check_ptes_set(mm, addr, ptep, pte, pte_num);
> + for (i = 0; i < pte_num; i++)
> + __set_pte_at(mm, ptep + i, pte);
> +}
[Severity: High]
Can this pass an unaligned faulting address to page_table_check_ptes_set()
and track the wrong PFNs?
When called from huge_ptep_set_access_flags(), addr is the unaligned
faulting address, whereas the clear path block-aligns it. This tracks and
untracks different virtual address ranges.
Additionally, passing pte_num to page_table_check_ptes_set() tracks
consecutive PFNs starting from the raw encoded PFN (base_PFN + 8 for order 4)
instead of the true base PFN.
[ ... ]
> @@ -256,19 +300,18 @@ void set_huge_pte_at(struct mm_struct *mm,
>
> if (!pte_present(pte)) {
> for (i = 0; i < pte_num; i++, ptep++, addr += pgsize)
> - set_ptes(mm, addr, ptep, pte, 1);
> + __set_ptes(mm, addr, ptep, pte, 1);
> return;
> }
>
> if (!pte_napot(pte)) {
> - set_ptes(mm, addr, ptep, pte, 1);
> + __set_ptes(mm, addr, ptep, pte, 1);
> return;
> }
[Severity: Medium]
Does this bypass the page_table_check framework for non-NAPOT 2MB huge pages?
The conversion to the raw __set_ptes() helper removes the internal
page_table_check_ptes_set() call that set_ptes() previously provided. Without
a manual page_table_check call on this fallback path, memory corruption
detection is lost for 2MB PMD-mapped huge pages.
[ ... ]
> @@ -306,14 +348,13 @@ pte_t huge_ptep_get_and_clear(struct mm_struct *mm,
> unsigned long addr,
> pte_t *ptep, unsigned long sz)
> {
> - size_t pgsize;
> - pte_t orig_pte = ptep_get(ptep);
> + pte_t orig_pte = __ptep_get(ptep);
> int pte_num;
>
> if (!pte_napot(orig_pte))
> - return ptep_get_and_clear(mm, addr, ptep);
> + return __ptep_get_and_clear(mm, addr, ptep);
[Severity: High]
This is a pre-existing issue, but does this fail to clear all contiguous PTEs
for non-present entries like migration or hwpoison entries?
Since non-present entries do not have the hardware _PAGE_NAPOT bit set,
pte_napot() evaluates to false. This fallback path returns early after
clearing only a single 4KB PTE, leaving the remaining 15 stale entries
in the page table.
[Severity: Medium]
Also related to the conversion to raw PTE helpers, does replacing
ptep_get_and_clear() with __ptep_get_and_clear() bypass the page_table_check
framework for non-NAPOT 2MB huge pages?
[ ... ]
> @@ -363,19 +403,16 @@ void huge_pte_clear(struct mm_struct *mm,
> pte_t *ptep,
> unsigned long sz)
> {
> - size_t pgsize;
> - pte_t pte = ptep_get(ptep);
> - int i, pte_num;
> + pte_t pte = __ptep_get(ptep);
> + int pte_num;
>
> if (!pte_napot(pte)) {
> - pte_clear(mm, addr, ptep);
> + __pte_clear(mm, addr, ptep);
> return;
> }
[Severity: High]
This is a pre-existing issue, but does this also fail to clear all contiguous
PTEs for non-present entries (migration or hwpoison) because pte_napot()
evaluates to false?
[Severity: Medium]
Similar to the issue above, does replacing pte_clear() with __pte_clear()
without a manual page_table_check call disable memory corruption tracking
for non-NAPOT 2MB huge pages?
--
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1784201104.git.cuiyunhui@bytedance.com?part=2
next prev parent reply other threads:[~2026-07-16 13:05 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-07-16 12:41 [PATCH v2 00/15] riscv: add Svnapot PTE folding support Yunhui Cui
2026-07-16 12:41 ` [PATCH v2 01/15] riscv: introduce raw PTE helpers Yunhui Cui
2026-07-16 12:59 ` sashiko-bot
2026-07-16 12:41 ` [PATCH v2 02/15] riscv: switch arch page-table users to " Yunhui Cui
2026-07-16 13:05 ` sashiko-bot [this message]
2026-07-16 12:41 ` [PATCH v2 03/15] riscv/mm: implement Svnapot contpte read-side helpers Yunhui Cui
2026-07-16 12:59 ` sashiko-bot
2026-07-16 12:41 ` [PATCH v2 04/15] riscv/mm: implement Svnapot contpte update helpers Yunhui Cui
2026-07-16 13:03 ` sashiko-bot
2026-07-16 12:41 ` [PATCH v2 05/15] mm: extend pte batch and leaf-size helpers Yunhui Cui
2026-07-16 13:06 ` sashiko-bot
2026-07-16 12:41 ` [PATCH v2 06/15] riscv: make pte_batch_hint() honor folio batch flags Yunhui Cui
2026-07-16 12:41 ` [PATCH v2 07/15] riscv/mm: preserve Svnapot leaf-size semantics for page-table consumers Yunhui Cui
2026-07-16 12:57 ` sashiko-bot
2026-07-16 12:41 ` [PATCH v2 08/15] riscv/mm: avoid redundant Svnapot A/D aggregation Yunhui Cui
2026-07-16 12:41 ` [PATCH v2 09/15] riscv/mm: avoid Svnapot consistency checks in ptep_get() Yunhui Cui
2026-07-16 13:03 ` sashiko-bot
2026-07-16 12:41 ` [PATCH v2 10/15] mm/gup: add fast-GUP specific lockless PTE helpers Yunhui Cui
2026-07-16 12:41 ` [PATCH v2 11/15] riscv: mm: avoid Svnapot A/D aggregation in fast-GUP Yunhui Cui
2026-07-16 12:41 ` [PATCH v2 12/15] arm64: mm: avoid contpte " Yunhui Cui
2026-07-16 12:41 ` [PATCH v2 13/15] riscv/mm: remove redundant TLB flush in napotpte_convert Yunhui Cui
2026-07-16 13:17 ` sashiko-bot
2026-07-16 12:41 ` [PATCH v2 14/15] riscv/mm: optimize mprotect for Svnapot mappings Yunhui Cui
2026-07-16 13:07 ` sashiko-bot
2026-07-16 12:41 ` [PATCH v2 15/15] riscv: mm: Request large exec folios for Svnapot Yunhui Cui
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260716130521.D9D071F000E9@smtp.kernel.org \
--to=sashiko-bot@kernel.org \
--cc=cuiyunhui@bytedance.com \
--cc=kvm@vger.kernel.org \
--cc=linux-perf-users@vger.kernel.org \
--cc=sashiko-reviews@lists.linux.dev \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox