linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v1] arm64: tlb: fix the TTL value of tlb_get_level
@ 2021-06-23  7:05 Zhenyu Ye
  2021-06-23 11:04 ` Will Deacon
  2021-06-23 14:00 ` Will Deacon
  0 siblings, 2 replies; 6+ messages in thread
From: Zhenyu Ye @ 2021-06-23  7:05 UTC (permalink / raw)
  To: Catalin Marinas, Will Deacon, aneesh.kumar, Marc Zyngier,
	steven.price, Peter Zijlstra
  Cc: linux-arm-kernel, linux-kernel, linux-arch, linux-mm,
	Xiexiangyou, liushixin2, huyaqin, zhurui3, yezhenyu2

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))
-- 
2.26.2

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v1] arm64: tlb: fix the TTL value of tlb_get_level
  2021-06-23  7:05 [PATCH v1] arm64: tlb: fix the TTL value of tlb_get_level Zhenyu Ye
@ 2021-06-23 11:04 ` Will Deacon
  2021-06-23 11:28   ` Catalin Marinas
  2021-06-24  1:55   ` Zhenyu Ye
  2021-06-23 14:00 ` Will Deacon
  1 sibling, 2 replies; 6+ messages in thread
From: Will Deacon @ 2021-06-23 11:04 UTC (permalink / raw)
  To: Zhenyu Ye
  Cc: Catalin Marinas, aneesh.kumar, Marc Zyngier, steven.price,
	Peter Zijlstra, linux-arm-kernel, linux-kernel, linux-arch,
	linux-mm, Xiexiangyou, liushixin2, huyaqin, zhurui3

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v1] arm64: tlb: fix the TTL value of tlb_get_level
  2021-06-23 11:04 ` Will Deacon
@ 2021-06-23 11:28   ` Catalin Marinas
  2021-06-24  1:55   ` Zhenyu Ye
  1 sibling, 0 replies; 6+ messages in thread
From: Catalin Marinas @ 2021-06-23 11:28 UTC (permalink / raw)
  To: Will Deacon
  Cc: Zhenyu Ye, aneesh.kumar, Marc Zyngier, steven.price,
	Peter Zijlstra, linux-arm-kernel, linux-kernel, linux-arch,
	linux-mm, Xiexiangyou, liushixin2, huyaqin, zhurui3

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

-- 
Catalin

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v1] arm64: tlb: fix the TTL value of tlb_get_level
  2021-06-23  7:05 [PATCH v1] arm64: tlb: fix the TTL value of tlb_get_level Zhenyu Ye
  2021-06-23 11:04 ` Will Deacon
@ 2021-06-23 14:00 ` Will Deacon
  1 sibling, 0 replies; 6+ messages in thread
From: Will Deacon @ 2021-06-23 14:00 UTC (permalink / raw)
  To: Peter Zijlstra, aneesh.kumar, Catalin Marinas, Marc Zyngier,
	steven.price, Zhenyu Ye
  Cc: kernel-team, Will Deacon, liushixin2, Xiexiangyou,
	linux-arm-kernel, huyaqin, linux-mm, linux-arch, zhurui3,
	linux-kernel

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,
-- 
Will

https://fixes.arm64.dev
https://next.arm64.dev
https://will.arm64.dev

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v1] arm64: tlb: fix the TTL value of tlb_get_level
  2021-06-23 11:04 ` Will Deacon
  2021-06-23 11:28   ` Catalin Marinas
@ 2021-06-24  1:55   ` Zhenyu Ye
  2021-06-24  8:29     ` Will Deacon
  1 sibling, 1 reply; 6+ messages in thread
From: Zhenyu Ye @ 2021-06-24  1:55 UTC (permalink / raw)
  To: Will Deacon
  Cc: Catalin Marinas, aneesh.kumar, Marc Zyngier, steven.price,
	Peter Zijlstra, linux-arm-kernel, linux-kernel, linux-arch,
	linux-mm, Xiexiangyou, liushixin2, huyaqin, zhurui3

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v1] arm64: tlb: fix the TTL value of tlb_get_level
  2021-06-24  1:55   ` Zhenyu Ye
@ 2021-06-24  8:29     ` Will Deacon
  0 siblings, 0 replies; 6+ messages in thread
From: Will Deacon @ 2021-06-24  8:29 UTC (permalink / raw)
  To: Zhenyu Ye
  Cc: Catalin Marinas, aneesh.kumar, Marc Zyngier, steven.price,
	Peter Zijlstra, linux-arm-kernel, linux-kernel, linux-arch,
	linux-mm, Xiexiangyou, liushixin2, huyaqin, zhurui3

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2021-06-24  8:29 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-23  7:05 [PATCH v1] arm64: tlb: fix the TTL value of tlb_get_level Zhenyu Ye
2021-06-23 11:04 ` Will Deacon
2021-06-23 11:28   ` Catalin Marinas
2021-06-24  1:55   ` Zhenyu Ye
2021-06-24  8:29     ` Will Deacon
2021-06-23 14:00 ` Will Deacon

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).