| Message ID | b80ead47-1f88-3a00-18e1-cacc22f54cc4@huawei.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v1] arm64: tlb: fix the TTL value of tlb_get_level | expand |
On Wed, Jun 23, 2021 at 03:05:22PM +0800, Zhenyu Ye wrote: > The TTL field indicates the level of page table walk holding the *leaf* > entry for the address being invalidated. But currently, the TTL field > may be set to an incorrent value in the following stack: > > pte_free_tlb > __pte_free_tlb > tlb_remove_table > tlb_table_invalidate > tlb_flush_mmu_tlbonly > tlb_flush > > In this case, we just want to flush a PTE page, but the tlb->cleared_pmds > is set and we get tlb_level = 2 in the tlb_get_level() function. This may > cause some unexpected problems. > > This patch set the TTL field to 0 if tlb->freed_tables is set. The > tlb->freed_tables indicates page table pages are freed, not the leaf > entry. > > Fixes: c4ab2cbc1d87 ("arm64: tlb: Set the TTL field in flush_tlb_range") > Reported-by: ZhuRui <zhurui3@huawei.com> > Signed-off-by: Zhenyu Ye <yezhenyu2@huawei.com> > --- > arch/arm64/include/asm/tlb.h | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/arch/arm64/include/asm/tlb.h b/arch/arm64/include/asm/tlb.h > index 61c97d3b58c7..c995d1f4594f 100644 > --- a/arch/arm64/include/asm/tlb.h > +++ b/arch/arm64/include/asm/tlb.h > @@ -28,6 +28,10 @@ static void tlb_flush(struct mmu_gather *tlb); > */ > static inline int tlb_get_level(struct mmu_gather *tlb) > { > + /* The TTL field is only valid for the leaf entry. */ > + if (tlb->freed_tables) > + return 0; > + > if (tlb->cleared_ptes && !(tlb->cleared_pmds || > tlb->cleared_puds || > tlb->cleared_p4ds)) Thanks. I can't see a better way around this, so I'll queue the patch. The stage-2 page-table code looks ok afaict, but please can you check it too? Cheers, Will
On Wed, Jun 23, 2021 at 12:04:12PM +0100, Will Deacon wrote: > On Wed, Jun 23, 2021 at 03:05:22PM +0800, Zhenyu Ye wrote: > > The TTL field indicates the level of page table walk holding the *leaf* > > entry for the address being invalidated. But currently, the TTL field > > may be set to an incorrent value in the following stack: > > > > pte_free_tlb > > __pte_free_tlb > > tlb_remove_table > > tlb_table_invalidate > > tlb_flush_mmu_tlbonly > > tlb_flush > > > > In this case, we just want to flush a PTE page, but the tlb->cleared_pmds > > is set and we get tlb_level = 2 in the tlb_get_level() function. This may > > cause some unexpected problems. > > > > This patch set the TTL field to 0 if tlb->freed_tables is set. The > > tlb->freed_tables indicates page table pages are freed, not the leaf > > entry. > > > > Fixes: c4ab2cbc1d87 ("arm64: tlb: Set the TTL field in flush_tlb_range") > > Reported-by: ZhuRui <zhurui3@huawei.com> > > Signed-off-by: Zhenyu Ye <yezhenyu2@huawei.com> > > --- > > arch/arm64/include/asm/tlb.h | 4 ++++ > > 1 file changed, 4 insertions(+) > > > > diff --git a/arch/arm64/include/asm/tlb.h b/arch/arm64/include/asm/tlb.h > > index 61c97d3b58c7..c995d1f4594f 100644 > > --- a/arch/arm64/include/asm/tlb.h > > +++ b/arch/arm64/include/asm/tlb.h > > @@ -28,6 +28,10 @@ static void tlb_flush(struct mmu_gather *tlb); > > */ > > static inline int tlb_get_level(struct mmu_gather *tlb) > > { > > + /* The TTL field is only valid for the leaf entry. */ > > + if (tlb->freed_tables) > > + return 0; > > + > > if (tlb->cleared_ptes && !(tlb->cleared_pmds || > > tlb->cleared_puds || > > tlb->cleared_p4ds)) > > Thanks. I can't see a better way around this, so I'll queue the patch. If you haven't queued it already, feel free to add: Acked-by: Catalin Marinas <catalin.marinas@arm.com> I'd also add: Cc: <stable@vger.kernel.org> # 5.9.x
On Wed, 23 Jun 2021 15:05:22 +0800, Zhenyu Ye wrote: > The TTL field indicates the level of page table walk holding the *leaf* > entry for the address being invalidated. But currently, the TTL field > may be set to an incorrent value in the following stack: > > pte_free_tlb > __pte_free_tlb > tlb_remove_table > tlb_table_invalidate > tlb_flush_mmu_tlbonly > tlb_flush > > [...] Applied to arm64 (for-next/mm), thanks! [1/1] arm64: tlb: fix the TTL value of tlb_get_level https://git.kernel.org/arm64/c/52218fcd61cb Cheers,
On 2021/6/23 19:04, Will Deacon wrote: > On Wed, Jun 23, 2021 at 03:05:22PM +0800, Zhenyu Ye wrote: >> diff --git a/arch/arm64/include/asm/tlb.h b/arch/arm64/include/asm/tlb.h >> index 61c97d3b58c7..c995d1f4594f 100644 >> --- a/arch/arm64/include/asm/tlb.h >> +++ b/arch/arm64/include/asm/tlb.h >> @@ -28,6 +28,10 @@ static void tlb_flush(struct mmu_gather *tlb); >> */ >> static inline int tlb_get_level(struct mmu_gather *tlb) >> { >> + /* The TTL field is only valid for the leaf entry. */ >> + if (tlb->freed_tables) >> + return 0; >> + >> if (tlb->cleared_ptes && !(tlb->cleared_pmds || >> tlb->cleared_puds || >> tlb->cleared_p4ds)) > > Thanks. I can't see a better way around this, so I'll queue the patch. > The stage-2 page-table code looks ok afaict, but please can you check it > too? The stage-2 page-table codes seem to be correct to me. Thanks, Zhenyu
On Thu, Jun 24, 2021 at 09:55:53AM +0800, Zhenyu Ye wrote: > On 2021/6/23 19:04, Will Deacon wrote: > > On Wed, Jun 23, 2021 at 03:05:22PM +0800, Zhenyu Ye wrote: > >> diff --git a/arch/arm64/include/asm/tlb.h b/arch/arm64/include/asm/tlb.h > >> index 61c97d3b58c7..c995d1f4594f 100644 > >> --- a/arch/arm64/include/asm/tlb.h > >> +++ b/arch/arm64/include/asm/tlb.h > >> @@ -28,6 +28,10 @@ static void tlb_flush(struct mmu_gather *tlb); > >> */ > >> static inline int tlb_get_level(struct mmu_gather *tlb) > >> { > >> + /* The TTL field is only valid for the leaf entry. */ > >> + if (tlb->freed_tables) > >> + return 0; > >> + > >> if (tlb->cleared_ptes && !(tlb->cleared_pmds || > >> tlb->cleared_puds || > >> tlb->cleared_p4ds)) > > > > Thanks. I can't see a better way around this, so I'll queue the patch. > > The stage-2 page-table code looks ok afaict, but please can you check it > > too? > > The stage-2 page-table codes seem to be correct to me. Thanks for having a look. Will
diff --git a/arch/arm64/include/asm/tlb.h b/arch/arm64/include/asm/tlb.h index 61c97d3b58c7..c995d1f4594f 100644 --- a/arch/arm64/include/asm/tlb.h +++ b/arch/arm64/include/asm/tlb.h @@ -28,6 +28,10 @@ static void tlb_flush(struct mmu_gather *tlb); */ static inline int tlb_get_level(struct mmu_gather *tlb) { + /* The TTL field is only valid for the leaf entry. */ + if (tlb->freed_tables) + return 0; + if (tlb->cleared_ptes && !(tlb->cleared_pmds || tlb->cleared_puds || tlb->cleared_p4ds))
The TTL field indicates the level of page table walk holding the *leaf* entry for the address being invalidated. But currently, the TTL field may be set to an incorrent value in the following stack: pte_free_tlb __pte_free_tlb tlb_remove_table tlb_table_invalidate tlb_flush_mmu_tlbonly tlb_flush In this case, we just want to flush a PTE page, but the tlb->cleared_pmds is set and we get tlb_level = 2 in the tlb_get_level() function. This may cause some unexpected problems. This patch set the TTL field to 0 if tlb->freed_tables is set. The tlb->freed_tables indicates page table pages are freed, not the leaf entry. Fixes: c4ab2cbc1d87 ("arm64: tlb: Set the TTL field in flush_tlb_range") Reported-by: ZhuRui <zhurui3@huawei.com> Signed-off-by: Zhenyu Ye <yezhenyu2@huawei.com> --- arch/arm64/include/asm/tlb.h | 4 ++++ 1 file changed, 4 insertions(+)