[PATCH v4 0/2] arm64: refactor the rodata=xxx

[PATCH v4 0/2] arm64: refactor the rodata=xxx

[Huang Shijie]

From Documentation/admin-guide/kernel-parameters.txt, we know that:
 rodata=	[KNL,EARLY]
	on	Mark read-only kernel memory as read-only (default).
	off	Leave read-only kernel memory writable for debugging.
	full	Mark read-only kernel memory and aliases as read-only
		[arm64]

So the "rodata=on" is the default.

But the current code does not follow the document, it makes "rodata=full"
as the default.

This patch set follows Anshuman Khandual's suggetions. 
It makes the "rodata=on" as the default, and removes the CONFIG_RODATA_FULL_DEFAULT_ENABLED.

v4:
  Follows Anshuman Khandual/Ard Biesheuvel's suggetions:
  - Change commit message format.
  - Change the titile name.
  - others

v3:
  Follows Anshuman Khandual's suggetions:
  - Merge patch 1 and patch 3 into one patch.
  - Remove patch 4 
  - update comments and document.
   https://lists.infradead.org/pipermail/linux-arm-kernel/2024-December/984344.html

v2:
  Follows Will's suggetions.
  Add a new file fine-tuning-tips.rst for the expert users.
   https://lists.infradead.org/pipermail/linux-arm-kernel/2024-November/981190.html

v1:
   https://lists.infradead.org/pipermail/linux-arm-kernel/2024-October/971415.html


Huang Shijie (2):
  arm64: refactor the rodata=xxx
  arm64/Kconfig: Remove CONFIG_RODATA_FULL_DEFAULT_ENABLED

 .../admin-guide/kernel-parameters.txt         |  2 +-
 arch/arm64/Kconfig                            | 14 ----------
 arch/arm64/include/asm/setup.h                | 26 +++++++++++++++++--
 arch/arm64/mm/pageattr.c                      |  2 +-
 4 files changed, 26 insertions(+), 18 deletions(-)

-- 
2.40.1

[PATCH v4 1/2] arm64: refactor the rodata=xxx

[Huang Shijie]

As per admin guide documentation, "rodata=on" should be the default on
platforms. Documentation/admin-guide/kernel-parameters.txt describes
these options as

   rodata=         [KNL,EARLY]
           on      Mark read-only kernel memory as read-only (default).
           off     Leave read-only kernel memory writable for debugging.
           full    Mark read-only kernel memory and aliases as read-only
                   [arm64]

But on arm64 platform, "rodata=full" is the default instead. This patch
implements the following changes.

 - Make "rodata=on" behaviour same as the original "rodata=full"
 - Make "rodata=noalias" (new) behaviour same as the original "rodata=on"
 - Drop the original "rodata=full"
 - Add comment for arch_parse_debug_rodata()
 - Update kernel-parameters.txt as required

After this patch, the "rodata=on" will be the default on arm64 platform
as well.

Signed-off-by: Huang Shijie <shijie@os.amperecomputing.com>
---
 .../admin-guide/kernel-parameters.txt         |  2 +-
 arch/arm64/include/asm/setup.h                | 26 +++++++++++++++++--
 2 files changed, 25 insertions(+), 3 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index a22b7e621007..51bce7b9d805 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -5901,7 +5901,7 @@
 	rodata=		[KNL,EARLY]
 		on	Mark read-only kernel memory as read-only (default).
 		off	Leave read-only kernel memory writable for debugging.
-		full	Mark read-only kernel memory and aliases as read-only
+		noalias	Use more block mappings,may have better performance.
 		        [arm64]
 
 	rockchip.usb_uart
diff --git a/arch/arm64/include/asm/setup.h b/arch/arm64/include/asm/setup.h
index ba269a7a3201..37f58a603224 100644
--- a/arch/arm64/include/asm/setup.h
+++ b/arch/arm64/include/asm/setup.h
@@ -13,6 +13,28 @@
 extern phys_addr_t __fdt_pointer __initdata;
 extern u64 __cacheline_aligned boot_args[4];
 
+/*
+ * rodata=on (default)
+ *
+ *    This applies read-only attributes to VM areas and to the linear
+ *    alias of the backing pages as well. This prevents code or read-
+ *    only data from being modified (inadvertently or intentionally),
+ *    via another mapping for the same memory page.
+ *
+ *    But this might cause linear map region to be mapped down to base
+ *    pages, which may adversely affect performance in some cases.
+ *
+ * rodata=off
+ *
+ *    This provides more block mappings and contiguous hints for linear
+ *    map region which would minimize TLB footprint. This also leaves
+ *    read-only kernel memory writable for debugging.
+ *
+ * rodata=noalias
+ *
+ *    This provides more block mappings and contiguous hints for linear
+ *    map region which would minimize TLB footprint.
+ */
 static inline bool arch_parse_debug_rodata(char *arg)
 {
 	extern bool rodata_enabled;
@@ -21,7 +43,7 @@ static inline bool arch_parse_debug_rodata(char *arg)
 	if (!arg)
 		return false;
 
-	if (!strcmp(arg, "full")) {
+	if (!strcmp(arg, "on")) {
 		rodata_enabled = rodata_full = true;
 		return true;
 	}
@@ -31,7 +53,7 @@ static inline bool arch_parse_debug_rodata(char *arg)
 		return true;
 	}
 
-	if (!strcmp(arg, "on")) {
+	if (!strcmp(arg, "noalias")) {
 		rodata_enabled = true;
 		rodata_full = false;
 		return true;
-- 
2.40.1

[PATCH v4 1/2 fix] arm64: refactor the rodata=xxx

[Huang Shijie]

As per admin guide documentation, "rodata=on" should be the default on
platforms. Documentation/admin-guide/kernel-parameters.txt describes
these options as

   rodata=         [KNL,EARLY]
           on      Mark read-only kernel memory as read-only (default).
           off     Leave read-only kernel memory writable for debugging.
           full    Mark read-only kernel memory and aliases as read-only
                   [arm64]

But on arm64 platform, "rodata=full" is the default instead. This patch
implements the following changes.

 - Make "rodata=on" behaviour same as the original "rodata=full"
 - Make "rodata=noalias" (new) behaviour same as the original "rodata=on"
 - Drop the original "rodata=full"
 - Add comment for arch_parse_debug_rodata()
 - Update kernel-parameters.txt as required

After this patch, the "rodata=on" will be the default on arm64 platform
as well.

Signed-off-by: Huang Shijie <shijie@os.amperecomputing.com>
---
Add more descriptions for "noalias":
  It is not a security feature yet.
---
 .../admin-guide/kernel-parameters.txt         |  3 ++-
 arch/arm64/include/asm/setup.h                | 27 +++++++++++++++++--
 2 files changed, 27 insertions(+), 3 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index a22b7e621007..f5db01eecbd3 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -5901,7 +5901,8 @@
 	rodata=		[KNL,EARLY]
 		on	Mark read-only kernel memory as read-only (default).
 		off	Leave read-only kernel memory writable for debugging.
-		full	Mark read-only kernel memory and aliases as read-only
+		noalias	Use more block mappings,may have better performance.
+			But this is not a security feature.
 		        [arm64]
 
 	rockchip.usb_uart
diff --git a/arch/arm64/include/asm/setup.h b/arch/arm64/include/asm/setup.h
index ba269a7a3201..0ef57d19fc2a 100644
--- a/arch/arm64/include/asm/setup.h
+++ b/arch/arm64/include/asm/setup.h
@@ -13,6 +13,29 @@
 extern phys_addr_t __fdt_pointer __initdata;
 extern u64 __cacheline_aligned boot_args[4];
 
+/*
+ * rodata=on (default)
+ *
+ *    This applies read-only attributes to VM areas and to the linear
+ *    alias of the backing pages as well. This prevents code or read-
+ *    only data from being modified (inadvertently or intentionally),
+ *    via another mapping for the same memory page.
+ *
+ *    But this might cause linear map region to be mapped down to base
+ *    pages, which may adversely affect performance in some cases.
+ *
+ * rodata=off
+ *
+ *    This provides more block mappings and contiguous hints for linear
+ *    map region which would minimize TLB footprint. This also leaves
+ *    read-only kernel memory writable for debugging.
+ *
+ * rodata=noalias
+ *
+ *    This provides more block mappings and contiguous hints for linear
+ *    map region which would minimize TLB footprint. This is not a
+ *    security feature yet.
+ */
 static inline bool arch_parse_debug_rodata(char *arg)
 {
 	extern bool rodata_enabled;
@@ -21,7 +44,7 @@ static inline bool arch_parse_debug_rodata(char *arg)
 	if (!arg)
 		return false;
 
-	if (!strcmp(arg, "full")) {
+	if (!strcmp(arg, "on")) {
 		rodata_enabled = rodata_full = true;
 		return true;
 	}
@@ -31,7 +54,7 @@ static inline bool arch_parse_debug_rodata(char *arg)
 		return true;
 	}
 
-	if (!strcmp(arg, "on")) {
+	if (!strcmp(arg, "noalias")) {
 		rodata_enabled = true;
 		rodata_full = false;
 		return true;
-- 
2.40.1

Re: [PATCH v4 1/2 fix] arm64: refactor the rodata=xxx

[Ard Biesheuvel]

Hello Huang Shije,

On Fri, 13 Dec 2024 at 06:32, Huang Shijie
<shijie@os.amperecomputing.com> wrote:
>
> As per admin guide documentation, "rodata=on" should be the default on
> platforms. Documentation/admin-guide/kernel-parameters.txt describes
> these options as
>
>    rodata=         [KNL,EARLY]
>            on      Mark read-only kernel memory as read-only (default).
>            off     Leave read-only kernel memory writable for debugging.
>            full    Mark read-only kernel memory and aliases as read-only
>                    [arm64]
>
> But on arm64 platform, "rodata=full" is the default instead. This patch
> implements the following changes.
>
>  - Make "rodata=on" behaviour same as the original "rodata=full"
>  - Make "rodata=noalias" (new) behaviour same as the original "rodata=on"
>  - Drop the original "rodata=full"
>  - Add comment for arch_parse_debug_rodata()
>  - Update kernel-parameters.txt as required
>
> After this patch, the "rodata=on" will be the default on arm64 platform
> as well.
>
> Signed-off-by: Huang Shijie <shijie@os.amperecomputing.com>
> ---
> Add more descriptions for "noalias":
>   It is not a security feature yet.

Why did you add that?

How do you envisage 'noalias' becoming a security feature? The point
of 'full' rodata was to harden the read-only regions in the vmalloc
space against inadvertent modification via the writeable linear alias,
so 'noalias' is less secure than rodata=full, and should be documented
as such.

> ---
>  .../admin-guide/kernel-parameters.txt         |  3 ++-
>  arch/arm64/include/asm/setup.h                | 27 +++++++++++++++++--
>  2 files changed, 27 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index a22b7e621007..f5db01eecbd3 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -5901,7 +5901,8 @@
>         rodata=         [KNL,EARLY]
>                 on      Mark read-only kernel memory as read-only (default).
>                 off     Leave read-only kernel memory writable for debugging.
> -               full    Mark read-only kernel memory and aliases as read-only
> +               noalias Use more block mappings,may have better performance.
> +                       But this is not a security feature.
>                         [arm64]
>
>         rockchip.usb_uart
> diff --git a/arch/arm64/include/asm/setup.h b/arch/arm64/include/asm/setup.h
> index ba269a7a3201..0ef57d19fc2a 100644
> --- a/arch/arm64/include/asm/setup.h
> +++ b/arch/arm64/include/asm/setup.h
> @@ -13,6 +13,29 @@
>  extern phys_addr_t __fdt_pointer __initdata;
>  extern u64 __cacheline_aligned boot_args[4];
>
> +/*
> + * rodata=on (default)
> + *
> + *    This applies read-only attributes to VM areas and to the linear
> + *    alias of the backing pages as well. This prevents code or read-
> + *    only data from being modified (inadvertently or intentionally),
> + *    via another mapping for the same memory page.
> + *
> + *    But this might cause linear map region to be mapped down to base
> + *    pages, which may adversely affect performance in some cases.
> + *
> + * rodata=off
> + *
> + *    This provides more block mappings and contiguous hints for linear
> + *    map region which would minimize TLB footprint. This also leaves
> + *    read-only kernel memory writable for debugging.
> + *
> + * rodata=noalias
> + *
> + *    This provides more block mappings and contiguous hints for linear
> + *    map region which would minimize TLB footprint. This is not a
> + *    security feature yet.

Better replace the last sentence with

"This leaves the linear alias of read-only mappings in the vmalloc
space writeable, making them susceptible to inadvertent modification
by software."

Re: [PATCH v4 1/2 fix] arm64: refactor the rodata=xxx

[Shijie Huang]

Hi Ard,

On 2024/12/13 15:30, Ard Biesheuvel wrote:
> Hello Huang Shije,
>
> On Fri, 13 Dec 2024 at 06:32, Huang Shijie
> <shijie@os.amperecomputing.com> wrote:
>> As per admin guide documentation, "rodata=on" should be the default on
>> platforms. Documentation/admin-guide/kernel-parameters.txt describes
>> these options as
>>
>>     rodata=         [KNL,EARLY]
>>             on      Mark read-only kernel memory as read-only (default).
>>             off     Leave read-only kernel memory writable for debugging.
>>             full    Mark read-only kernel memory and aliases as read-only
>>                     [arm64]
>>
>> But on arm64 platform, "rodata=full" is the default instead. This patch
>> implements the following changes.
>>
>>   - Make "rodata=on" behaviour same as the original "rodata=full"
>>   - Make "rodata=noalias" (new) behaviour same as the original "rodata=on"
>>   - Drop the original "rodata=full"
>>   - Add comment for arch_parse_debug_rodata()
>>   - Update kernel-parameters.txt as required
>>
>> After this patch, the "rodata=on" will be the default on arm64 platform
>> as well.
>>
>> Signed-off-by: Huang Shijie <shijie@os.amperecomputing.com>
>> ---
>> Add more descriptions for "noalias":
>>    It is not a security feature yet.
> Why did you add that?
The following is the current status of "rodata=noalias" by checking the 
kernel page table in my machine:

    1) the kernel code keeps read-only in both the "vmalloc area" and 
the "linear area".

    2) But there is a read-only memory range which is read-only in 
"vmalloc area", but its linear alias is read-write in the "linear area".


Maybe the "security" is not a proper word.


>
> How do you envisage 'noalias' becoming a security feature? The point

for the case 2) above, if its linear alias is also mapped as read-only,

can we think it is safe as the original "rodata=full"?


> of 'full' rodata was to harden the read-only regions in the vmalloc
> space against inadvertent modification via the writeable linear alias,
> so 'noalias' is less secure than rodata=full, and should be documented
> as such.


>> ---
>>   .../admin-guide/kernel-parameters.txt         |  3 ++-
>>   arch/arm64/include/asm/setup.h                | 27 +++++++++++++++++--
>>   2 files changed, 27 insertions(+), 3 deletions(-)
>>
>> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
>> index a22b7e621007..f5db01eecbd3 100644
>> --- a/Documentation/admin-guide/kernel-parameters.txt
>> +++ b/Documentation/admin-guide/kernel-parameters.txt
>> @@ -5901,7 +5901,8 @@
>>          rodata=         [KNL,EARLY]
>>                  on      Mark read-only kernel memory as read-only (default).
>>                  off     Leave read-only kernel memory writable for debugging.
>> -               full    Mark read-only kernel memory and aliases as read-only
>> +               noalias Use more block mappings,may have better performance.
>> +                       But this is not a security feature.
>>                          [arm64]

What's about change it to:

    diff --git a/Documentation/admin-guide/kernel-parameters.txt 
b/Documentation/admin-guide/kernel-parameters.txt
index a22b7e621007..3d4aef0d0811 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -5901,7 +5901,8 @@
         rodata=         [KNL,EARLY]
                  on      Mark read-only kernel memory as read-only 
(default).
                  off     Leave read-only kernel memory writable for 
debugging.
-               full     Mark read-only kernel memory and aliases as 
read-only
+               noalias Use more block mappings,may have better performance.
+                        It is less secure then "rodata=on".
                         [arm64]



>>
>>          rockchip.usb_uart
>> diff --git a/arch/arm64/include/asm/setup.h b/arch/arm64/include/asm/setup.h
>> index ba269a7a3201..0ef57d19fc2a 100644
>> --- a/arch/arm64/include/asm/setup.h
>> +++ b/arch/arm64/include/asm/setup.h
>> @@ -13,6 +13,29 @@
>>   extern phys_addr_t __fdt_pointer __initdata;
>>   extern u64 __cacheline_aligned boot_args[4];
>>
>> +/*
>> + * rodata=on (default)
>> + *
>> + *    This applies read-only attributes to VM areas and to the linear
>> + *    alias of the backing pages as well. This prevents code or read-
>> + *    only data from being modified (inadvertently or intentionally),
>> + *    via another mapping for the same memory page.
>> + *
>> + *    But this might cause linear map region to be mapped down to base
>> + *    pages, which may adversely affect performance in some cases.
>> + *
>> + * rodata=off
>> + *
>> + *    This provides more block mappings and contiguous hints for linear
>> + *    map region which would minimize TLB footprint. This also leaves
>> + *    read-only kernel memory writable for debugging.
>> + *
>> + * rodata=noalias
>> + *
>> + *    This provides more block mappings and contiguous hints for linear
>> + *    map region which would minimize TLB footprint. This is not a
>> + *    security feature yet.
> Better replace the last sentence with
>
> "This leaves the linear alias of read-only mappings in the vmalloc
> space writeable, making them susceptible to inadvertent modification
> by software."

No problem.


Thanks

Huang Shijie

Re: [PATCH v4 1/2 fix] arm64: refactor the rodata=xxx

[Ard Biesheuvel]

On Fri, 13 Dec 2024 at 09:23, Shijie Huang
<shijie@amperemail.onmicrosoft.com> wrote:
>
> Hi Ard,
>
> On 2024/12/13 15:30, Ard Biesheuvel wrote:
> > Hello Huang Shije,
> >
> > On Fri, 13 Dec 2024 at 06:32, Huang Shijie
> > <shijie@os.amperecomputing.com> wrote:
> >> As per admin guide documentation, "rodata=on" should be the default on
> >> platforms. Documentation/admin-guide/kernel-parameters.txt describes
> >> these options as
> >>
> >>     rodata=         [KNL,EARLY]
> >>             on      Mark read-only kernel memory as read-only (default).
> >>             off     Leave read-only kernel memory writable for debugging.
> >>             full    Mark read-only kernel memory and aliases as read-only
> >>                     [arm64]
> >>
> >> But on arm64 platform, "rodata=full" is the default instead. This patch
> >> implements the following changes.
> >>
> >>   - Make "rodata=on" behaviour same as the original "rodata=full"
> >>   - Make "rodata=noalias" (new) behaviour same as the original "rodata=on"
> >>   - Drop the original "rodata=full"
> >>   - Add comment for arch_parse_debug_rodata()
> >>   - Update kernel-parameters.txt as required
> >>
> >> After this patch, the "rodata=on" will be the default on arm64 platform
> >> as well.
> >>
> >> Signed-off-by: Huang Shijie <shijie@os.amperecomputing.com>
> >> ---
> >> Add more descriptions for "noalias":
> >>    It is not a security feature yet.
> > Why did you add that?
> The following is the current status of "rodata=noalias" by checking the
> kernel page table in my machine:
>
>     1) the kernel code keeps read-only in both the "vmalloc area" and
> the "linear area".
>
>     2) But there is a read-only memory range which is read-only in
> "vmalloc area", but its linear alias is read-write in the "linear area".
>
>
> Maybe the "security" is not a proper word.
>

There is nothing wrong with the word 'security' as long as you are
clear about the fact that rodata=noalias decreases it.

>
> >
> > How do you envisage 'noalias' becoming a security feature? The point
>
> for the case 2) above, if its linear alias is also mapped as read-only,
>
> can we think it is safe as the original "rodata=full"?
>

No, it is not. Why would we bother with rodata=full (which is costly
in terms of TLB pressure) if rodata=noalias is equally safe?

Re: [PATCH v4 1/2 fix] arm64: refactor the rodata=xxx

[Shijie Huang]

On 2024/12/13 16:26, Ard Biesheuvel wrote:
>>> How do you envisage 'noalias' becoming a security feature? The point
>> for the case 2) above, if its linear alias is also mapped as read-only,
>>
>> can we think it is safe as the original "rodata=full"?
>>
> No, it is not. Why would we bother with rodata=full (which is costly
> in terms of TLB pressure) if rodata=noalias is equally safe?

okay, thanks.

I will wait for two days, and send out the new patch if there no other 
comment.


Thanks

Huang Shijie

[PATCH v4 1/2 fix-v2 ] arm64: refactor the rodata=xxx

[Huang Shijie]

As per admin guide documentation, "rodata=on" should be the default on
platforms. Documentation/admin-guide/kernel-parameters.txt describes
these options as

   rodata=         [KNL,EARLY]
           on      Mark read-only kernel memory as read-only (default).
           off     Leave read-only kernel memory writable for debugging.
           full    Mark read-only kernel memory and aliases as read-only
                   [arm64]

But on arm64 platform, "rodata=full" is the default instead. This patch
implements the following changes.

 - Make "rodata=on" behaviour same as the original "rodata=full"
 - Make "rodata=noalias" (new) behaviour same as the original "rodata=on"
 - Drop the original "rodata=full"
 - Add comment for arch_parse_debug_rodata()
 - Update kernel-parameters.txt as required

After this patch, the "rodata=on" will be the default on arm64 platform
as well.

Signed-off-by: Huang Shijie <shijie@os.amperecomputing.com>
---
Add more comment for "rodata=noalias" in arch_parse_debug_rodata() from Ard.
---
 .../admin-guide/kernel-parameters.txt         |  2 +-
 arch/arm64/include/asm/setup.h                | 28 +++++++++++++++++--
 2 files changed, 27 insertions(+), 3 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 97c497bdafac..639669324350 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -6105,7 +6105,7 @@
 	rodata=		[KNL,EARLY]
 		on	Mark read-only kernel memory as read-only (default).
 		off	Leave read-only kernel memory writable for debugging.
-		full	Mark read-only kernel memory and aliases as read-only
+		noalias	Use more block mappings,may have better performance.
 		        [arm64]
 
 	rockchip.usb_uart
diff --git a/arch/arm64/include/asm/setup.h b/arch/arm64/include/asm/setup.h
index ba269a7a3201..6b994d0881d1 100644
--- a/arch/arm64/include/asm/setup.h
+++ b/arch/arm64/include/asm/setup.h
@@ -13,6 +13,30 @@
 extern phys_addr_t __fdt_pointer __initdata;
 extern u64 __cacheline_aligned boot_args[4];
 
+/*
+ * rodata=on (default)
+ *
+ *    This applies read-only attributes to VM areas and to the linear
+ *    alias of the backing pages as well. This prevents code or read-
+ *    only data from being modified (inadvertently or intentionally),
+ *    via another mapping for the same memory page.
+ *
+ *    But this might cause linear map region to be mapped down to base
+ *    pages, which may adversely affect performance in some cases.
+ *
+ * rodata=off
+ *
+ *    This provides more block mappings and contiguous hints for linear
+ *    map region which would minimize TLB footprint. This also leaves
+ *    read-only kernel memory writable for debugging.
+ *
+ * rodata=noalias
+ *
+ *    This provides more block mappings and contiguous hints for linear
+ *    map region which would minimize TLB footprint. This leaves the linear
+ *    alias of read-only mappings in the vmalloc space writeable, making
+ *    them susceptible to inadvertent modification by software.
+ */
 static inline bool arch_parse_debug_rodata(char *arg)
 {
 	extern bool rodata_enabled;
@@ -21,7 +45,7 @@ static inline bool arch_parse_debug_rodata(char *arg)
 	if (!arg)
 		return false;
 
-	if (!strcmp(arg, "full")) {
+	if (!strcmp(arg, "on")) {
 		rodata_enabled = rodata_full = true;
 		return true;
 	}
@@ -31,7 +55,7 @@ static inline bool arch_parse_debug_rodata(char *arg)
 		return true;
 	}
 
-	if (!strcmp(arg, "on")) {
+	if (!strcmp(arg, "noalias")) {
 		rodata_enabled = true;
 		rodata_full = false;
 		return true;
-- 
2.40.1

Re: [PATCH v4 1/2 fix-v2 ] arm64: refactor the rodata=xxx

[Will Deacon]

Digging up an old thread...

On Tue, Dec 17, 2024 at 03:17:15PM +0800, Huang Shijie wrote:
> As per admin guide documentation, "rodata=on" should be the default on
> platforms. Documentation/admin-guide/kernel-parameters.txt describes
> these options as
> 
>    rodata=         [KNL,EARLY]
>            on      Mark read-only kernel memory as read-only (default).
>            off     Leave read-only kernel memory writable for debugging.
>            full    Mark read-only kernel memory and aliases as read-only
>                    [arm64]
> 
> But on arm64 platform, "rodata=full" is the default instead. This patch
> implements the following changes.
> 
>  - Make "rodata=on" behaviour same as the original "rodata=full"
>  - Make "rodata=noalias" (new) behaviour same as the original "rodata=on"
>  - Drop the original "rodata=full"
>  - Add comment for arch_parse_debug_rodata()
>  - Update kernel-parameters.txt as required
> 
> After this patch, the "rodata=on" will be the default on arm64 platform
> as well.
> 
> Signed-off-by: Huang Shijie <shijie@os.amperecomputing.com>
> ---
> Add more comment for "rodata=noalias" in arch_parse_debug_rodata() from Ard.
> ---
>  .../admin-guide/kernel-parameters.txt         |  2 +-
>  arch/arm64/include/asm/setup.h                | 28 +++++++++++++++++--
>  2 files changed, 27 insertions(+), 3 deletions(-)

Sorry, but I'd missed this as you'd sent it as a reply to an existing
series. When you send a new version of a patch, please can you post it
as a new thread with an updated version?

I think the idea of this series is good, so if you send a v5 against
mainline then I can review it.

Thanks,

Will

Re: [PATCH v4 1/2 fix-v2 ] arm64: refactor the rodata=xxx

[Shijie Huang]

On 2025/6/27 23:44, Will Deacon wrote:
> Digging up an old thread...
>
> On Tue, Dec 17, 2024 at 03:17:15PM +0800, Huang Shijie wrote:
>> As per admin guide documentation, "rodata=on" should be the default on
>> platforms. Documentation/admin-guide/kernel-parameters.txt describes
>> these options as
>>
>>     rodata=         [KNL,EARLY]
>>             on      Mark read-only kernel memory as read-only (default).
>>             off     Leave read-only kernel memory writable for debugging.
>>             full    Mark read-only kernel memory and aliases as read-only
>>                     [arm64]
>>
>> But on arm64 platform, "rodata=full" is the default instead. This patch
>> implements the following changes.
>>
>>   - Make "rodata=on" behaviour same as the original "rodata=full"
>>   - Make "rodata=noalias" (new) behaviour same as the original "rodata=on"
>>   - Drop the original "rodata=full"
>>   - Add comment for arch_parse_debug_rodata()
>>   - Update kernel-parameters.txt as required
>>
>> After this patch, the "rodata=on" will be the default on arm64 platform
>> as well.
>>
>> Signed-off-by: Huang Shijie <shijie@os.amperecomputing.com>
>> ---
>> Add more comment for "rodata=noalias" in arch_parse_debug_rodata() from Ard.
>> ---
>>   .../admin-guide/kernel-parameters.txt         |  2 +-
>>   arch/arm64/include/asm/setup.h                | 28 +++++++++++++++++--
>>   2 files changed, 27 insertions(+), 3 deletions(-)
> Sorry, but I'd missed this as you'd sent it as a reply to an existing
> series. When you send a new version of a patch, please can you post it
> as a new thread with an updated version?

Okay, I will rebase this patch set and send out it later..


Thanks

Huang Shijie

[PATCH v4 2/2] arm64/Kconfig: Remove CONFIG_RODATA_FULL_DEFAULT_ENABLED

[Huang Shijie]

After patch "arm64: refacotr the rodata=xxx",
the "rodata=on" becomes the default.

     ......................................
	if (!strcmp(arg, "on")) {
		rodata_enabled = rodata_full = true;
		return true;
	}
     ......................................

The rodata_full is always "true" via "rodata=on" and does not
depend on the config RODATA_FULL_DEFAULT_ENABLED anymore,
so it can be dropped.

Reviewed-by: Anshuman Khandual <anshuman.khandual@arm.com>
Signed-off-by: Huang Shijie <shijie@os.amperecomputing.com>
---
 arch/arm64/Kconfig       | 14 --------------
 arch/arm64/mm/pageattr.c |  2 +-
 2 files changed, 1 insertion(+), 15 deletions(-)

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index cbfd357f94a6..1c69982302ed 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -1666,20 +1666,6 @@ config MITIGATE_SPECTRE_BRANCH_HISTORY
 	  When taking an exception from user-space, a sequence of branches
 	  or a firmware call overwrites the branch history.
 
-config RODATA_FULL_DEFAULT_ENABLED
-	bool "Apply r/o permissions of VM areas also to their linear aliases"
-	default y
-	help
-	  Apply read-only attributes of VM areas to the linear alias of
-	  the backing pages as well. This prevents code or read-only data
-	  from being modified (inadvertently or intentionally) via another
-	  mapping of the same memory page. This additional enhancement can
-	  be turned off at runtime by passing rodata=[off|on] (and turned on
-	  with rodata=full if this option is set to 'n')
-
-	  This requires the linear region to be mapped down to pages,
-	  which may adversely affect performance in some cases.
-
 config ARM64_SW_TTBR0_PAN
 	bool "Emulate Privileged Access Never using TTBR0_EL1 switching"
 	depends on !KCSAN
diff --git a/arch/arm64/mm/pageattr.c b/arch/arm64/mm/pageattr.c
index 39fd1f7ff02a..6eef08d8451e 100644
--- a/arch/arm64/mm/pageattr.c
+++ b/arch/arm64/mm/pageattr.c
@@ -20,7 +20,7 @@ struct page_change_data {
 	pgprot_t clear_mask;
 };
 
-bool rodata_full __ro_after_init = IS_ENABLED(CONFIG_RODATA_FULL_DEFAULT_ENABLED);
+bool rodata_full __ro_after_init = true;
 
 bool can_set_direct_map(void)
 {
-- 
2.40.1