diff mbox series

[v3,1/3] module: Prepare for addition of new ro_after_init sections

Message ID 20190410195708.162185-1-joel@joelfernandes.org (mailing list archive)
State New, archived
Headers show
Series [v3,1/3] module: Prepare for addition of new ro_after_init sections | expand

Commit Message

Joel Fernandes April 10, 2019, 7:57 p.m. UTC 
For the purposes of hardening modules by adding sections to
ro_after_init sections, prepare for addition of new ro_after_init
entries which we do in future patches. Create a table to which new
entries could be added later. This makes it less error prone and reduce
code duplication.

Cc: paulmck@linux.vnet.ibm.com
Cc: rostedt@goodmis.org
Cc: mathieu.desnoyers@efficios.com
Cc: rcu@vger.kernel.org
Cc: kernel-hardening@lists.openwall.com
Cc: kernel-team@android.com
Suggested-by: keescook@chromium.org
Reviewed-by: keescook@chromium.org
Acked-by: rostedt@goodmis.org
Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>

---
 kernel/module.c | 41 +++++++++++++++++++++++------------------
 1 file changed, 23 insertions(+), 18 deletions(-)

Comments

Miroslav Benes April 11, 2019, 8:32 a.m. UTC | #1 
On Wed, 10 Apr 2019, Joel Fernandes (Google) wrote:

> For the purposes of hardening modules by adding sections to
> ro_after_init sections, prepare for addition of new ro_after_init
> entries which we do in future patches. Create a table to which new
> entries could be added later. This makes it less error prone and reduce
> code duplication.
> 
> Cc: paulmck@linux.vnet.ibm.com
> Cc: rostedt@goodmis.org
> Cc: mathieu.desnoyers@efficios.com
> Cc: rcu@vger.kernel.org
> Cc: kernel-hardening@lists.openwall.com
> Cc: kernel-team@android.com
> Suggested-by: keescook@chromium.org
> Reviewed-by: keescook@chromium.org
> Acked-by: rostedt@goodmis.org
> Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>

Reviewed-by: Miroslav Benes <mbenes@suse.cz>

M
Jessica Yu April 17, 2019, 1:29 p.m. UTC | #2 
+++ Joel Fernandes (Google) [10/04/19 15:57 -0400]:
>For the purposes of hardening modules by adding sections to
>ro_after_init sections, prepare for addition of new ro_after_init
>entries which we do in future patches. Create a table to which new
>entries could be added later. This makes it less error prone and reduce
>code duplication.
>
>Cc: paulmck@linux.vnet.ibm.com
>Cc: rostedt@goodmis.org
>Cc: mathieu.desnoyers@efficios.com
>Cc: rcu@vger.kernel.org
>Cc: kernel-hardening@lists.openwall.com
>Cc: kernel-team@android.com
>Suggested-by: keescook@chromium.org
>Reviewed-by: keescook@chromium.org
>Acked-by: rostedt@goodmis.org
>Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>

Hi Joel!

Thanks for the patchset! Apologies for the late reply, I've returned
from vacation this week and am catching up on the previous threads..

And if I'm all caught up now, it looks like the plan now is to drop
this patchset in favor of just marking the srcu pointer array const,
is that correct? That should put it in a rodata section (ie., a
section without SHF_WRITE) and the module loader will mark it RO
appropriately after relocations are applied, so it really doesn't have
to be a ro_after_init section then if I'm understanding correctly.

Jessica

>---
> kernel/module.c | 41 +++++++++++++++++++++++------------------
> 1 file changed, 23 insertions(+), 18 deletions(-)
>
>diff --git a/kernel/module.c b/kernel/module.c
>index 524da609c884..42e4e289d6c7 100644
>--- a/kernel/module.c
>+++ b/kernel/module.c
>@@ -3300,11 +3300,27 @@ static bool blacklisted(const char *module_name)
> }
> core_param(module_blacklist, module_blacklist, charp, 0400);
>
>+/*
>+ * Mark ro_after_init section with SHF_RO_AFTER_INIT so that
>+ * layout_sections() can put it in the right place.
>+ * Note: ro_after_init sections also have SHF_{WRITE,ALLOC} set.
>+ */
>+static const char * const ro_after_init_sections[] = {
>+	".data..ro_after_init",
>+
>+	/*
>+	 * __jump_table 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.
>+	 */
>+	"__jump_table",
>+};
>+
> static struct module *layout_and_allocate(struct load_info *info, int flags)
> {
> 	struct module *mod;
> 	unsigned int ndx;
>-	int err;
>+	int err, i;
>
> 	err = check_modinfo(info->mod, info, flags);
> 	if (err)
>@@ -3319,23 +3335,12 @@ static struct module *layout_and_allocate(struct load_info *info, int flags)
> 	/* We will do a special allocation for per-cpu sections later. */
> 	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.
>-	 * 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;
>+	/* Set sh_flags for read-only after init sections */
>+	for (i = 0; i < ARRAY_SIZE(ro_after_init_sections); i++) {
>+		ndx = find_sec(info, ro_after_init_sections[i]);
>+		if (ndx)
>+			info->sechdrs[ndx].sh_flags |= SHF_RO_AFTER_INIT;
>+	}
>
> 	/* Determine total sizes, and put offsets in sh_entsize.  For now
> 	   this is done generically; there doesn't appear to be any
>-- 
>2.21.0.392.gf8f6787159e-goog
>
diff mbox series

Patch

diff --git a/kernel/module.c b/kernel/module.c
index 524da609c884..42e4e289d6c7 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -3300,11 +3300,27 @@  static bool blacklisted(const char *module_name)
 }
 core_param(module_blacklist, module_blacklist, charp, 0400);
 
+/*
+ * Mark ro_after_init section with SHF_RO_AFTER_INIT so that
+ * layout_sections() can put it in the right place.
+ * Note: ro_after_init sections also have SHF_{WRITE,ALLOC} set.
+ */
+static const char * const ro_after_init_sections[] = {
+	".data..ro_after_init",
+
+	/*
+	 * __jump_table 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.
+	 */
+	"__jump_table",
+};
+
 static struct module *layout_and_allocate(struct load_info *info, int flags)
 {
 	struct module *mod;
 	unsigned int ndx;
-	int err;
+	int err, i;
 
 	err = check_modinfo(info->mod, info, flags);
 	if (err)
@@ -3319,23 +3335,12 @@  static struct module *layout_and_allocate(struct load_info *info, int flags)
 	/* We will do a special allocation for per-cpu sections later. */
 	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.
-	 * 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;
+	/* Set sh_flags for read-only after init sections */
+	for (i = 0; i < ARRAY_SIZE(ro_after_init_sections); i++) {
+		ndx = find_sec(info, ro_after_init_sections[i]);
+		if (ndx)
+			info->sechdrs[ndx].sh_flags |= SHF_RO_AFTER_INIT;
+	}
 
 	/* Determine total sizes, and put offsets in sh_entsize.  For now
 	   this is done generically; there doesn't appear to be any