All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH -stable] arm64: mm: don't write garbage into TTBR1_EL1 register
@ 2018-02-23 18:29 Ard Biesheuvel
  2018-02-23 20:33 ` Nicolas Dechesne
                   ` (4 more replies)
  0 siblings, 5 replies; 19+ messages in thread
From: Ard Biesheuvel @ 2018-02-23 18:29 UTC (permalink / raw)
  To: linux-arm-kernel

Stable backport commit 173358a49173 ("arm64: kpti: Add ->enable callback
to remap swapper using nG mappings") of upstream commit f992b4dfd58b did
not survive the backporting process unscathed, and ends up writing garbage
into the TTBR1_EL1 register, rather than pointing it to the zero page to
disable translations. Fix that.

Cc: <stable@vger.kernel.org> #v4.14
Reported-by: Nicolas Dechesne <nicolas.dechesne@linaro.org>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 arch/arm64/mm/proc.S | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/mm/proc.S b/arch/arm64/mm/proc.S
index 08572f95bd8a..2b473ddeb7a3 100644
--- a/arch/arm64/mm/proc.S
+++ b/arch/arm64/mm/proc.S
@@ -155,7 +155,7 @@ ENDPROC(cpu_do_switch_mm)
 
 .macro	__idmap_cpu_set_reserved_ttbr1, tmp1, tmp2
 	adrp	\tmp1, empty_zero_page
-	msr	ttbr1_el1, \tmp2
+	msr	ttbr1_el1, \tmp1
 	isb
 	tlbi	vmalle1
 	dsb	nsh
-- 
2.11.0

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

* [PATCH -stable] arm64: mm: don't write garbage into TTBR1_EL1 register
  2018-02-23 18:29 [PATCH -stable] arm64: mm: don't write garbage into TTBR1_EL1 register Ard Biesheuvel
@ 2018-02-23 20:33 ` Nicolas Dechesne
  2018-02-24  8:34   ` Greg KH
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 19+ messages in thread
From: Nicolas Dechesne @ 2018-02-23 20:33 UTC (permalink / raw)
  To: linux-arm-kernel

hi Ard,

many thanks for your help and the debug session ;-)

On Fri, Feb 23, 2018 at 7:29 PM, Ard Biesheuvel
<ard.biesheuvel@linaro.org> wrote:
> Stable backport commit 173358a49173 ("arm64: kpti: Add ->enable callback
> to remap swapper using nG mappings") of upstream commit f992b4dfd58b did
> not survive the backporting process unscathed, and ends up writing garbage
> into the TTBR1_EL1 register, rather than pointing it to the zero page to
> disable translations. Fix that.
>
> Cc: <stable@vger.kernel.org> #v4.14
> Reported-by: Nicolas Dechesne <nicolas.dechesne@linaro.org>
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

I have tested this patch on Qualcomm Dragonboard 410c where the issue
was found initially.

Tested-by: Nicolas Dechesne <nicolas.dechesne@linaro.org>

This patch is also needed on 4.15-stable.

> ---
>  arch/arm64/mm/proc.S | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/arm64/mm/proc.S b/arch/arm64/mm/proc.S
> index 08572f95bd8a..2b473ddeb7a3 100644
> --- a/arch/arm64/mm/proc.S
> +++ b/arch/arm64/mm/proc.S
> @@ -155,7 +155,7 @@ ENDPROC(cpu_do_switch_mm)
>
>  .macro __idmap_cpu_set_reserved_ttbr1, tmp1, tmp2
>         adrp    \tmp1, empty_zero_page
> -       msr     ttbr1_el1, \tmp2
> +       msr     ttbr1_el1, \tmp1
>         isb
>         tlbi    vmalle1
>         dsb     nsh
> --
> 2.11.0
>

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

* Re: [PATCH -stable] arm64: mm: don't write garbage into TTBR1_EL1 register
  2018-02-23 18:29 [PATCH -stable] arm64: mm: don't write garbage into TTBR1_EL1 register Ard Biesheuvel
@ 2018-02-24  8:34   ` Greg KH
  2018-02-24  8:34   ` Greg KH
                     ` (3 subsequent siblings)
  4 siblings, 0 replies; 19+ messages in thread
From: Greg KH @ 2018-02-24  8:34 UTC (permalink / raw)
  To: Ard Biesheuvel, stable
  Cc: linux-arm-kernel, catalin.marinas, will.deacon, marc.zyngier,
	mark.rutland, nicolas.dechesne

On Fri, Feb 23, 2018 at 06:29:02PM +0000, Ard Biesheuvel wrote:
> Stable backport commit 173358a49173 ("arm64: kpti: Add ->enable callback
> to remap swapper using nG mappings") of upstream commit f992b4dfd58b did
> not survive the backporting process unscathed, and ends up writing garbage
> into the TTBR1_EL1 register, rather than pointing it to the zero page to
> disable translations. Fix that.
> 
> Cc: <stable@vger.kernel.org> #v4.14
> Reported-by: Nicolas Dechesne <nicolas.dechesne@linaro.org>
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  arch/arm64/mm/proc.S | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Any reason why you didn't cc: the stable list, as this is a patch that
is not needed in mainline, right?

> diff --git a/arch/arm64/mm/proc.S b/arch/arm64/mm/proc.S
> index 08572f95bd8a..2b473ddeb7a3 100644
> --- a/arch/arm64/mm/proc.S
> +++ b/arch/arm64/mm/proc.S
> @@ -155,7 +155,7 @@ ENDPROC(cpu_do_switch_mm)
>  
>  .macro	__idmap_cpu_set_reserved_ttbr1, tmp1, tmp2
>  	adrp	\tmp1, empty_zero_page
> -	msr	ttbr1_el1, \tmp2
> +	msr	ttbr1_el1, \tmp1

I don't understand why this isn't also needed in Linus's tree.  What
commit there prevents this from being required?

thanks,

greg k-h

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

* [PATCH -stable] arm64: mm: don't write garbage into TTBR1_EL1 register
@ 2018-02-24  8:34   ` Greg KH
  0 siblings, 0 replies; 19+ messages in thread
From: Greg KH @ 2018-02-24  8:34 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, Feb 23, 2018 at 06:29:02PM +0000, Ard Biesheuvel wrote:
> Stable backport commit 173358a49173 ("arm64: kpti: Add ->enable callback
> to remap swapper using nG mappings") of upstream commit f992b4dfd58b did
> not survive the backporting process unscathed, and ends up writing garbage
> into the TTBR1_EL1 register, rather than pointing it to the zero page to
> disable translations. Fix that.
> 
> Cc: <stable@vger.kernel.org> #v4.14
> Reported-by: Nicolas Dechesne <nicolas.dechesne@linaro.org>
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  arch/arm64/mm/proc.S | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Any reason why you didn't cc: the stable list, as this is a patch that
is not needed in mainline, right?

> diff --git a/arch/arm64/mm/proc.S b/arch/arm64/mm/proc.S
> index 08572f95bd8a..2b473ddeb7a3 100644
> --- a/arch/arm64/mm/proc.S
> +++ b/arch/arm64/mm/proc.S
> @@ -155,7 +155,7 @@ ENDPROC(cpu_do_switch_mm)
>  
>  .macro	__idmap_cpu_set_reserved_ttbr1, tmp1, tmp2
>  	adrp	\tmp1, empty_zero_page
> -	msr	ttbr1_el1, \tmp2
> +	msr	ttbr1_el1, \tmp1

I don't understand why this isn't also needed in Linus's tree.  What
commit there prevents this from being required?

thanks,

greg k-h

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

* Re: [PATCH -stable] arm64: mm: don't write garbage into TTBR1_EL1 register
  2018-02-24  8:34   ` Greg KH
@ 2018-02-24  8:49     ` Nicolas Dechesne
  -1 siblings, 0 replies; 19+ messages in thread
From: Nicolas Dechesne @ 2018-02-24  8:49 UTC (permalink / raw)
  To: Greg KH
  Cc: Ard Biesheuvel, stable, linux-arm-kernel, Catalin Marinas,
	Will Deacon, marc.zyngier, Mark Rutland

On Sat, Feb 24, 2018 at 9:34 AM, Greg KH <gregkh@linuxfoundation.org> wrote:
> On Fri, Feb 23, 2018 at 06:29:02PM +0000, Ard Biesheuvel wrote:
>> Stable backport commit 173358a49173 ("arm64: kpti: Add ->enable callback
>> to remap swapper using nG mappings") of upstream commit f992b4dfd58b did
>> not survive the backporting process unscathed, and ends up writing garbage
>> into the TTBR1_EL1 register, rather than pointing it to the zero page to
>> disable translations. Fix that.
>>
>> Cc: <stable@vger.kernel.org> #v4.14
>> Reported-by: Nicolas Dechesne <nicolas.dechesne@linaro.org>
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> ---
>>  arch/arm64/mm/proc.S | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> Any reason why you didn't cc: the stable list, as this is a patch that
> is not needed in mainline, right?
>
>> diff --git a/arch/arm64/mm/proc.S b/arch/arm64/mm/proc.S
>> index 08572f95bd8a..2b473ddeb7a3 100644
>> --- a/arch/arm64/mm/proc.S
>> +++ b/arch/arm64/mm/proc.S
>> @@ -155,7 +155,7 @@ ENDPROC(cpu_do_switch_mm)
>>
>>  .macro       __idmap_cpu_set_reserved_ttbr1, tmp1, tmp2
>>       adrp    \tmp1, empty_zero_page
>> -     msr     ttbr1_el1, \tmp2
>> +     msr     ttbr1_el1, \tmp1
>
> I don't understand why this isn't also needed in Linus's tree.  What
> commit there prevents this from being required?

in master this code is

.macro  __idmap_cpu_set_reserved_ttbr1, tmp1, tmp2
    adrp    \tmp1, empty_zero_page
    phys_to_ttbr \tmp2, \tmp1
    msr ttbr1_el1, \tmp2
    isb

which can also explain why the (non trivial) cherry-picked commit
ended up wrong.

this change in master came from

529c4b05a3cb arm64: handle 52-bit addresses in TTBR

which afaik, is not needed on stable


>
> thanks,
>
> greg k-h

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

* [PATCH -stable] arm64: mm: don't write garbage into TTBR1_EL1 register
@ 2018-02-24  8:49     ` Nicolas Dechesne
  0 siblings, 0 replies; 19+ messages in thread
From: Nicolas Dechesne @ 2018-02-24  8:49 UTC (permalink / raw)
  To: linux-arm-kernel

On Sat, Feb 24, 2018 at 9:34 AM, Greg KH <gregkh@linuxfoundation.org> wrote:
> On Fri, Feb 23, 2018 at 06:29:02PM +0000, Ard Biesheuvel wrote:
>> Stable backport commit 173358a49173 ("arm64: kpti: Add ->enable callback
>> to remap swapper using nG mappings") of upstream commit f992b4dfd58b did
>> not survive the backporting process unscathed, and ends up writing garbage
>> into the TTBR1_EL1 register, rather than pointing it to the zero page to
>> disable translations. Fix that.
>>
>> Cc: <stable@vger.kernel.org> #v4.14
>> Reported-by: Nicolas Dechesne <nicolas.dechesne@linaro.org>
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> ---
>>  arch/arm64/mm/proc.S | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> Any reason why you didn't cc: the stable list, as this is a patch that
> is not needed in mainline, right?
>
>> diff --git a/arch/arm64/mm/proc.S b/arch/arm64/mm/proc.S
>> index 08572f95bd8a..2b473ddeb7a3 100644
>> --- a/arch/arm64/mm/proc.S
>> +++ b/arch/arm64/mm/proc.S
>> @@ -155,7 +155,7 @@ ENDPROC(cpu_do_switch_mm)
>>
>>  .macro       __idmap_cpu_set_reserved_ttbr1, tmp1, tmp2
>>       adrp    \tmp1, empty_zero_page
>> -     msr     ttbr1_el1, \tmp2
>> +     msr     ttbr1_el1, \tmp1
>
> I don't understand why this isn't also needed in Linus's tree.  What
> commit there prevents this from being required?

in master this code is

.macro  __idmap_cpu_set_reserved_ttbr1, tmp1, tmp2
    adrp    \tmp1, empty_zero_page
    phys_to_ttbr \tmp2, \tmp1
    msr ttbr1_el1, \tmp2
    isb

which can also explain why the (non trivial) cherry-picked commit
ended up wrong.

this change in master came from

529c4b05a3cb arm64: handle 52-bit addresses in TTBR

which afaik, is not needed on stable


>
> thanks,
>
> greg k-h

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

* Re: [PATCH -stable] arm64: mm: don't write garbage into TTBR1_EL1 register
  2018-02-24  8:34   ` Greg KH
@ 2018-02-24  8:50     ` Ard Biesheuvel
  -1 siblings, 0 replies; 19+ messages in thread
From: Ard Biesheuvel @ 2018-02-24  8:50 UTC (permalink / raw)
  To: Greg KH
  Cc: stable, linux-arm-kernel, Catalin Marinas, Will Deacon,
	Marc Zyngier, Mark Rutland, Nicolas Dechesne

On 24 February 2018 at 08:34, Greg KH <gregkh@linuxfoundation.org> wrote:
> On Fri, Feb 23, 2018 at 06:29:02PM +0000, Ard Biesheuvel wrote:
>> Stable backport commit 173358a49173 ("arm64: kpti: Add ->enable callback
>> to remap swapper using nG mappings") of upstream commit f992b4dfd58b did
>> not survive the backporting process unscathed, and ends up writing garbage
>> into the TTBR1_EL1 register, rather than pointing it to the zero page to
>> disable translations. Fix that.
>>
>> Cc: <stable@vger.kernel.org> #v4.14
>> Reported-by: Nicolas Dechesne <nicolas.dechesne@linaro.org>
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> ---
>>  arch/arm64/mm/proc.S | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> Any reason why you didn't cc: the stable list, as this is a patch that
> is not needed in mainline, right?
>

Indeed, apologies. I added the Cc: tag but it appears not to have been
picked up by git send-email.

Also, i suppose it is unclear from the tag that this should be applied
to both v4.15 and v4.14

>> diff --git a/arch/arm64/mm/proc.S b/arch/arm64/mm/proc.S
>> index 08572f95bd8a..2b473ddeb7a3 100644
>> --- a/arch/arm64/mm/proc.S
>> +++ b/arch/arm64/mm/proc.S
>> @@ -155,7 +155,7 @@ ENDPROC(cpu_do_switch_mm)
>>
>>  .macro       __idmap_cpu_set_reserved_ttbr1, tmp1, tmp2
>>       adrp    \tmp1, empty_zero_page
>> -     msr     ttbr1_el1, \tmp2
>> +     msr     ttbr1_el1, \tmp1
>
> I don't understand why this isn't also needed in Linus's tree.  What
> commit there prevents this from being required?
>

Linus's tree has

 +.macro __idmap_cpu_set_reserved_ttbr1, tmp1, tmp2
+     adrp \tmp1, empty_zero_page
+     phys_to_ttbr \tmp1, \tmp2
+     msr ttbr1_el1, \tmp2
+     isb
+     tlbi vmalle1
+     dsb nsh
+     isb
+.endm

but phys_to_ttbr does not exist in the v4.15 and earlier trees (it is
related to 52-bit physical address support which landed in v4.16), so
it was removed for the backport. However, that means tmp2 is never
assigned, and whatever was there is poked into the translation table
base register.

But let's wait for team-ARM to ack this in any case.

Thanks,
Ard.

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

* [PATCH -stable] arm64: mm: don't write garbage into TTBR1_EL1 register
@ 2018-02-24  8:50     ` Ard Biesheuvel
  0 siblings, 0 replies; 19+ messages in thread
From: Ard Biesheuvel @ 2018-02-24  8:50 UTC (permalink / raw)
  To: linux-arm-kernel

On 24 February 2018 at 08:34, Greg KH <gregkh@linuxfoundation.org> wrote:
> On Fri, Feb 23, 2018 at 06:29:02PM +0000, Ard Biesheuvel wrote:
>> Stable backport commit 173358a49173 ("arm64: kpti: Add ->enable callback
>> to remap swapper using nG mappings") of upstream commit f992b4dfd58b did
>> not survive the backporting process unscathed, and ends up writing garbage
>> into the TTBR1_EL1 register, rather than pointing it to the zero page to
>> disable translations. Fix that.
>>
>> Cc: <stable@vger.kernel.org> #v4.14
>> Reported-by: Nicolas Dechesne <nicolas.dechesne@linaro.org>
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> ---
>>  arch/arm64/mm/proc.S | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> Any reason why you didn't cc: the stable list, as this is a patch that
> is not needed in mainline, right?
>

Indeed, apologies. I added the Cc: tag but it appears not to have been
picked up by git send-email.

Also, i suppose it is unclear from the tag that this should be applied
to both v4.15 and v4.14

>> diff --git a/arch/arm64/mm/proc.S b/arch/arm64/mm/proc.S
>> index 08572f95bd8a..2b473ddeb7a3 100644
>> --- a/arch/arm64/mm/proc.S
>> +++ b/arch/arm64/mm/proc.S
>> @@ -155,7 +155,7 @@ ENDPROC(cpu_do_switch_mm)
>>
>>  .macro       __idmap_cpu_set_reserved_ttbr1, tmp1, tmp2
>>       adrp    \tmp1, empty_zero_page
>> -     msr     ttbr1_el1, \tmp2
>> +     msr     ttbr1_el1, \tmp1
>
> I don't understand why this isn't also needed in Linus's tree.  What
> commit there prevents this from being required?
>

Linus's tree has

 +.macro __idmap_cpu_set_reserved_ttbr1, tmp1, tmp2
+     adrp \tmp1, empty_zero_page
+     phys_to_ttbr \tmp1, \tmp2
+     msr ttbr1_el1, \tmp2
+     isb
+     tlbi vmalle1
+     dsb nsh
+     isb
+.endm

but phys_to_ttbr does not exist in the v4.15 and earlier trees (it is
related to 52-bit physical address support which landed in v4.16), so
it was removed for the backport. However, that means tmp2 is never
assigned, and whatever was there is poked into the translation table
base register.

But let's wait for team-ARM to ack this in any case.

Thanks,
Ard.

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

* Re: [PATCH -stable] arm64: mm: don't write garbage into TTBR1_EL1 register
  2018-02-24  8:50     ` Ard Biesheuvel
@ 2018-02-26 11:30       ` Will Deacon
  -1 siblings, 0 replies; 19+ messages in thread
From: Will Deacon @ 2018-02-26 11:30 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: Greg KH, stable, linux-arm-kernel, Catalin Marinas, Marc Zyngier,
	Mark Rutland, Nicolas Dechesne

On Sat, Feb 24, 2018 at 08:50:42AM +0000, Ard Biesheuvel wrote:
> On 24 February 2018 at 08:34, Greg KH <gregkh@linuxfoundation.org> wrote:
> > On Fri, Feb 23, 2018 at 06:29:02PM +0000, Ard Biesheuvel wrote:
> >> diff --git a/arch/arm64/mm/proc.S b/arch/arm64/mm/proc.S
> >> index 08572f95bd8a..2b473ddeb7a3 100644
> >> --- a/arch/arm64/mm/proc.S
> >> +++ b/arch/arm64/mm/proc.S
> >> @@ -155,7 +155,7 @@ ENDPROC(cpu_do_switch_mm)
> >>
> >>  .macro       __idmap_cpu_set_reserved_ttbr1, tmp1, tmp2
> >>       adrp    \tmp1, empty_zero_page
> >> -     msr     ttbr1_el1, \tmp2
> >> +     msr     ttbr1_el1, \tmp1
> >
> > I don't understand why this isn't also needed in Linus's tree.  What
> > commit there prevents this from being required?
> >
> 
> Linus's tree has
> 
>  +.macro __idmap_cpu_set_reserved_ttbr1, tmp1, tmp2
> +     adrp \tmp1, empty_zero_page
> +     phys_to_ttbr \tmp1, \tmp2
> +     msr ttbr1_el1, \tmp2
> +     isb
> +     tlbi vmalle1
> +     dsb nsh
> +     isb
> +.endm
> 
> but phys_to_ttbr does not exist in the v4.15 and earlier trees (it is
> related to 52-bit physical address support which landed in v4.16), so
> it was removed for the backport. However, that means tmp2 is never
> assigned, and whatever was there is poked into the translation table
> base register.

Damnit, sorry again. I changed the argument order of phys_to_ttbr along
the way, so must've confused myself during the backporting exercise. It's
also one of those things that will lead to potential TLB corruption in rare
circumstances where the junk in TTBR1 ends up giving a valid translation,
so it didn't crop up in my testing. How did Nicolas see this? The bug
report I saw didn't look related.

> But let's wait for team-ARM to ack this in any case.

Acked-by: Will Deacon <will.deacon@arm.com>

Greg -- please can you apply this to the 4.14 and 4.15 stable trees?

Will

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

* [PATCH -stable] arm64: mm: don't write garbage into TTBR1_EL1 register
@ 2018-02-26 11:30       ` Will Deacon
  0 siblings, 0 replies; 19+ messages in thread
From: Will Deacon @ 2018-02-26 11:30 UTC (permalink / raw)
  To: linux-arm-kernel

On Sat, Feb 24, 2018 at 08:50:42AM +0000, Ard Biesheuvel wrote:
> On 24 February 2018 at 08:34, Greg KH <gregkh@linuxfoundation.org> wrote:
> > On Fri, Feb 23, 2018 at 06:29:02PM +0000, Ard Biesheuvel wrote:
> >> diff --git a/arch/arm64/mm/proc.S b/arch/arm64/mm/proc.S
> >> index 08572f95bd8a..2b473ddeb7a3 100644
> >> --- a/arch/arm64/mm/proc.S
> >> +++ b/arch/arm64/mm/proc.S
> >> @@ -155,7 +155,7 @@ ENDPROC(cpu_do_switch_mm)
> >>
> >>  .macro       __idmap_cpu_set_reserved_ttbr1, tmp1, tmp2
> >>       adrp    \tmp1, empty_zero_page
> >> -     msr     ttbr1_el1, \tmp2
> >> +     msr     ttbr1_el1, \tmp1
> >
> > I don't understand why this isn't also needed in Linus's tree.  What
> > commit there prevents this from being required?
> >
> 
> Linus's tree has
> 
>  +.macro __idmap_cpu_set_reserved_ttbr1, tmp1, tmp2
> +     adrp \tmp1, empty_zero_page
> +     phys_to_ttbr \tmp1, \tmp2
> +     msr ttbr1_el1, \tmp2
> +     isb
> +     tlbi vmalle1
> +     dsb nsh
> +     isb
> +.endm
> 
> but phys_to_ttbr does not exist in the v4.15 and earlier trees (it is
> related to 52-bit physical address support which landed in v4.16), so
> it was removed for the backport. However, that means tmp2 is never
> assigned, and whatever was there is poked into the translation table
> base register.

Damnit, sorry again. I changed the argument order of phys_to_ttbr along
the way, so must've confused myself during the backporting exercise. It's
also one of those things that will lead to potential TLB corruption in rare
circumstances where the junk in TTBR1 ends up giving a valid translation,
so it didn't crop up in my testing. How did Nicolas see this? The bug
report I saw didn't look related.

> But let's wait for team-ARM to ack this in any case.

Acked-by: Will Deacon <will.deacon@arm.com>

Greg -- please can you apply this to the 4.14 and 4.15 stable trees?

Will

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

* Re: [PATCH -stable] arm64: mm: don't write garbage into TTBR1_EL1 register
  2018-02-26 11:30       ` Will Deacon
@ 2018-02-26 11:37         ` Ard Biesheuvel
  -1 siblings, 0 replies; 19+ messages in thread
From: Ard Biesheuvel @ 2018-02-26 11:37 UTC (permalink / raw)
  To: Will Deacon
  Cc: Greg KH, stable, linux-arm-kernel, Catalin Marinas, Marc Zyngier,
	Mark Rutland, Nicolas Dechesne

On 26 February 2018 at 11:30, Will Deacon <will.deacon@arm.com> wrote:
> On Sat, Feb 24, 2018 at 08:50:42AM +0000, Ard Biesheuvel wrote:
>> On 24 February 2018 at 08:34, Greg KH <gregkh@linuxfoundation.org> wrote:
>> > On Fri, Feb 23, 2018 at 06:29:02PM +0000, Ard Biesheuvel wrote:
>> >> diff --git a/arch/arm64/mm/proc.S b/arch/arm64/mm/proc.S
>> >> index 08572f95bd8a..2b473ddeb7a3 100644
>> >> --- a/arch/arm64/mm/proc.S
>> >> +++ b/arch/arm64/mm/proc.S
>> >> @@ -155,7 +155,7 @@ ENDPROC(cpu_do_switch_mm)
>> >>
>> >>  .macro       __idmap_cpu_set_reserved_ttbr1, tmp1, tmp2
>> >>       adrp    \tmp1, empty_zero_page
>> >> -     msr     ttbr1_el1, \tmp2
>> >> +     msr     ttbr1_el1, \tmp1
>> >
>> > I don't understand why this isn't also needed in Linus's tree.  What
>> > commit there prevents this from being required?
>> >
>>
>> Linus's tree has
>>
>>  +.macro __idmap_cpu_set_reserved_ttbr1, tmp1, tmp2
>> +     adrp \tmp1, empty_zero_page
>> +     phys_to_ttbr \tmp1, \tmp2
>> +     msr ttbr1_el1, \tmp2
>> +     isb
>> +     tlbi vmalle1
>> +     dsb nsh
>> +     isb
>> +.endm
>>
>> but phys_to_ttbr does not exist in the v4.15 and earlier trees (it is
>> related to 52-bit physical address support which landed in v4.16), so
>> it was removed for the backport. However, that means tmp2 is never
>> assigned, and whatever was there is poked into the translation table
>> base register.
>
> Damnit, sorry again. I changed the argument order of phys_to_ttbr along
> the way, so must've confused myself during the backporting exercise. It's
> also one of those things that will lead to potential TLB corruption in rare
> circumstances where the junk in TTBR1 ends up giving a valid translation,
> so it didn't crop up in my testing. How did Nicolas see this? The bug
> report I saw didn't look related.
>

This broke the boot on the Dragonboard 410c, but the bisect identified
another unrelated commit initially.


>> But let's wait for team-ARM to ack this in any case.
>
> Acked-by: Will Deacon <will.deacon@arm.com>
>
> Greg -- please can you apply this to the 4.14 and 4.15 stable trees?
>
> Will

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

* [PATCH -stable] arm64: mm: don't write garbage into TTBR1_EL1 register
@ 2018-02-26 11:37         ` Ard Biesheuvel
  0 siblings, 0 replies; 19+ messages in thread
From: Ard Biesheuvel @ 2018-02-26 11:37 UTC (permalink / raw)
  To: linux-arm-kernel

On 26 February 2018 at 11:30, Will Deacon <will.deacon@arm.com> wrote:
> On Sat, Feb 24, 2018 at 08:50:42AM +0000, Ard Biesheuvel wrote:
>> On 24 February 2018 at 08:34, Greg KH <gregkh@linuxfoundation.org> wrote:
>> > On Fri, Feb 23, 2018 at 06:29:02PM +0000, Ard Biesheuvel wrote:
>> >> diff --git a/arch/arm64/mm/proc.S b/arch/arm64/mm/proc.S
>> >> index 08572f95bd8a..2b473ddeb7a3 100644
>> >> --- a/arch/arm64/mm/proc.S
>> >> +++ b/arch/arm64/mm/proc.S
>> >> @@ -155,7 +155,7 @@ ENDPROC(cpu_do_switch_mm)
>> >>
>> >>  .macro       __idmap_cpu_set_reserved_ttbr1, tmp1, tmp2
>> >>       adrp    \tmp1, empty_zero_page
>> >> -     msr     ttbr1_el1, \tmp2
>> >> +     msr     ttbr1_el1, \tmp1
>> >
>> > I don't understand why this isn't also needed in Linus's tree.  What
>> > commit there prevents this from being required?
>> >
>>
>> Linus's tree has
>>
>>  +.macro __idmap_cpu_set_reserved_ttbr1, tmp1, tmp2
>> +     adrp \tmp1, empty_zero_page
>> +     phys_to_ttbr \tmp1, \tmp2
>> +     msr ttbr1_el1, \tmp2
>> +     isb
>> +     tlbi vmalle1
>> +     dsb nsh
>> +     isb
>> +.endm
>>
>> but phys_to_ttbr does not exist in the v4.15 and earlier trees (it is
>> related to 52-bit physical address support which landed in v4.16), so
>> it was removed for the backport. However, that means tmp2 is never
>> assigned, and whatever was there is poked into the translation table
>> base register.
>
> Damnit, sorry again. I changed the argument order of phys_to_ttbr along
> the way, so must've confused myself during the backporting exercise. It's
> also one of those things that will lead to potential TLB corruption in rare
> circumstances where the junk in TTBR1 ends up giving a valid translation,
> so it didn't crop up in my testing. How did Nicolas see this? The bug
> report I saw didn't look related.
>

This broke the boot on the Dragonboard 410c, but the bisect identified
another unrelated commit initially.


>> But let's wait for team-ARM to ack this in any case.
>
> Acked-by: Will Deacon <will.deacon@arm.com>
>
> Greg -- please can you apply this to the 4.14 and 4.15 stable trees?
>
> Will

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

* Patch "arm64: mm: don't write garbage into TTBR1_EL1 register" has been added to the 4.14-stable tree
  2018-02-23 18:29 [PATCH -stable] arm64: mm: don't write garbage into TTBR1_EL1 register Ard Biesheuvel
  2018-02-23 20:33 ` Nicolas Dechesne
  2018-02-24  8:34   ` Greg KH
@ 2018-02-26 13:02 ` gregkh
  2018-02-26 13:02 ` Patch "arm64: mm: don't write garbage into TTBR1_EL1 register" has been added to the 4.15-stable tree gregkh
  2018-02-26 13:02 ` Patch "arm64: mm: don't write garbage into TTBR1_EL1 register" has been added to the 4.4-stable tree gregkh
  4 siblings, 0 replies; 19+ messages in thread
From: gregkh @ 2018-02-26 13:02 UTC (permalink / raw)
  To: ard.biesheuvel, gregkh, nicolas.dechesne, stable, will.deacon
  Cc: stable, stable-commits


This is a note to let you know that I've just added the patch titled

    arm64: mm: don't write garbage into TTBR1_EL1 register

to the 4.14-stable tree which can be found at:
    http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary

The filename of the patch is:
     arm64-mm-don-t-write-garbage-into-ttbr1_el1-register.patch
and it can be found in the queue-4.14 subdirectory.

If you, or anyone else, feels it should not be added to the stable tree,
please let <stable@vger.kernel.org> know about it.


>From ard.biesheuvel@linaro.org  Mon Feb 26 13:53:22 2018
From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Date: Fri, 23 Feb 2018 18:29:02 +0000
Subject: arm64: mm: don't write garbage into TTBR1_EL1 register
To: linux-arm-kernel@lists.infradead.org
Cc: catalin.marinas@arm.com, will.deacon@arm.com, marc.zyngier@arm.com, mark.rutland@arm.com, nicolas.dechesne@linaro.org, gregkh@linuxfoundation.org, Ard Biesheuvel <ard.biesheuvel@linaro.org>
Message-ID: <20180223182902.24873-1-ard.biesheuvel@linaro.org>

From: Ard Biesheuvel <ard.biesheuvel@linaro.org>

Stable backport commit 173358a49173 ("arm64: kpti: Add ->enable callback
to remap swapper using nG mappings") of upstream commit f992b4dfd58b did
not survive the backporting process unscathed, and ends up writing garbage
into the TTBR1_EL1 register, rather than pointing it to the zero page to
disable translations. Fix that.

Cc: <stable@vger.kernel.org> #v4.14
Reported-by: Nicolas Dechesne <nicolas.dechesne@linaro.org>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Acked-by: Will Deacon <will.deacon@arm.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 arch/arm64/mm/proc.S |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- a/arch/arm64/mm/proc.S
+++ b/arch/arm64/mm/proc.S
@@ -155,7 +155,7 @@ ENDPROC(cpu_do_switch_mm)
 
 .macro	__idmap_cpu_set_reserved_ttbr1, tmp1, tmp2
 	adrp	\tmp1, empty_zero_page
-	msr	ttbr1_el1, \tmp2
+	msr	ttbr1_el1, \tmp1
 	isb
 	tlbi	vmalle1
 	dsb	nsh


Patches currently in stable-queue which might be from ard.biesheuvel@linaro.org are

queue-4.14/arm64-mm-don-t-write-garbage-into-ttbr1_el1-register.patch

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

* Patch "arm64: mm: don't write garbage into TTBR1_EL1 register" has been added to the 4.15-stable tree
  2018-02-23 18:29 [PATCH -stable] arm64: mm: don't write garbage into TTBR1_EL1 register Ard Biesheuvel
                   ` (2 preceding siblings ...)
  2018-02-26 13:02 ` Patch "arm64: mm: don't write garbage into TTBR1_EL1 register" has been added to the 4.14-stable tree gregkh
@ 2018-02-26 13:02 ` gregkh
  2018-02-26 13:02 ` Patch "arm64: mm: don't write garbage into TTBR1_EL1 register" has been added to the 4.4-stable tree gregkh
  4 siblings, 0 replies; 19+ messages in thread
From: gregkh @ 2018-02-26 13:02 UTC (permalink / raw)
  To: ard.biesheuvel, gregkh, nicolas.dechesne, stable, will.deacon
  Cc: stable, stable-commits


This is a note to let you know that I've just added the patch titled

    arm64: mm: don't write garbage into TTBR1_EL1 register

to the 4.15-stable tree which can be found at:
    http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary

The filename of the patch is:
     arm64-mm-don-t-write-garbage-into-ttbr1_el1-register.patch
and it can be found in the queue-4.15 subdirectory.

If you, or anyone else, feels it should not be added to the stable tree,
please let <stable@vger.kernel.org> know about it.


>From ard.biesheuvel@linaro.org  Mon Feb 26 13:53:22 2018
From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Date: Fri, 23 Feb 2018 18:29:02 +0000
Subject: arm64: mm: don't write garbage into TTBR1_EL1 register
To: linux-arm-kernel@lists.infradead.org
Cc: catalin.marinas@arm.com, will.deacon@arm.com, marc.zyngier@arm.com, mark.rutland@arm.com, nicolas.dechesne@linaro.org, gregkh@linuxfoundation.org, Ard Biesheuvel <ard.biesheuvel@linaro.org>
Message-ID: <20180223182902.24873-1-ard.biesheuvel@linaro.org>

From: Ard Biesheuvel <ard.biesheuvel@linaro.org>

Stable backport commit 173358a49173 ("arm64: kpti: Add ->enable callback
to remap swapper using nG mappings") of upstream commit f992b4dfd58b did
not survive the backporting process unscathed, and ends up writing garbage
into the TTBR1_EL1 register, rather than pointing it to the zero page to
disable translations. Fix that.

Cc: <stable@vger.kernel.org> #v4.14
Reported-by: Nicolas Dechesne <nicolas.dechesne@linaro.org>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Acked-by: Will Deacon <will.deacon@arm.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 arch/arm64/mm/proc.S |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- a/arch/arm64/mm/proc.S
+++ b/arch/arm64/mm/proc.S
@@ -155,7 +155,7 @@ ENDPROC(cpu_do_switch_mm)
 
 .macro	__idmap_cpu_set_reserved_ttbr1, tmp1, tmp2
 	adrp	\tmp1, empty_zero_page
-	msr	ttbr1_el1, \tmp2
+	msr	ttbr1_el1, \tmp1
 	isb
 	tlbi	vmalle1
 	dsb	nsh


Patches currently in stable-queue which might be from ard.biesheuvel@linaro.org are

queue-4.15/arm64-mm-don-t-write-garbage-into-ttbr1_el1-register.patch

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

* Patch "arm64: mm: don't write garbage into TTBR1_EL1 register" has been added to the 4.4-stable tree
  2018-02-23 18:29 [PATCH -stable] arm64: mm: don't write garbage into TTBR1_EL1 register Ard Biesheuvel
                   ` (3 preceding siblings ...)
  2018-02-26 13:02 ` Patch "arm64: mm: don't write garbage into TTBR1_EL1 register" has been added to the 4.15-stable tree gregkh
@ 2018-02-26 13:02 ` gregkh
  2018-02-26 13:09   ` Ard Biesheuvel
  4 siblings, 1 reply; 19+ messages in thread
From: gregkh @ 2018-02-26 13:02 UTC (permalink / raw)
  To: ard.biesheuvel, gregkh, nicolas.dechesne, stable, will.deacon
  Cc: stable, stable-commits


This is a note to let you know that I've just added the patch titled

    arm64: mm: don't write garbage into TTBR1_EL1 register

to the 4.4-stable tree which can be found at:
    http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary

The filename of the patch is:
     arm64-mm-don-t-write-garbage-into-ttbr1_el1-register.patch
and it can be found in the queue-4.4 subdirectory.

If you, or anyone else, feels it should not be added to the stable tree,
please let <stable@vger.kernel.org> know about it.


>From ard.biesheuvel@linaro.org  Mon Feb 26 13:53:22 2018
From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Date: Fri, 23 Feb 2018 18:29:02 +0000
Subject: arm64: mm: don't write garbage into TTBR1_EL1 register
To: linux-arm-kernel@lists.infradead.org
Cc: catalin.marinas@arm.com, will.deacon@arm.com, marc.zyngier@arm.com, mark.rutland@arm.com, nicolas.dechesne@linaro.org, gregkh@linuxfoundation.org, Ard Biesheuvel <ard.biesheuvel@linaro.org>
Message-ID: <20180223182902.24873-1-ard.biesheuvel@linaro.org>

From: Ard Biesheuvel <ard.biesheuvel@linaro.org>

Stable backport commit 173358a49173 ("arm64: kpti: Add ->enable callback
to remap swapper using nG mappings") of upstream commit f992b4dfd58b did
not survive the backporting process unscathed, and ends up writing garbage
into the TTBR1_EL1 register, rather than pointing it to the zero page to
disable translations. Fix that.

Cc: <stable@vger.kernel.org> #v4.14
Reported-by: Nicolas Dechesne <nicolas.dechesne@linaro.org>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Acked-by: Will Deacon <will.deacon@arm.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 arch/arm64/mm/proc.S | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/mm/proc.S b/arch/arm64/mm/proc.S
index 08572f95bd8a..2b473ddeb7a3 100644
--- a/arch/arm64/mm/proc.S
+++ b/arch/arm64/mm/proc.S
@@ -155,7 +155,7 @@ ENDPROC(cpu_do_switch_mm)
 
 .macro	__idmap_cpu_set_reserved_ttbr1, tmp1, tmp2
 	adrp	\tmp1, empty_zero_page
-	msr	ttbr1_el1, \tmp2
+	msr	ttbr1_el1, \tmp1
 	isb
 	tlbi	vmalle1
 	dsb	nsh
-- 
2.11.0



Patches currently in stable-queue which might be from ard.biesheuvel@linaro.org are

queue-4.4/arm64-mm-don-t-write-garbage-into-ttbr1_el1-register.patch

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

* Re: Patch "arm64: mm: don't write garbage into TTBR1_EL1 register" has been added to the 4.4-stable tree
  2018-02-26 13:02 ` Patch "arm64: mm: don't write garbage into TTBR1_EL1 register" has been added to the 4.4-stable tree gregkh
@ 2018-02-26 13:09   ` Ard Biesheuvel
  2018-02-26 14:26     ` Greg Kroah-Hartman
  0 siblings, 1 reply; 19+ messages in thread
From: Ard Biesheuvel @ 2018-02-26 13:09 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: Nicolas Dechesne, stable, Will Deacon, stable-commits

On 26 February 2018 at 13:02,  <gregkh@linuxfoundation.org> wrote:
>
> This is a note to let you know that I've just added the patch titled
>
>     arm64: mm: don't write garbage into TTBR1_EL1 register
>
> to the 4.4-stable tree which can be found at:
>     http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
>

Please drop this. It only applies to v4.14 and v4.15

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

* Re: Patch "arm64: mm: don't write garbage into TTBR1_EL1 register" has been added to the 4.4-stable tree
  2018-02-26 13:09   ` Ard Biesheuvel
@ 2018-02-26 14:26     ` Greg Kroah-Hartman
  0 siblings, 0 replies; 19+ messages in thread
From: Greg Kroah-Hartman @ 2018-02-26 14:26 UTC (permalink / raw)
  To: Ard Biesheuvel; +Cc: Nicolas Dechesne, stable, Will Deacon, stable-commits

On Mon, Feb 26, 2018 at 01:09:36PM +0000, Ard Biesheuvel wrote:
> On 26 February 2018 at 13:02,  <gregkh@linuxfoundation.org> wrote:
> >
> > This is a note to let you know that I've just added the patch titled
> >
> >     arm64: mm: don't write garbage into TTBR1_EL1 register
> >
> > to the 4.4-stable tree which can be found at:
> >     http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
> >
> 
> Please drop this. It only applies to v4.14 and v4.15

Ugh, my fault, it didn't even apply, but ended up in the wrong
directory.  Thanks for pointing it out, now dropped.

greg k-h

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

* Re: [PATCH -stable] arm64: mm: don't write garbage into TTBR1_EL1 register
  2018-02-26 11:30       ` Will Deacon
@ 2018-02-28 10:23         ` Jan Glauber
  -1 siblings, 0 replies; 19+ messages in thread
From: Jan Glauber @ 2018-02-28 10:23 UTC (permalink / raw)
  To: Will Deacon
  Cc: Ard Biesheuvel, Mark Rutland, Marc Zyngier, Greg KH, stable,
	Catalin Marinas, Nicolas Dechesne, linux-arm-kernel

On Mon, Feb 26, 2018 at 11:30:50AM +0000, Will Deacon wrote:
> Damnit, sorry again. I changed the argument order of phys_to_ttbr along
> the way, so must've confused myself during the backporting exercise. It's
> also one of those things that will lead to potential TLB corruption in rare
> circumstances where the junk in TTBR1 ends up giving a valid translation,
> so it didn't crop up in my testing. How did Nicolas see this? The bug
> report I saw didn't look related.

FWIW, we've been hitting this bug with a distribution backport on
ThunderX2 on every boot. Due to bad luck there was a non-zero value
in TTBR1 that crashed the kernel immediately and dropped us to firmware.

--Jan

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

* [PATCH -stable] arm64: mm: don't write garbage into TTBR1_EL1 register
@ 2018-02-28 10:23         ` Jan Glauber
  0 siblings, 0 replies; 19+ messages in thread
From: Jan Glauber @ 2018-02-28 10:23 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Feb 26, 2018 at 11:30:50AM +0000, Will Deacon wrote:
> Damnit, sorry again. I changed the argument order of phys_to_ttbr along
> the way, so must've confused myself during the backporting exercise. It's
> also one of those things that will lead to potential TLB corruption in rare
> circumstances where the junk in TTBR1 ends up giving a valid translation,
> so it didn't crop up in my testing. How did Nicolas see this? The bug
> report I saw didn't look related.

FWIW, we've been hitting this bug with a distribution backport on
ThunderX2 on every boot. Due to bad luck there was a non-zero value
in TTBR1 that crashed the kernel immediately and dropped us to firmware.

--Jan

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

end of thread, other threads:[~2018-02-28 10:23 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-02-23 18:29 [PATCH -stable] arm64: mm: don't write garbage into TTBR1_EL1 register Ard Biesheuvel
2018-02-23 20:33 ` Nicolas Dechesne
2018-02-24  8:34 ` Greg KH
2018-02-24  8:34   ` Greg KH
2018-02-24  8:49   ` Nicolas Dechesne
2018-02-24  8:49     ` Nicolas Dechesne
2018-02-24  8:50   ` Ard Biesheuvel
2018-02-24  8:50     ` Ard Biesheuvel
2018-02-26 11:30     ` Will Deacon
2018-02-26 11:30       ` Will Deacon
2018-02-26 11:37       ` Ard Biesheuvel
2018-02-26 11:37         ` Ard Biesheuvel
2018-02-28 10:23       ` Jan Glauber
2018-02-28 10:23         ` Jan Glauber
2018-02-26 13:02 ` Patch "arm64: mm: don't write garbage into TTBR1_EL1 register" has been added to the 4.14-stable tree gregkh
2018-02-26 13:02 ` Patch "arm64: mm: don't write garbage into TTBR1_EL1 register" has been added to the 4.15-stable tree gregkh
2018-02-26 13:02 ` Patch "arm64: mm: don't write garbage into TTBR1_EL1 register" has been added to the 4.4-stable tree gregkh
2018-02-26 13:09   ` Ard Biesheuvel
2018-02-26 14:26     ` Greg Kroah-Hartman

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.