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: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Suggested-by: [email protected]
Reviewed-by: [email protected]
Acked-by: [email protected]
Signed-off-by: Joel Fernandes (Google) <[email protected]>
---
kernel/module.c | 41 +++++++++++++++++++++++------------------
1 file changed, 23 insertions(+), 18 deletions(-)
diff --git a/kernel/module.c b/kernel/module.c
index 524da609c884..1acddb93282a 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 char *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; ro_after_init_sections[i]; 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
On Wed, Apr 10, 2019 at 03:08:21PM -0400, 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: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Suggested-by: [email protected]
> Reviewed-by: [email protected]
> Acked-by: [email protected]
> Signed-off-by: Joel Fernandes (Google) <[email protected]>
>
> ---
> kernel/module.c | 41 +++++++++++++++++++++++------------------
> 1 file changed, 23 insertions(+), 18 deletions(-)
>
> diff --git a/kernel/module.c b/kernel/module.c
> index 524da609c884..1acddb93282a 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 char *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; ro_after_init_sections[i]; i++) {
Seems the fixup for this based on Kees suggestion of using NULL got squashed
into 2/3, so allow me to send a v3 to fix this ;-) Sorry! I am doing that
now.
The patches applied together are still code-correct thought.
thanks,
- Joel
Since commit title ("srcu: Allocate per-CPU data for DEFINE_SRCU() in
modules"), modules that call DEFINE_{STATIC,}SRCU will have a new array
of srcu_struct pointers which is used by srcu code to initialize and
clean up these structures.
There is no reason for this array of pointers to be writable, and can
cause security or other hidden bugs. Mark these are read-only after the
module init has completed.
Suggested-by: [email protected]
Suggested-by: [email protected]
Acked-by: [email protected]
Signed-off-by: Joel Fernandes (Google) <[email protected]>
---
kernel/module.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/kernel/module.c b/kernel/module.c
index 1acddb93282a..8b9631e789f0 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -3305,7 +3305,7 @@ core_param(module_blacklist, module_blacklist, charp, 0400);
* layout_sections() can put it in the right place.
* Note: ro_after_init sections also have SHF_{WRITE,ALLOC} set.
*/
-static char *ro_after_init_sections[] = {
+static const char * const ro_after_init_sections[] = {
".data..ro_after_init",
/*
@@ -3314,6 +3314,12 @@ static char *ro_after_init_sections[] = {
* annotated as such at module load time.
*/
"__jump_table",
+
+ /*
+ * Used for SRCU structures which need to be initialized/cleaned up
+ * by the SRCU notifiers
+ */
+ "___srcu_struct_ptrs",
};
static struct module *layout_and_allocate(struct load_info *info, int flags)
@@ -3336,7 +3342,7 @@ static struct module *layout_and_allocate(struct load_info *info, int flags)
info->sechdrs[info->index.pcpu].sh_flags &= ~(unsigned long)SHF_ALLOC;
/* Set sh_flags for read-only after init sections */
- for (i = 0; ro_after_init_sections[i]; i++) {
+ 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;
--
2.21.0.392.gf8f6787159e-goog
This series hardens the tracepoints in modules by making the array of
pointers referring to the tracepoints as read-only. This array is needed
during module unloading to verify that the tracepoint is quiescent.
There is no reason for the array to be to be writable after init, and
can cause security or other hidden bugs. Mark these as ro_after_init.
Suggested-by: [email protected]
Suggested-by: [email protected]
Suggested-by: [email protected]
Cc: [email protected]
Signed-off-by: Joel Fernandes (Google) <[email protected]>
---
kernel/module.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/kernel/module.c b/kernel/module.c
index 8b9631e789f0..be980aaa8804 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -3320,6 +3320,12 @@ static const char * const ro_after_init_sections[] = {
* by the SRCU notifiers
*/
"___srcu_struct_ptrs",
+
+ /*
+ * Array of tracepoint pointers used for checking if tracepoints are
+ * quiescent during unloading.
+ */
+ "__tracepoints_ptrs",
};
static struct module *layout_and_allocate(struct load_info *info, int flags)
--
2.21.0.392.gf8f6787159e-goog