Disallow a module to load if SCS dynamic patching fails for its code. For
module loading, instead of running a dry-run to check for patching errors,
try to run patching in the first run and propagate any errors so module
loading will fail.
Signed-off-by: Adrian Barnaś <abarnas@google.com>
---
arch/arm64/include/asm/scs.h | 2 +-
arch/arm64/kernel/module.c | 6 ++++--
arch/arm64/kernel/pi/map_kernel.c | 2 +-
arch/arm64/kernel/pi/patch-scs.c | 15 +++++++++++----
arch/arm64/kernel/pi/pi.h | 2 +-
5 files changed, 18 insertions(+), 9 deletions(-)
diff --git a/arch/arm64/include/asm/scs.h b/arch/arm64/include/asm/scs.h
index a76f9b387a26..ffcfcda87f10 100644
--- a/arch/arm64/include/asm/scs.h
+++ b/arch/arm64/include/asm/scs.h
@@ -53,7 +53,7 @@ enum {
EDYNSCS_INVALID_CFA_OPCODE = 4,
};
-int __pi_scs_patch(const u8 eh_frame[], int size);
+int __pi_scs_patch(const u8 eh_frame[], int size, bool is_module);
#endif /* __ASSEMBLY __ */
diff --git a/arch/arm64/kernel/module.c b/arch/arm64/kernel/module.c
index 40148d2725ce..5d6d228c6156 100644
--- a/arch/arm64/kernel/module.c
+++ b/arch/arm64/kernel/module.c
@@ -484,10 +484,12 @@ int module_finalize(const Elf_Ehdr *hdr,
if (scs_is_dynamic()) {
s = find_section(hdr, sechdrs, ".init.eh_frame");
if (s) {
- ret = __pi_scs_patch((void *)s->sh_addr, s->sh_size);
- if (ret)
+ ret = __pi_scs_patch((void *)s->sh_addr, s->sh_size, true);
+ if (ret) {
pr_err("module %s: error occurred during dynamic SCS patching (%d)\n",
me->name, ret);
+ return -ENOEXEC;
+ }
}
}
diff --git a/arch/arm64/kernel/pi/map_kernel.c b/arch/arm64/kernel/pi/map_kernel.c
index 0f4bd7771859..7187eda9e8a5 100644
--- a/arch/arm64/kernel/pi/map_kernel.c
+++ b/arch/arm64/kernel/pi/map_kernel.c
@@ -98,7 +98,7 @@ static void __init map_kernel(u64 kaslr_offset, u64 va_offset, int root_level)
if (enable_scs) {
scs_patch(__eh_frame_start + va_offset,
- __eh_frame_end - __eh_frame_start);
+ __eh_frame_end - __eh_frame_start, false);
asm("ic ialluis");
dynamic_scs_is_enabled = true;
diff --git a/arch/arm64/kernel/pi/patch-scs.c b/arch/arm64/kernel/pi/patch-scs.c
index 55d0cd64ef71..78266fb1fa61 100644
--- a/arch/arm64/kernel/pi/patch-scs.c
+++ b/arch/arm64/kernel/pi/patch-scs.c
@@ -225,7 +225,7 @@ static int scs_handle_fde_frame(const struct eh_frame *frame,
return 0;
}
-int scs_patch(const u8 eh_frame[], int size)
+int scs_patch(const u8 eh_frame[], int size, bool is_module)
{
int code_alignment_factor = 1;
bool fde_use_sdata8 = false;
@@ -276,12 +276,19 @@ int scs_patch(const u8 eh_frame[], int size)
return EDYNSCS_INVALID_CIE_SDATA_SIZE;
}
} else {
+ /*
+ * For loadable module instead of running a dry run try
+ * to patch scs instruction in place and trigger error
+ * if failed, to prevent module loading.
+ */
ret = scs_handle_fde_frame(frame, code_alignment_factor,
- fde_use_sdata8, true);
+ fde_use_sdata8, !is_module);
if (ret)
return ret;
- scs_handle_fde_frame(frame, code_alignment_factor,
- fde_use_sdata8, false);
+
+ if (!is_module)
+ scs_handle_fde_frame(frame, code_alignment_factor,
+ fde_use_sdata8, false);
}
p += sizeof(frame->size) + frame->size;
diff --git a/arch/arm64/kernel/pi/pi.h b/arch/arm64/kernel/pi/pi.h
index 46cafee7829f..4ccbba24fadc 100644
--- a/arch/arm64/kernel/pi/pi.h
+++ b/arch/arm64/kernel/pi/pi.h
@@ -27,7 +27,7 @@ extern pgd_t init_pg_dir[], init_pg_end[];
void init_feature_override(u64 boot_status, const void *fdt, int chosen);
u64 kaslr_early_init(void *fdt, int chosen);
void relocate_kernel(u64 offset);
-int scs_patch(const u8 eh_frame[], int size);
+int scs_patch(const u8 eh_frame[], int size, bool is_module);
void map_range(u64 *pgd, u64 start, u64 end, u64 pa, pgprot_t prot,
int level, pte_t *tbl, bool may_use_cont, u64 va_offset);
--
2.51.0.534.gc79095c0ca-googHi,
On Fri, 19 Sept 2025 at 14:23, Adrian Barnaś <abarnas@google.com> wrote:
>
> Disallow a module to load if SCS dynamic patching fails for its code. For
> module loading, instead of running a dry-run to check for patching errors,
> try to run patching in the first run and propagate any errors so module
> loading will fail.
>
> Signed-off-by: Adrian Barnaś <abarnas@google.com>
> ---
> arch/arm64/include/asm/scs.h | 2 +-
> arch/arm64/kernel/module.c | 6 ++++--
> arch/arm64/kernel/pi/map_kernel.c | 2 +-
> arch/arm64/kernel/pi/patch-scs.c | 15 +++++++++++----
> arch/arm64/kernel/pi/pi.h | 2 +-
> 5 files changed, 18 insertions(+), 9 deletions(-)
>
> diff --git a/arch/arm64/include/asm/scs.h b/arch/arm64/include/asm/scs.h
> index a76f9b387a26..ffcfcda87f10 100644
> --- a/arch/arm64/include/asm/scs.h
> +++ b/arch/arm64/include/asm/scs.h
> @@ -53,7 +53,7 @@ enum {
> EDYNSCS_INVALID_CFA_OPCODE = 4,
> };
>
> -int __pi_scs_patch(const u8 eh_frame[], int size);
> +int __pi_scs_patch(const u8 eh_frame[], int size, bool is_module);
>
Calling the parameter 'is_module' puts the policy in the SCS patching
code, which now has to reason about how patching a module differs from
patching other code.
So I'd prefer to call this 'two_pass' or 'dry_run' or whatever, where
setting it guarantees that when an error is returned, no function will
be left in an inconsistent state.
> #endif /* __ASSEMBLY __ */
>
> diff --git a/arch/arm64/kernel/module.c b/arch/arm64/kernel/module.c
> index 40148d2725ce..5d6d228c6156 100644
> --- a/arch/arm64/kernel/module.c
> +++ b/arch/arm64/kernel/module.c
> @@ -484,10 +484,12 @@ int module_finalize(const Elf_Ehdr *hdr,
> if (scs_is_dynamic()) {
> s = find_section(hdr, sechdrs, ".init.eh_frame");
> if (s) {
> - ret = __pi_scs_patch((void *)s->sh_addr, s->sh_size);
> - if (ret)
> + ret = __pi_scs_patch((void *)s->sh_addr, s->sh_size, true);
> + if (ret) {
> pr_err("module %s: error occurred during dynamic SCS patching (%d)\n",
> me->name, ret);
> + return -ENOEXEC;
> + }
> }
> }
>
> diff --git a/arch/arm64/kernel/pi/map_kernel.c b/arch/arm64/kernel/pi/map_kernel.c
> index 0f4bd7771859..7187eda9e8a5 100644
> --- a/arch/arm64/kernel/pi/map_kernel.c
> +++ b/arch/arm64/kernel/pi/map_kernel.c
> @@ -98,7 +98,7 @@ static void __init map_kernel(u64 kaslr_offset, u64 va_offset, int root_level)
>
> if (enable_scs) {
> scs_patch(__eh_frame_start + va_offset,
> - __eh_frame_end - __eh_frame_start);
> + __eh_frame_end - __eh_frame_start, false);
> asm("ic ialluis");
>
> dynamic_scs_is_enabled = true;
> diff --git a/arch/arm64/kernel/pi/patch-scs.c b/arch/arm64/kernel/pi/patch-scs.c
> index 55d0cd64ef71..78266fb1fa61 100644
> --- a/arch/arm64/kernel/pi/patch-scs.c
> +++ b/arch/arm64/kernel/pi/patch-scs.c
> @@ -225,7 +225,7 @@ static int scs_handle_fde_frame(const struct eh_frame *frame,
> return 0;
> }
>
> -int scs_patch(const u8 eh_frame[], int size)
> +int scs_patch(const u8 eh_frame[], int size, bool is_module)
> {
> int code_alignment_factor = 1;
> bool fde_use_sdata8 = false;
> @@ -276,12 +276,19 @@ int scs_patch(const u8 eh_frame[], int size)
> return EDYNSCS_INVALID_CIE_SDATA_SIZE;
> }
> } else {
> + /*
> + * For loadable module instead of running a dry run try
> + * to patch scs instruction in place and trigger error
> + * if failed, to prevent module loading.
> + */
Move this comment to the module loader, and explain why the two pass
approach is not needed in this case.
> ret = scs_handle_fde_frame(frame, code_alignment_factor,
> - fde_use_sdata8, true);
> + fde_use_sdata8, !is_module);
> if (ret)
> return ret;
> - scs_handle_fde_frame(frame, code_alignment_factor,
> - fde_use_sdata8, false);
> +
> + if (!is_module)
> + scs_handle_fde_frame(frame, code_alignment_factor,
> + fde_use_sdata8, false);
> }
>
> p += sizeof(frame->size) + frame->size;
> diff --git a/arch/arm64/kernel/pi/pi.h b/arch/arm64/kernel/pi/pi.h
> index 46cafee7829f..4ccbba24fadc 100644
> --- a/arch/arm64/kernel/pi/pi.h
> +++ b/arch/arm64/kernel/pi/pi.h
> @@ -27,7 +27,7 @@ extern pgd_t init_pg_dir[], init_pg_end[];
> void init_feature_override(u64 boot_status, const void *fdt, int chosen);
> u64 kaslr_early_init(void *fdt, int chosen);
> void relocate_kernel(u64 offset);
> -int scs_patch(const u8 eh_frame[], int size);
> +int scs_patch(const u8 eh_frame[], int size, bool is_module);
>
> void map_range(u64 *pgd, u64 start, u64 end, u64 pa, pgprot_t prot,
> int level, pte_t *tbl, bool may_use_cont, u64 va_offset);
> --
> 2.51.0.534.gc79095c0ca-goog
>
Hi,
On Fri, Sep 19, 2025 at 03:59:21PM +0200, Ard Biesheuvel wrote:
>Hi,
>
>On Fri, 19 Sept 2025 at 14:23, Adrian Barnaś <abarnas@google.com> wrote:
>>
>> Disallow a module to load if SCS dynamic patching fails for its code. For
>> module loading, instead of running a dry-run to check for patching errors,
>> try to run patching in the first run and propagate any errors so module
>> loading will fail.
>>
>> Signed-off-by: Adrian Barnaś <abarnas@google.com>
>> ---
>> arch/arm64/include/asm/scs.h | 2 +-
>> arch/arm64/kernel/module.c | 6 ++++--
>> arch/arm64/kernel/pi/map_kernel.c | 2 +-
>> arch/arm64/kernel/pi/patch-scs.c | 15 +++++++++++----
>> arch/arm64/kernel/pi/pi.h | 2 +-
>> 5 files changed, 18 insertions(+), 9 deletions(-)
>>
>> diff --git a/arch/arm64/include/asm/scs.h b/arch/arm64/include/asm/scs.h
>> index a76f9b387a26..ffcfcda87f10 100644
>> --- a/arch/arm64/include/asm/scs.h
>> +++ b/arch/arm64/include/asm/scs.h
>> @@ -53,7 +53,7 @@ enum {
>> EDYNSCS_INVALID_CFA_OPCODE = 4,
>> };
>>
>> -int __pi_scs_patch(const u8 eh_frame[], int size);
>> +int __pi_scs_patch(const u8 eh_frame[], int size, bool is_module);
>>
>
>Calling the parameter 'is_module' puts the policy in the SCS patching
>code, which now has to reason about how patching a module differs from
>patching other code.
Agreed.
>So I'd prefer to call this 'two_pass' or 'dry_run' or whatever, where
>setting it guarantees that when an error is returned, no function will
>be left in an inconsistent state.
Maybe `bool skip_dry_run`?
>
>> #endif /* __ASSEMBLY __ */
>>
>> diff --git a/arch/arm64/kernel/module.c b/arch/arm64/kernel/module.c
>> index 40148d2725ce..5d6d228c6156 100644
>> --- a/arch/arm64/kernel/module.c
>> +++ b/arch/arm64/kernel/module.c
>> @@ -484,10 +484,12 @@ int module_finalize(const Elf_Ehdr *hdr,
>> if (scs_is_dynamic()) {
>> s = find_section(hdr, sechdrs, ".init.eh_frame");
>> if (s) {
>> - ret = __pi_scs_patch((void *)s->sh_addr, s->sh_size);
>> - if (ret)
>> + ret = __pi_scs_patch((void *)s->sh_addr, s->sh_size, true);
>> + if (ret) {
>> pr_err("module %s: error occurred during dynamic SCS patching (%d)\n",
>> me->name, ret);
>> + return -ENOEXEC;
>> + }
>> }
>> }
>>
>> diff --git a/arch/arm64/kernel/pi/map_kernel.c b/arch/arm64/kernel/pi/map_kernel.c
>> index 0f4bd7771859..7187eda9e8a5 100644
>> --- a/arch/arm64/kernel/pi/map_kernel.c
>> +++ b/arch/arm64/kernel/pi/map_kernel.c
>> @@ -98,7 +98,7 @@ static void __init map_kernel(u64 kaslr_offset, u64 va_offset, int root_level)
>>
>> if (enable_scs) {
>> scs_patch(__eh_frame_start + va_offset,
>> - __eh_frame_end - __eh_frame_start);
>> + __eh_frame_end - __eh_frame_start, false);
>> asm("ic ialluis");
>>
>> dynamic_scs_is_enabled = true;
>> diff --git a/arch/arm64/kernel/pi/patch-scs.c b/arch/arm64/kernel/pi/patch-scs.c
>> index 55d0cd64ef71..78266fb1fa61 100644
>> --- a/arch/arm64/kernel/pi/patch-scs.c
>> +++ b/arch/arm64/kernel/pi/patch-scs.c
>> @@ -225,7 +225,7 @@ static int scs_handle_fde_frame(const struct eh_frame *frame,
>> return 0;
>> }
>>
>> -int scs_patch(const u8 eh_frame[], int size)
>> +int scs_patch(const u8 eh_frame[], int size, bool is_module)
>> {
>> int code_alignment_factor = 1;
>> bool fde_use_sdata8 = false;
>> @@ -276,12 +276,19 @@ int scs_patch(const u8 eh_frame[], int size)
>> return EDYNSCS_INVALID_CIE_SDATA_SIZE;
>> }
>> } else {
>> + /*
>> + * For loadable module instead of running a dry run try
>> + * to patch scs instruction in place and trigger error
>> + * if failed, to prevent module loading.
>> + */
>
>Move this comment to the module loader, and explain why the two pass
>approach is not needed in this case.
>
Will do.
>> ret = scs_handle_fde_frame(frame, code_alignment_factor,
>> - fde_use_sdata8, true);
>> + fde_use_sdata8, !is_module);
>> if (ret)
>> return ret;
>> - scs_handle_fde_frame(frame, code_alignment_factor,
>> - fde_use_sdata8, false);
>> +
>> + if (!is_module)
>> + scs_handle_fde_frame(frame, code_alignment_factor,
>> + fde_use_sdata8, false);
>> }
>>
>> p += sizeof(frame->size) + frame->size;
>> diff --git a/arch/arm64/kernel/pi/pi.h b/arch/arm64/kernel/pi/pi.h
>> index 46cafee7829f..4ccbba24fadc 100644
>> --- a/arch/arm64/kernel/pi/pi.h
>> +++ b/arch/arm64/kernel/pi/pi.h
>> @@ -27,7 +27,7 @@ extern pgd_t init_pg_dir[], init_pg_end[];
>> void init_feature_override(u64 boot_status, const void *fdt, int chosen);
>> u64 kaslr_early_init(void *fdt, int chosen);
>> void relocate_kernel(u64 offset);
>> -int scs_patch(const u8 eh_frame[], int size);
>> +int scs_patch(const u8 eh_frame[], int size, bool is_module);
>>
>> void map_range(u64 *pgd, u64 start, u64 end, u64 pa, pgprot_t prot,
>> int level, pte_t *tbl, bool may_use_cont, u64 va_offset);
>> --
>> 2.51.0.534.gc79095c0ca-goog
>>
Thank you for a review,
Adrian
© 2016 - 2025 Red Hat, Inc.