* [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.