Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] arm64: mm: Defer read-only remap of data/bss linear alias
@ 2026-06-19 16:39 Ard Biesheuvel
  2026-06-22 15:23 ` Kevin Brodsky
  2026-06-22 18:13 ` Fuad Tabba
  0 siblings, 2 replies; 5+ messages in thread
From: Ard Biesheuvel @ 2026-06-19 16:39 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: linux-kernel, catalin.marinas, will, Ard Biesheuvel, Fuad Tabba

From: Ard Biesheuvel <ardb@kernel.org>

Fuad reports that in some cases, the KVM init code may apply relocations
to variables that reside in .data, and does so via the linear map. This
means that remapping .data read-only beforehand is a bad idea, and
results in an early boot crash.

These variables in .data are only present when CONFIG_NVHE_EL2_DEBUG or
CONFIG_NVHE_EL2_TRACING are enabled, which is why it was not spotted in
testing.

So move the remap to mark_rodata_ro(), which is a reasonable place to
put this, and ensures that it happens much later during the boot. It
also means that rodata=off is now taken into account, and so the linear
alias will remain writable in that case.

Cc: Fuad Tabba <fuad.tabba@linux.dev>
Fixes: f2ba877402e5 ("arm64: mm: Map the kernel data/bss read-only in the linear map")
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 arch/arm64/mm/mmu.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
index 9f354971b7e4..1f7eca86b5c1 100644
--- a/arch/arm64/mm/mmu.c
+++ b/arch/arm64/mm/mmu.c
@@ -1198,11 +1198,6 @@ static void __init map_mem(void)
 		__map_memblock(start, end, pgprot_tagged(PAGE_KERNEL),
 			       flags);
 	}
-
-	/* Map the kernel data/bss read-only in the linear map */
-	__map_memblock(init_end, kernel_end, PAGE_KERNEL_RO, flags);
-	flush_tlb_kernel_range((unsigned long)lm_alias(__init_end),
-			       (unsigned long)lm_alias(__bss_stop));
 }
 
 void mark_rodata_ro(void)
@@ -1221,6 +1216,12 @@ void mark_rodata_ro(void)
 	update_mapping_prot(__pa_symbol(_text), (unsigned long)_text,
 			    (unsigned long)_stext - (unsigned long)_text,
 			    PAGE_KERNEL_RO);
+
+	/* Map the kernel data/bss read-only in the linear map */
+	update_mapping_prot(__pa_symbol(__init_end),
+			    (unsigned long)lm_alias(__init_end),
+			    (unsigned long)__bss_stop - (unsigned long)__init_end,
+			    PAGE_KERNEL_RO);
 }
 
 static void __init declare_vma(struct vm_struct *vma,
-- 
2.55.0.rc0.738.g0c8ab3ebcc-goog



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

* Re: [PATCH] arm64: mm: Defer read-only remap of data/bss linear alias
  2026-06-19 16:39 [PATCH] arm64: mm: Defer read-only remap of data/bss linear alias Ard Biesheuvel
@ 2026-06-22 15:23 ` Kevin Brodsky
  2026-06-22 16:42   ` Ard Biesheuvel
  2026-06-22 18:13 ` Fuad Tabba
  1 sibling, 1 reply; 5+ messages in thread
From: Kevin Brodsky @ 2026-06-22 15:23 UTC (permalink / raw)
  To: Ard Biesheuvel, linux-arm-kernel
  Cc: linux-kernel, catalin.marinas, will, Ard Biesheuvel, Fuad Tabba

On 19/06/2026 18:39, Ard Biesheuvel wrote:
> From: Ard Biesheuvel <ardb@kernel.org>
>
> Fuad reports that in some cases, the KVM init code may apply relocations
> to variables that reside in .data, and does so via the linear map. This
> means that remapping .data read-only beforehand is a bad idea, and
> results in an early boot crash.
>
> These variables in .data are only present when CONFIG_NVHE_EL2_DEBUG or
> CONFIG_NVHE_EL2_TRACING are enabled, which is why it was not spotted in
> testing.
>
> So move the remap to mark_rodata_ro(), which is a reasonable place to
> put this, and ensures that it happens much later during the boot. It
> also means that rodata=off is now taken into account, and so the linear
> alias will remain writable in that case.
>
> Cc: Fuad Tabba <fuad.tabba@linux.dev>
> Fixes: f2ba877402e5 ("arm64: mm: Map the kernel data/bss read-only in the linear map")
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> ---
>  arch/arm64/mm/mmu.c | 11 ++++++-----
>  1 file changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
> index 9f354971b7e4..1f7eca86b5c1 100644
> --- a/arch/arm64/mm/mmu.c
> +++ b/arch/arm64/mm/mmu.c
> @@ -1198,11 +1198,6 @@ static void __init map_mem(void)
>  		__map_memblock(start, end, pgprot_tagged(PAGE_KERNEL),
>  			       flags);
>  	}
> -
> -	/* Map the kernel data/bss read-only in the linear map */
> -	__map_memblock(init_end, kernel_end, PAGE_KERNEL_RO, flags);
> -	flush_tlb_kernel_range((unsigned long)lm_alias(__init_end),
> -			       (unsigned long)lm_alias(__bss_stop));
>  }
>  
>  void mark_rodata_ro(void)
> @@ -1221,6 +1216,12 @@ void mark_rodata_ro(void)
>  	update_mapping_prot(__pa_symbol(_text), (unsigned long)_text,
>  			    (unsigned long)_stext - (unsigned long)_text,
>  			    PAGE_KERNEL_RO);
> +
> +	/* Map the kernel data/bss read-only in the linear map */
> +	update_mapping_prot(__pa_symbol(__init_end),
> +			    (unsigned long)lm_alias(__init_end),
> +			    (unsigned long)__bss_stop - (unsigned long)__init_end,
> +			    PAGE_KERNEL_RO);

I was rather confused by this patch until I saw in master:

f102131c842d Revert "arm64: mm: Unmap kernel data/bss entirely from the
linear map"
81479b6888f8 Revert "arm64: mm: Defer remap of linear alias of data/bss"

... which explains the diff, but it looks like there are multiple issues
here. Is my understanding correct that these reverts fix kvm_arm_init()
because it needs to *read* from the linear map, while this new patch
fixes some KVM configurations that need to *write* to the linear map?

If so it might be good to give a recap in the commit message, because
the reverts are easy to miss.

It would also be good to explain why we can't do this in
mark_linear_text_alias_ro() (presumably because this is too early). I do
agree that honouring rodata=off is a good thing, though.

BTW, if only kvm_arm_init() needs to read the data/bss via the linear
map, maybe we can still unmap it later?

- Kevin


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

* Re: [PATCH] arm64: mm: Defer read-only remap of data/bss linear alias
  2026-06-22 15:23 ` Kevin Brodsky
@ 2026-06-22 16:42   ` Ard Biesheuvel
  2026-06-23  7:53     ` Kevin Brodsky
  0 siblings, 1 reply; 5+ messages in thread
From: Ard Biesheuvel @ 2026-06-22 16:42 UTC (permalink / raw)
  To: Kevin Brodsky, Ard Biesheuvel, linux-arm-kernel
  Cc: linux-kernel, Catalin Marinas, Will Deacon, Fuad Tabba

Hi Kevin,

On Mon, Jun 22, 2026 at 5:23 PM Kevin Brodsky <kevin.brodsky@arm.com> wrote:
>
> On 19/06/2026 18:39, Ard Biesheuvel wrote:
> > From: Ard Biesheuvel <ardb@kernel.org>
> >
> > Fuad reports that in some cases, the KVM init code may apply relocations
> > to variables that reside in .data, and does so via the linear map. This
> > means that remapping .data read-only beforehand is a bad idea, and
> > results in an early boot crash.
> >
> > These variables in .data are only present when CONFIG_NVHE_EL2_DEBUG or
> > CONFIG_NVHE_EL2_TRACING are enabled, which is why it was not spotted in
> > testing.
> >
> > So move the remap to mark_rodata_ro(), which is a reasonable place to
> > put this, and ensures that it happens much later during the boot. It
> > also means that rodata=off is now taken into account, and so the linear
> > alias will remain writable in that case.
> >
> > Cc: Fuad Tabba <fuad.tabba@linux.dev>
> > Fixes: f2ba877402e5 ("arm64: mm: Map the kernel data/bss read-only in the linear map")
> > Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> > ---
> >  arch/arm64/mm/mmu.c | 11 ++++++-----
> >  1 file changed, 6 insertions(+), 5 deletions(-)
> >
> > diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
> > index 9f354971b7e4..1f7eca86b5c1 100644
> > --- a/arch/arm64/mm/mmu.c
> > +++ b/arch/arm64/mm/mmu.c
> > @@ -1198,11 +1198,6 @@ static void __init map_mem(void)
> >               __map_memblock(start, end, pgprot_tagged(PAGE_KERNEL),
> >                              flags);
> >       }
> > -
> > -     /* Map the kernel data/bss read-only in the linear map */
> > -     __map_memblock(init_end, kernel_end, PAGE_KERNEL_RO, flags);
> > -     flush_tlb_kernel_range((unsigned long)lm_alias(__init_end),
> > -                            (unsigned long)lm_alias(__bss_stop));
> >  }
> > 
> >  void mark_rodata_ro(void)
> > @@ -1221,6 +1216,12 @@ void mark_rodata_ro(void)
> >       update_mapping_prot(__pa_symbol(_text), (unsigned long)_text,
> >                           (unsigned long)_stext - (unsigned long)_text,
> >                           PAGE_KERNEL_RO);
> > +
> > +     /* Map the kernel data/bss read-only in the linear map */
> > +     update_mapping_prot(__pa_symbol(__init_end),
> > +                         (unsigned long)lm_alias(__init_end),
> > +                         (unsigned long)__bss_stop - (unsigned long)__init_end,
> > +                         PAGE_KERNEL_RO);
>
> I was rather confused by this patch until I saw in master:
>
> f102131c842d Revert "arm64: mm: Unmap kernel data/bss entirely from the
> linear map"
> 81479b6888f8 Revert "arm64: mm: Defer remap of linear alias of data/bss"
>
> ... which explains the diff, but it looks like there are multiple issues
> here. Is my understanding correct that these reverts fix kvm_arm_init()
> because it needs to *read* from the linear map, while this new patch
> fixes some KVM configurations that need to *write* to the linear map?
>

Basically, yes.

> If so it might be good to give a recap in the commit message, because
> the reverts are easy to miss.
>

Ok

> It would also be good to explain why we can't do this in
> mark_linear_text_alias_ro() (presumably because this is too early). I do
> agree that honouring rodata=off is a good thing, though.
>

I didn't consider that tbh - I'll check whether that is a more suitable
location. But generally, I think doing it later rather than earlier is
fine because it reduces the likelihood of hidden issues such as the one
being addressed by this patch, and I don't think doing this very early
makes a meaningful difference in terms of robustness.

> BTW, if only kvm_arm_init() needs to read the data/bss via the linear
> map, maybe we can still unmap it later?
>

Yes, it is still my intent to bring back the patches that unmap this alias
entirely, including some additional tweaks for the fixmap PTE level table
that I dropped after an earlier revision. But that will be 7.3 material,
whereas this fixes a regression due to the changes that were pulled for 7.2.
 


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

* Re: [PATCH] arm64: mm: Defer read-only remap of data/bss linear alias
  2026-06-19 16:39 [PATCH] arm64: mm: Defer read-only remap of data/bss linear alias Ard Biesheuvel
  2026-06-22 15:23 ` Kevin Brodsky
@ 2026-06-22 18:13 ` Fuad Tabba
  1 sibling, 0 replies; 5+ messages in thread
From: Fuad Tabba @ 2026-06-22 18:13 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: linux-arm-kernel, linux-kernel, catalin.marinas, will,
	Ard Biesheuvel

On Fri, 19 Jun 2026 at 17:40, Ard Biesheuvel <ardb+git@google.com> wrote:
>
> From: Ard Biesheuvel <ardb@kernel.org>
>
> Fuad reports that in some cases, the KVM init code may apply relocations
> to variables that reside in .data, and does so via the linear map. This
> means that remapping .data read-only beforehand is a bad idea, and
> results in an early boot crash.
>
> These variables in .data are only present when CONFIG_NVHE_EL2_DEBUG or
> CONFIG_NVHE_EL2_TRACING are enabled, which is why it was not spotted in
> testing.
>
> So move the remap to mark_rodata_ro(), which is a reasonable place to
> put this, and ensures that it happens much later during the boot. It
> also means that rodata=off is now taken into account, and so the linear
> alias will remain writable in that case.
>
> Cc: Fuad Tabba <fuad.tabba@linux.dev>
> Fixes: f2ba877402e5 ("arm64: mm: Map the kernel data/bss read-only in the linear map")
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> ---

This fixes it, and the changes look good to me, thanks.

Reviewed-by: Fuad Tabba <fuad.tabba@linux.dev>
Tested-by: Fuad Tabba < fuad.tabba@linux.dev>

Cheers,
/fuad

>  arch/arm64/mm/mmu.c | 11 ++++++-----
>  1 file changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
> index 9f354971b7e4..1f7eca86b5c1 100644
> --- a/arch/arm64/mm/mmu.c
> +++ b/arch/arm64/mm/mmu.c
> @@ -1198,11 +1198,6 @@ static void __init map_mem(void)
>                 __map_memblock(start, end, pgprot_tagged(PAGE_KERNEL),
>                                flags);
>         }
> -
> -       /* Map the kernel data/bss read-only in the linear map */
> -       __map_memblock(init_end, kernel_end, PAGE_KERNEL_RO, flags);
> -       flush_tlb_kernel_range((unsigned long)lm_alias(__init_end),
> -                              (unsigned long)lm_alias(__bss_stop));
>  }
>
>  void mark_rodata_ro(void)
> @@ -1221,6 +1216,12 @@ void mark_rodata_ro(void)
>         update_mapping_prot(__pa_symbol(_text), (unsigned long)_text,
>                             (unsigned long)_stext - (unsigned long)_text,
>                             PAGE_KERNEL_RO);
> +
> +       /* Map the kernel data/bss read-only in the linear map */
> +       update_mapping_prot(__pa_symbol(__init_end),
> +                           (unsigned long)lm_alias(__init_end),
> +                           (unsigned long)__bss_stop - (unsigned long)__init_end,
> +                           PAGE_KERNEL_RO);
>  }
>
>  static void __init declare_vma(struct vm_struct *vma,
> --
> 2.55.0.rc0.738.g0c8ab3ebcc-goog
>


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

* Re: [PATCH] arm64: mm: Defer read-only remap of data/bss linear alias
  2026-06-22 16:42   ` Ard Biesheuvel
@ 2026-06-23  7:53     ` Kevin Brodsky
  0 siblings, 0 replies; 5+ messages in thread
From: Kevin Brodsky @ 2026-06-23  7:53 UTC (permalink / raw)
  To: Ard Biesheuvel, Ard Biesheuvel, linux-arm-kernel
  Cc: linux-kernel, Catalin Marinas, Will Deacon, Fuad Tabba

On 22/06/2026 18:42, Ard Biesheuvel wrote:
>> It would also be good to explain why we can't do this in
>> mark_linear_text_alias_ro() (presumably because this is too early). I do
>> agree that honouring rodata=off is a good thing, though.
>>
> I didn't consider that tbh - I'll check whether that is a more suitable
> location. But generally, I think doing it later rather than earlier is
> fine because it reduces the likelihood of hidden issues such as the one
> being addressed by this patch, and I don't think doing this very early
> makes a meaningful difference in terms of robustness.

That makes sense, probably not worth the hassle of checking if
mark_linear_text_alias_ro() would work.

>> BTW, if only kvm_arm_init() needs to read the data/bss via the linear
>> map, maybe we can still unmap it later?
>>
> Yes, it is still my intent to bring back the patches that unmap this alias
> entirely, including some additional tweaks for the fixmap PTE level table
> that I dropped after an earlier revision. But that will be 7.3 material,
> whereas this fixes a regression due to the changes that were pulled for 7.2.

Sounds good.

- Kevin


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

end of thread, other threads:[~2026-06-23  7:53 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-19 16:39 [PATCH] arm64: mm: Defer read-only remap of data/bss linear alias Ard Biesheuvel
2026-06-22 15:23 ` Kevin Brodsky
2026-06-22 16:42   ` Ard Biesheuvel
2026-06-23  7:53     ` Kevin Brodsky
2026-06-22 18:13 ` Fuad Tabba

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox