* [PATCH 1/3] module: Constify parameters of module_enforce_rwx_sections()
2024-12-23 9:37 [PATCH 0/3] module: Make .static_call_sites read-only after init Petr Pavlu
@ 2024-12-23 9:37 ` Petr Pavlu
2024-12-23 9:37 ` [PATCH 2/3] module: Add a separate function to mark sections as read-only after init Petr Pavlu
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: Petr Pavlu @ 2024-12-23 9:37 UTC (permalink / raw)
To: Luis Chamberlain, Petr Pavlu, Sami Tolvanen, Daniel Gomez,
Peter Zijlstra, Josh Poimboeuf, Jason Baron, Steven Rostedt,
Ard Biesheuvel
Cc: linux-modules, linux-kernel
Minor cleanup, this is a non-functional change.
Signed-off-by: Petr Pavlu <petr.pavlu@suse.com>
---
kernel/module/internal.h | 5 +++--
kernel/module/strict_rwx.c | 5 +++--
2 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/kernel/module/internal.h b/kernel/module/internal.h
index daef2be83902..7e78f6a8e85e 100644
--- a/kernel/module/internal.h
+++ b/kernel/module/internal.h
@@ -330,8 +330,9 @@ static inline struct module *mod_find(unsigned long addr, struct mod_tree_root *
int module_enable_rodata_ro(const struct module *mod, bool after_init);
int module_enable_data_nx(const struct module *mod);
int module_enable_text_rox(const struct module *mod);
-int module_enforce_rwx_sections(Elf_Ehdr *hdr, Elf_Shdr *sechdrs,
- char *secstrings, struct module *mod);
+int module_enforce_rwx_sections(const Elf_Ehdr *hdr, const Elf_Shdr *sechdrs,
+ const char *secstrings,
+ const struct module *mod);
#ifdef CONFIG_MODULE_SIG
int module_sig_check(struct load_info *info, int flags);
diff --git a/kernel/module/strict_rwx.c b/kernel/module/strict_rwx.c
index 239e5013359d..d1c43309bb25 100644
--- a/kernel/module/strict_rwx.c
+++ b/kernel/module/strict_rwx.c
@@ -81,8 +81,9 @@ int module_enable_data_nx(const struct module *mod)
return 0;
}
-int module_enforce_rwx_sections(Elf_Ehdr *hdr, Elf_Shdr *sechdrs,
- char *secstrings, struct module *mod)
+int module_enforce_rwx_sections(const Elf_Ehdr *hdr, const Elf_Shdr *sechdrs,
+ const char *secstrings,
+ const struct module *mod)
{
const unsigned long shf_wx = SHF_WRITE | SHF_EXECINSTR;
int i;
--
2.43.0
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH 2/3] module: Add a separate function to mark sections as read-only after init
2024-12-23 9:37 [PATCH 0/3] module: Make .static_call_sites read-only after init Petr Pavlu
2024-12-23 9:37 ` [PATCH 1/3] module: Constify parameters of module_enforce_rwx_sections() Petr Pavlu
@ 2024-12-23 9:37 ` Petr Pavlu
2024-12-23 9:37 ` [PATCH 3/3] module: Make .static_call_sites " Petr Pavlu
2025-01-03 14:06 ` [PATCH 0/3] " Christophe Leroy
3 siblings, 0 replies; 6+ messages in thread
From: Petr Pavlu @ 2024-12-23 9:37 UTC (permalink / raw)
To: Luis Chamberlain, Petr Pavlu, Sami Tolvanen, Daniel Gomez,
Peter Zijlstra, Josh Poimboeuf, Jason Baron, Steven Rostedt,
Ard Biesheuvel
Cc: linux-modules, linux-kernel
Move the logic to mark special sections as read-only after module
initialization into a separate function, along other related code in
strict_rwx.c. Use a table with names of such sections to make it easier to
add more.
Signed-off-by: Petr Pavlu <petr.pavlu@suse.com>
---
kernel/module/internal.h | 2 ++
kernel/module/main.c | 18 +++---------------
kernel/module/strict_rwx.c | 33 +++++++++++++++++++++++++++++++++
3 files changed, 38 insertions(+), 15 deletions(-)
diff --git a/kernel/module/internal.h b/kernel/module/internal.h
index 7e78f6a8e85e..c4c918d9b65e 100644
--- a/kernel/module/internal.h
+++ b/kernel/module/internal.h
@@ -333,6 +333,8 @@ int module_enable_text_rox(const struct module *mod);
int module_enforce_rwx_sections(const Elf_Ehdr *hdr, const Elf_Shdr *sechdrs,
const char *secstrings,
const struct module *mod);
+void module_mark_ro_after_init(const Elf_Ehdr *hdr, Elf_Shdr *sechdrs,
+ const char *secstrings);
#ifdef CONFIG_MODULE_SIG
int module_sig_check(struct load_info *info, int flags);
diff --git a/kernel/module/main.c b/kernel/module/main.c
index 5399c182b3cb..12ca6c6d7efd 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -2710,7 +2710,6 @@ core_param(module_blacklist, module_blacklist, charp, 0400);
static struct module *layout_and_allocate(struct load_info *info, int flags)
{
struct module *mod;
- unsigned int ndx;
int err;
/* Allow arches to frob section contents and sizes. */
@@ -2728,22 +2727,11 @@ static struct module *layout_and_allocate(struct load_info *info, int flags)
info->sechdrs[info->index.pcpu].sh_flags &= ~(unsigned long)SHF_ALLOC;
/*
- * Mark ro_after_init section with SHF_RO_AFTER_INIT so that
- * layout_sections() can put it in the right place.
+ * Mark relevant sections as SHF_RO_AFTER_INIT so layout_sections() can
+ * put them in the right place.
* Note: ro_after_init sections also have SHF_{WRITE,ALLOC} set.
*/
- ndx = find_sec(info, ".data..ro_after_init");
- if (ndx)
- info->sechdrs[ndx].sh_flags |= SHF_RO_AFTER_INIT;
- /*
- * Mark the __jump_table section as ro_after_init as well: these data
- * structures are never modified, with the exception of entries that
- * refer to code in the __init section, which are annotated as such
- * at module load time.
- */
- ndx = find_sec(info, "__jump_table");
- if (ndx)
- info->sechdrs[ndx].sh_flags |= SHF_RO_AFTER_INIT;
+ module_mark_ro_after_init(info->hdr, info->sechdrs, info->secstrings);
/*
* Determine total sizes, and put offsets in sh_entsize. For now
diff --git a/kernel/module/strict_rwx.c b/kernel/module/strict_rwx.c
index d1c43309bb25..598f89195336 100644
--- a/kernel/module/strict_rwx.c
+++ b/kernel/module/strict_rwx.c
@@ -101,3 +101,36 @@ int module_enforce_rwx_sections(const Elf_Ehdr *hdr, const Elf_Shdr *sechdrs,
return 0;
}
+
+static const char *const ro_after_init[] = {
+ /*
+ * Section .data..ro_after_init holds data explicitly annotated by
+ * __ro_after_init.
+ */
+ ".data..ro_after_init",
+
+ /*
+ * Section __jump_table holds data structures that are never modified,
+ * with the exception of entries that refer to code in the __init
+ * section, which are marked as such at module load time.
+ */
+ "__jump_table",
+};
+
+void module_mark_ro_after_init(const Elf_Ehdr *hdr, Elf_Shdr *sechdrs,
+ const char *secstrings)
+{
+ int i, j;
+
+ for (i = 1; i < hdr->e_shnum; i++) {
+ Elf_Shdr *shdr = &sechdrs[i];
+
+ for (j = 0; j < ARRAY_SIZE(ro_after_init); j++) {
+ if (strcmp(secstrings + shdr->sh_name,
+ ro_after_init[j]) == 0) {
+ shdr->sh_flags |= SHF_RO_AFTER_INIT;
+ break;
+ }
+ }
+ }
+}
--
2.43.0
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH 3/3] module: Make .static_call_sites read-only after init
2024-12-23 9:37 [PATCH 0/3] module: Make .static_call_sites read-only after init Petr Pavlu
2024-12-23 9:37 ` [PATCH 1/3] module: Constify parameters of module_enforce_rwx_sections() Petr Pavlu
2024-12-23 9:37 ` [PATCH 2/3] module: Add a separate function to mark sections as read-only after init Petr Pavlu
@ 2024-12-23 9:37 ` Petr Pavlu
2025-01-03 14:06 ` [PATCH 0/3] " Christophe Leroy
3 siblings, 0 replies; 6+ messages in thread
From: Petr Pavlu @ 2024-12-23 9:37 UTC (permalink / raw)
To: Luis Chamberlain, Petr Pavlu, Sami Tolvanen, Daniel Gomez,
Peter Zijlstra, Josh Poimboeuf, Jason Baron, Steven Rostedt,
Ard Biesheuvel
Cc: linux-modules, linux-kernel
Section .static_call_sites holds data structures that need to be sorted and
processed only at module load time. This initial processing happens in
static_call_add_module(), which is invoked as a callback to the
MODULE_STATE_COMING notification from prepare_coming_module().
The section is never modified afterwards. Make it therefore read-only after
module initialization to avoid any (non-)accidental modifications.
Signed-off-by: Petr Pavlu <petr.pavlu@suse.com>
---
kernel/module/strict_rwx.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/kernel/module/strict_rwx.c b/kernel/module/strict_rwx.c
index 598f89195336..4ae5facab569 100644
--- a/kernel/module/strict_rwx.c
+++ b/kernel/module/strict_rwx.c
@@ -115,6 +115,15 @@ static const char *const ro_after_init[] = {
* section, which are marked as such at module load time.
*/
"__jump_table",
+
+#ifdef CONFIG_HAVE_STATIC_CALL_INLINE
+ /*
+ * Section .static_call_sites holds data structures that need to be
+ * sorted and processed at module load time but are never modified
+ * afterwards.
+ */
+ ".static_call_sites",
+#endif
};
void module_mark_ro_after_init(const Elf_Ehdr *hdr, Elf_Shdr *sechdrs,
--
2.43.0
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 0/3] module: Make .static_call_sites read-only after init
2024-12-23 9:37 [PATCH 0/3] module: Make .static_call_sites read-only after init Petr Pavlu
` (2 preceding siblings ...)
2024-12-23 9:37 ` [PATCH 3/3] module: Make .static_call_sites " Petr Pavlu
@ 2025-01-03 14:06 ` Christophe Leroy
2025-01-03 16:24 ` Petr Pavlu
3 siblings, 1 reply; 6+ messages in thread
From: Christophe Leroy @ 2025-01-03 14:06 UTC (permalink / raw)
To: Petr Pavlu, Luis Chamberlain, Sami Tolvanen, Daniel Gomez,
Peter Zijlstra, Josh Poimboeuf, Jason Baron, Steven Rostedt,
Ard Biesheuvel
Cc: linux-modules, linux-kernel
Hi Petr,
Le 23/12/2024 à 10:37, Petr Pavlu a écrit :
> Section .static_call_sites holds data structures that need to be sorted and
> processed only at module load time. The section is never modified afterwards.
> Make it therefore read-only after module initialization to avoid any
> (non-)accidental modifications.
>
> Petr Pavlu (3):
> module: Constify parameters of module_enforce_rwx_sections()
> module: Add a separate function to mark sections as read-only after
> init
> module: Make .static_call_sites read-only after init
>
> kernel/module/internal.h | 7 ++++--
> kernel/module/main.c | 18 +++------------
> kernel/module/strict_rwx.c | 47 ++++++++++++++++++++++++++++++++++++--
> 3 files changed, 53 insertions(+), 19 deletions(-)
>
>
We have a problem at the moment with ro_after_init sections, isn't it
better to fix it before adding new stuff to ro_after_init ?
This series conflicts with my series which aims at fixing up
ro_after_init handling in modules, see
https://patchwork.kernel.org/project/linux-modules/cover/cover.1733427536.git.christophe.leroy@csgroup.eu/
I was expecting my series to land in modules-next, do you or Luis plan
to take it anytime soon ?
Christophe
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 0/3] module: Make .static_call_sites read-only after init
2025-01-03 14:06 ` [PATCH 0/3] " Christophe Leroy
@ 2025-01-03 16:24 ` Petr Pavlu
0 siblings, 0 replies; 6+ messages in thread
From: Petr Pavlu @ 2025-01-03 16:24 UTC (permalink / raw)
To: Christophe Leroy
Cc: Luis Chamberlain, Sami Tolvanen, Daniel Gomez, Peter Zijlstra,
Josh Poimboeuf, Jason Baron, Steven Rostedt, Ard Biesheuvel,
linux-modules, linux-kernel
On 1/3/25 15:06, Christophe Leroy wrote:
> Le 23/12/2024 à 10:37, Petr Pavlu a écrit :
>> Section .static_call_sites holds data structures that need to be sorted and
>> processed only at module load time. The section is never modified afterwards.
>> Make it therefore read-only after module initialization to avoid any
>> (non-)accidental modifications.
>>
>> Petr Pavlu (3):
>> module: Constify parameters of module_enforce_rwx_sections()
>> module: Add a separate function to mark sections as read-only after
>> init
>> module: Make .static_call_sites read-only after init
>>
>> kernel/module/internal.h | 7 ++++--
>> kernel/module/main.c | 18 +++------------
>> kernel/module/strict_rwx.c | 47 ++++++++++++++++++++++++++++++++++++--
>> 3 files changed, 53 insertions(+), 19 deletions(-)
>>
>>
>
> We have a problem at the moment with ro_after_init sections, isn't it
> better to fix it before adding new stuff to ro_after_init ?
>
> This series conflicts with my series which aims at fixing up
> ro_after_init handling in modules, see
> https://patchwork.kernel.org/project/linux-modules/cover/cover.1733427536.git.christophe.leroy@csgroup.eu/
Sure, this is a fairly minor change to keep what the modules loader does
in sync with how the same .static_call_sites data in vmlinux is handled.
It can wait.
> I was expecting my series to land in modules-next, do you or Luis plan
> to take it anytime soon ?
Sorry for the lack of clarity. I've been waiting with applying that
series as there was still some discussion. I have now replied in its
thread [1].
[1] https://lore.kernel.org/linux-modules/f0e892c7-43cd-4310-9d60-1d6e839f5bb2@suse.com/
--
Thanks,
Petr
^ permalink raw reply [flat|nested] 6+ messages in thread