This patch is a logical extension of the protection provided by
CONFIG_DEBUG_RODATA to LKMs. The protection is provided by splitting
module_core and module_init into three logical parts each and setting
appropriate page access permissions for each individual section:
1. Code: RO+X
2. RO data: RO+NX
3. RW data: RW+NX
In order to achieve proper protection, layout_sections() have been
modified to align each of the three parts mentioned above onto page
boundary. Next, the corresponding page access permissions are set
right before successful exit from load_module(). Further,
module_free() have been modified to set module_core or module_init as
RW+NX right before calling vfree(). Functionality of this patch is
enabled only when CONFIG_DEBUG_RODATA defined at compile time.
The patch have been developed for Linux 2.6.30 i386.
If there are any issues with the patch formatting and/or kernel
version, please let me know and I will gladly correct the issues.
Thank you.
Signed-off-by: Siarhei Liakh <[email protected]>
---
diff --git a/arch/x86/kernel/module_32.c b/arch/x86/kernel/module_32.c
index 0edd819..8a72ef8 100644
--- a/arch/x86/kernel/module_32.c
+++ b/arch/x86/kernel/module_32.c
@@ -22,6 +22,7 @@
#include <linux/string.h>
#include <linux/kernel.h>
#include <linux/bug.h>
+#include <asm/cacheflush.h>
#if 0
#define DEBUGP printk
@@ -29,6 +30,14 @@
#define DEBUGP(fmt...)
#endif
+#ifdef CONFIG_DEBUG_RODATA
+/* Given BASE and SIZE this macro calculates the number of pages this
+ * memory regions occupies */
+#define NUMBER_OF_PAGES(BASE, SIZE) ( \
+ ( ( (unsigned long) BASE + (unsigned long) SIZE ) >> PAGE_SHIFT ) - \
+ ( ( (unsigned long) BASE ) >> PAGE_SHIFT) + ( (SIZE>0) ? 1:0 ) )
+#endif
+
void *module_alloc(unsigned long size)
{
if (size == 0)
@@ -40,6 +49,26 @@ void *module_alloc(unsigned long size)
/* Free memory returned from module_alloc */
void module_free(struct module *mod, void *module_region)
{
+#ifdef CONFIG_DEBUG_RODATA
+ unsigned long total_pages;
+
+ if(mod->module_core == module_region){
+ /* Set core as NX+RW */
+ total_pages = NUMBER_OF_PAGES( mod->module_core, mod->core_size );
+ DEBUGP(KERN_WARNING "RELEASING MODULE CORE: 0x%lx %lu\n",
+ (unsigned long)mod->module_core, total_pages);
+ set_memory_nx( (unsigned long)mod->module_core, total_pages);
+ set_memory_rw( (unsigned long)mod->module_core, total_pages);
+
+ }else if(mod->module_init == module_region){
+ /* Set init as NX+RW */
+ total_pages = NUMBER_OF_PAGES( mod->module_init, mod->init_size );
+ DEBUGP(KERN_WARNING "RELEASING MODULE INIT: 0x%lx %lu\n",
+ (unsigned long)mod->module_init, total_pages);
+ set_memory_nx( (unsigned long)mod->module_init, total_pages);
+ set_memory_rw( (unsigned long)mod->module_init, total_pages);
+ }
+#endif
vfree(module_region);
/* FIXME: If module_region == mod->init_region, trim exception
table entries. */
diff --git a/include/linux/module.h b/include/linux/module.h
index 627ac08..75e7428 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -293,6 +293,11 @@ struct module
/* The size of the executable code in each section. */
unsigned int init_text_size, core_text_size;
+#ifdef CONFIG_DEBUG_RODATA
+ /* Size of RO sections of the module (text+rodata) */
+ unsigned int init_ro_size, core_ro_size;
+#endif
+
/* Arch-specific module values */
struct mod_arch_specific arch;
diff --git a/kernel/module.c b/kernel/module.c
index e797812..4afcb06 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -63,6 +63,14 @@
#define ARCH_SHF_SMALL 0
#endif
+#ifdef CONFIG_DEBUG_RODATA
+/* Given BASE and SIZE this macro calculates the number of pages this
+ * memory regions occupies */
+#define NUMBER_OF_PAGES(BASE, SIZE) ( \
+ ( ( (unsigned long) BASE + (unsigned long) SIZE ) >> PAGE_SHIFT ) - \
+ ( ( (unsigned long) BASE ) >> PAGE_SHIFT) + ( (SIZE>0) ? 1:0 ) )
+#endif
+
/* If this is set, the section belongs in the init part of the module */
#define INIT_OFFSET_MASK (1UL << (BITS_PER_LONG-1))
@@ -1661,6 +1669,10 @@ static void layout_sections(struct module *mod,
{ ARCH_SHF_SMALL | SHF_ALLOC, 0 }
};
unsigned int m, i;
+#ifdef CONFIG_DEBUG_RODATA
+ /* always align first section on page boundary */
+ int need_alignment = 1;
+#endif
for (i = 0; i < hdr->e_shnum; i++)
sechdrs[i].sh_entsize = ~0UL;
@@ -1675,13 +1687,39 @@ static void layout_sections(struct module *mod,
|| s->sh_entsize != ~0UL
|| strstarts(secstrings + s->sh_name, ".init"))
continue;
+#ifdef CONFIG_DEBUG_RODATA
+ if(need_alignment){
+ mod->core_size = ALIGN(mod->core_size, PAGE_SIZE);
+ need_alignment = 0;
+ }
+#endif
s->sh_entsize = get_offset(mod, &mod->core_size, s, i);
DEBUGP("\t%s\n", secstrings + s->sh_name);
}
- if (m == 0)
+
+ if (m == 0) {
+ /* module code size */
mod->core_text_size = mod->core_size;
+#ifdef CONFIG_DEBUG_RODATA
+ /* next section should start on new page */
+ need_alignment = 1;
+#endif
+ }
+
+#ifdef CONFIG_DEBUG_RODATA
+ if (m == 1) {
+ /* module RO size (text+rodata) */
+ mod->core_ro_size = mod->core_size;
+ /* next section should start on new page */
+ need_alignment = 1;
+ }
+#endif
}
+#ifdef CONFIG_DEBUG_RODATA
+ /* always align first section on page boundary */
+ need_alignment = 1;
+#endif
DEBUGP("Init section allocation order:\n");
for (m = 0; m < ARRAY_SIZE(masks); ++m) {
for (i = 0; i < hdr->e_shnum; ++i) {
@@ -1692,13 +1730,50 @@ static void layout_sections(struct module *mod,
|| s->sh_entsize != ~0UL
|| !strstarts(secstrings + s->sh_name, ".init"))
continue;
+#ifdef CONFIG_DEBUG_RODATA
+ if(need_alignment){
+ mod->init_size = ALIGN(mod->init_size, PAGE_SIZE);
+ need_alignment = 0;
+ }
+#endif
+
s->sh_entsize = (get_offset(mod, &mod->init_size, s, i)
| INIT_OFFSET_MASK);
DEBUGP("\t%s\n", secstrings + s->sh_name);
}
- if (m == 0)
+
+ if (m == 0) {
+ /* module code size */
mod->init_text_size = mod->init_size;
+#ifdef CONFIG_DEBUG_RODATA
+ /* next section should start on new page */
+ need_alignment = 1;
+#endif
+ }
+
+#ifdef CONFIG_DEBUG_RODATA
+ if (m == 1) {
+ // module RO size (text+rodata)
+ mod->init_ro_size = mod->init_size;
+ // next section should start on new page
+ need_alignment = 1;
+ }
+#endif
}
+#ifdef CONFIG_DEBUG_RODATA
+ DEBUGP( "core_text_size: %i\n"
+ "core_ro_size: %i\n"
+ "core_size: %i\n"
+ "init_text_size: %i\n"
+ "init_ro_size: %i\n"
+ "init_size: %i\n",
+ mod->core_text_size,
+ mod->core_ro_size,
+ mod->core_size,
+ mod->init_text_size,
+ mod->init_ro_size,
+ mod->init_size);
+#endif
}
static void set_license(struct module *mod, const char *license)
@@ -1898,6 +1973,12 @@ static noinline struct module *load_module(void
__user *umod,
void *percpu = NULL, *ptr = NULL; /* Stops spurious gcc warning */
unsigned long *mseg;
mm_segment_t old_fs;
+#ifdef CONFIG_DEBUG_RODATA
+ /* these variables are used to set RO and NX regions */
+ unsigned long total_pages;
+ unsigned long text_pages;
+ unsigned long ro_pages;
+#endif
DEBUGP("load_module: umod=%p, len=%lu, uargs=%p\n",
umod, len, uargs);
@@ -2291,6 +2372,73 @@ static noinline struct module *load_module(void
__user *umod,
/* Get rid of temporary copy */
vfree(hdr);
+#ifdef CONFIG_DEBUG_RODATA
+ /* In this section we calculate the number of pages for
+ * each part of core and init: code, ro data, rw data.
+ * Then we set code as RO+X, ro data as RO+NX, and rw data as RW+NX
+ * Siarhei Liakh
+ * Xuxian Jiang */
+ DEBUGP( "module_core: 0x%lx %lu\n"
+ "module_init: 0x%lx %lu\n",
+ (unsigned long)mod->module_core,
+ (unsigned long)mod->core_size,
+ (unsigned long)mod->module_init,
+ (unsigned long)mod->init_size);
+
+ /* calculate RO and NX regions for core */
+ total_pages = NUMBER_OF_PAGES( mod->module_core, mod->core_size);
+ text_pages = NUMBER_OF_PAGES( mod->module_core, mod->core_text_size);
+ ro_pages = NUMBER_OF_PAGES( mod->module_core, mod->core_ro_size);
+
+ DEBUGP( "CORE:\n text_pages: %lu\n"
+ " ro_pages: %lu\n total_pages: %lu\n",
+ text_pages, ro_pages,total_pages);
+
+ /* Set text and RO data as RO */
+ DEBUGP( " RO: 0x%lx %lu\n",
+ (unsigned long)mod->module_core, ro_pages);
+
+ if(ro_pages>0)
+ set_memory_ro( (unsigned long)mod->module_core, ro_pages);
+
+ /* Set all data as NX */
+ DEBUGP( " NX: 0x%lx %lu\n",
+ (unsigned long)mod->module_core +
+ (text_pages << PAGE_SHIFT) ,
+ total_pages - text_pages );
+
+ if(total_pages > text_pages)
+ set_memory_nx( (unsigned long)mod->module_core +
+ (text_pages << PAGE_SHIFT) ,
+ total_pages - text_pages );
+
+ /* calculate RO and NX regions for init */
+ total_pages = NUMBER_OF_PAGES( mod->module_init, mod->init_size);
+ text_pages = NUMBER_OF_PAGES( mod->module_init, mod->init_text_size);
+ ro_pages = NUMBER_OF_PAGES( mod->module_init, mod->init_ro_size);
+
+ DEBUGP( "INIT:\n text_pages: %lu\n"
+ " ro_pages: %lu\n total_pages: %lu\n",
+ text_pages, ro_pages, total_pages);
+
+ /* Set text and RO data as RO */
+ DEBUGP( " RO: 0x%lx %lu\n",
+ (unsigned long)mod->module_init, ro_pages);
+
+ if(ro_pages>0)
+ set_memory_ro( (unsigned long)mod->module_init, ro_pages);
+
+ /* Set all data as NX */
+ DEBUGP( " NX: 0x%lx %lu\n",
+ (unsigned long)mod->module_init + (text_pages << PAGE_SHIFT),
+ total_pages - text_pages );
+
+ if(total_pages > text_pages)
+ set_memory_nx( (unsigned long)mod->module_init +
+ (text_pages << PAGE_SHIFT) ,
+ total_pages - text_pages );
+#endif
+
/* Done! */
return mod;
On Thu, 18 Jun 2009, Siarhei Liakh wrote:
> This patch is a logical extension of the protection provided by
> CONFIG_DEBUG_RODATA to LKMs. The protection is provided by splitting
> module_core and module_init into three logical parts each and setting
> appropriate page access permissions for each individual section:
>
> 1. Code: RO+X
> 2. RO data: RO+NX
> 3. RW data: RW+NX
>
> In order to achieve proper protection, layout_sections() have been
> modified to align each of the three parts mentioned above onto page
> boundary. Next, the corresponding page access permissions are set
> right before successful exit from load_module(). Further,
> module_free() have been modified to set module_core or module_init as
> RW+NX right before calling vfree(). Functionality of this patch is
> enabled only when CONFIG_DEBUG_RODATA defined at compile time.
>
This looks potentially useful to me, but I'm not an x86 expert (several
now added to Cc:).
> The patch have been developed for Linux 2.6.30 i386.
>
> If there are any issues with the patch formatting and/or kernel
> version, please let me know and I will gladly correct the issues.
> Thank you.
>
> Signed-off-by: Siarhei Liakh <[email protected]>
> ---
>
> diff --git a/arch/x86/kernel/module_32.c b/arch/x86/kernel/module_32.c
> index 0edd819..8a72ef8 100644
> --- a/arch/x86/kernel/module_32.c
> +++ b/arch/x86/kernel/module_32.c
> @@ -22,6 +22,7 @@
> #include <linux/string.h>
> #include <linux/kernel.h>
> #include <linux/bug.h>
> +#include <asm/cacheflush.h>
>
> #if 0
> #define DEBUGP printk
> @@ -29,6 +30,14 @@
> #define DEBUGP(fmt...)
> #endif
>
> +#ifdef CONFIG_DEBUG_RODATA
> +/* Given BASE and SIZE this macro calculates the number of pages this
> + * memory regions occupies */
> +#define NUMBER_OF_PAGES(BASE, SIZE) ( \
> + ( ( (unsigned long) BASE + (unsigned long) SIZE ) >> PAGE_SHIFT ) - \
> + ( ( (unsigned long) BASE ) >> PAGE_SHIFT) + ( (SIZE>0) ? 1:0 ) )
> +#endif
> +
> void *module_alloc(unsigned long size)
> {
> if (size == 0)
> @@ -40,6 +49,26 @@ void *module_alloc(unsigned long size)
> /* Free memory returned from module_alloc */
> void module_free(struct module *mod, void *module_region)
> {
> +#ifdef CONFIG_DEBUG_RODATA
> + unsigned long total_pages;
> +
> + if(mod->module_core == module_region){
> + /* Set core as NX+RW */
> + total_pages = NUMBER_OF_PAGES( mod->module_core, mod->core_size );
> + DEBUGP(KERN_WARNING "RELEASING MODULE CORE: 0x%lx %lu\n",
> + (unsigned long)mod->module_core, total_pages);
> + set_memory_nx( (unsigned long)mod->module_core, total_pages);
> + set_memory_rw( (unsigned long)mod->module_core, total_pages);
> +
> + }else if(mod->module_init == module_region){
> + /* Set init as NX+RW */
> + total_pages = NUMBER_OF_PAGES( mod->module_init, mod->init_size );
> + DEBUGP(KERN_WARNING "RELEASING MODULE INIT: 0x%lx %lu\n",
> + (unsigned long)mod->module_init, total_pages);
> + set_memory_nx( (unsigned long)mod->module_init, total_pages);
> + set_memory_rw( (unsigned long)mod->module_init, total_pages);
> + }
> +#endif
> vfree(module_region);
> /* FIXME: If module_region == mod->init_region, trim exception
> table entries. */
> diff --git a/include/linux/module.h b/include/linux/module.h
> index 627ac08..75e7428 100644
> --- a/include/linux/module.h
> +++ b/include/linux/module.h
> @@ -293,6 +293,11 @@ struct module
> /* The size of the executable code in each section. */
> unsigned int init_text_size, core_text_size;
>
> +#ifdef CONFIG_DEBUG_RODATA
> + /* Size of RO sections of the module (text+rodata) */
> + unsigned int init_ro_size, core_ro_size;
> +#endif
> +
> /* Arch-specific module values */
> struct mod_arch_specific arch;
>
> diff --git a/kernel/module.c b/kernel/module.c
> index e797812..4afcb06 100644
> --- a/kernel/module.c
> +++ b/kernel/module.c
> @@ -63,6 +63,14 @@
> #define ARCH_SHF_SMALL 0
> #endif
>
> +#ifdef CONFIG_DEBUG_RODATA
> +/* Given BASE and SIZE this macro calculates the number of pages this
> + * memory regions occupies */
> +#define NUMBER_OF_PAGES(BASE, SIZE) ( \
> + ( ( (unsigned long) BASE + (unsigned long) SIZE ) >> PAGE_SHIFT ) - \
> + ( ( (unsigned long) BASE ) >> PAGE_SHIFT) + ( (SIZE>0) ? 1:0 ) )
> +#endif
> +
> /* If this is set, the section belongs in the init part of the module */
> #define INIT_OFFSET_MASK (1UL << (BITS_PER_LONG-1))
>
> @@ -1661,6 +1669,10 @@ static void layout_sections(struct module *mod,
> { ARCH_SHF_SMALL | SHF_ALLOC, 0 }
> };
> unsigned int m, i;
> +#ifdef CONFIG_DEBUG_RODATA
> + /* always align first section on page boundary */
> + int need_alignment = 1;
> +#endif
>
> for (i = 0; i < hdr->e_shnum; i++)
> sechdrs[i].sh_entsize = ~0UL;
> @@ -1675,13 +1687,39 @@ static void layout_sections(struct module *mod,
> || s->sh_entsize != ~0UL
> || strstarts(secstrings + s->sh_name, ".init"))
> continue;
> +#ifdef CONFIG_DEBUG_RODATA
> + if(need_alignment){
> + mod->core_size = ALIGN(mod->core_size, PAGE_SIZE);
> + need_alignment = 0;
> + }
> +#endif
> s->sh_entsize = get_offset(mod, &mod->core_size, s, i);
> DEBUGP("\t%s\n", secstrings + s->sh_name);
> }
> - if (m == 0)
> +
> + if (m == 0) {
> + /* module code size */
> mod->core_text_size = mod->core_size;
> +#ifdef CONFIG_DEBUG_RODATA
> + /* next section should start on new page */
> + need_alignment = 1;
> +#endif
> + }
> +
> +#ifdef CONFIG_DEBUG_RODATA
> + if (m == 1) {
> + /* module RO size (text+rodata) */
> + mod->core_ro_size = mod->core_size;
> + /* next section should start on new page */
> + need_alignment = 1;
> + }
> +#endif
> }
>
> +#ifdef CONFIG_DEBUG_RODATA
> + /* always align first section on page boundary */
> + need_alignment = 1;
> +#endif
> DEBUGP("Init section allocation order:\n");
> for (m = 0; m < ARRAY_SIZE(masks); ++m) {
> for (i = 0; i < hdr->e_shnum; ++i) {
> @@ -1692,13 +1730,50 @@ static void layout_sections(struct module *mod,
> || s->sh_entsize != ~0UL
> || !strstarts(secstrings + s->sh_name, ".init"))
> continue;
> +#ifdef CONFIG_DEBUG_RODATA
> + if(need_alignment){
> + mod->init_size = ALIGN(mod->init_size, PAGE_SIZE);
> + need_alignment = 0;
> + }
> +#endif
> +
> s->sh_entsize = (get_offset(mod, &mod->init_size, s, i)
> | INIT_OFFSET_MASK);
> DEBUGP("\t%s\n", secstrings + s->sh_name);
> }
> - if (m == 0)
> +
> + if (m == 0) {
> + /* module code size */
> mod->init_text_size = mod->init_size;
> +#ifdef CONFIG_DEBUG_RODATA
> + /* next section should start on new page */
> + need_alignment = 1;
> +#endif
> + }
> +
> +#ifdef CONFIG_DEBUG_RODATA
> + if (m == 1) {
> + // module RO size (text+rodata)
> + mod->init_ro_size = mod->init_size;
> + // next section should start on new page
> + need_alignment = 1;
> + }
> +#endif
> }
> +#ifdef CONFIG_DEBUG_RODATA
> + DEBUGP( "core_text_size: %i\n"
> + "core_ro_size: %i\n"
> + "core_size: %i\n"
> + "init_text_size: %i\n"
> + "init_ro_size: %i\n"
> + "init_size: %i\n",
> + mod->core_text_size,
> + mod->core_ro_size,
> + mod->core_size,
> + mod->init_text_size,
> + mod->init_ro_size,
> + mod->init_size);
> +#endif
> }
>
> static void set_license(struct module *mod, const char *license)
> @@ -1898,6 +1973,12 @@ static noinline struct module *load_module(void
> __user *umod,
> void *percpu = NULL, *ptr = NULL; /* Stops spurious gcc warning */
> unsigned long *mseg;
> mm_segment_t old_fs;
> +#ifdef CONFIG_DEBUG_RODATA
> + /* these variables are used to set RO and NX regions */
> + unsigned long total_pages;
> + unsigned long text_pages;
> + unsigned long ro_pages;
> +#endif
>
> DEBUGP("load_module: umod=%p, len=%lu, uargs=%p\n",
> umod, len, uargs);
> @@ -2291,6 +2372,73 @@ static noinline struct module *load_module(void
> __user *umod,
> /* Get rid of temporary copy */
> vfree(hdr);
>
> +#ifdef CONFIG_DEBUG_RODATA
> + /* In this section we calculate the number of pages for
> + * each part of core and init: code, ro data, rw data.
> + * Then we set code as RO+X, ro data as RO+NX, and rw data as RW+NX
> + * Siarhei Liakh
> + * Xuxian Jiang */
> + DEBUGP( "module_core: 0x%lx %lu\n"
> + "module_init: 0x%lx %lu\n",
> + (unsigned long)mod->module_core,
> + (unsigned long)mod->core_size,
> + (unsigned long)mod->module_init,
> + (unsigned long)mod->init_size);
> +
> + /* calculate RO and NX regions for core */
> + total_pages = NUMBER_OF_PAGES( mod->module_core, mod->core_size);
> + text_pages = NUMBER_OF_PAGES( mod->module_core, mod->core_text_size);
> + ro_pages = NUMBER_OF_PAGES( mod->module_core, mod->core_ro_size);
> +
> + DEBUGP( "CORE:\n text_pages: %lu\n"
> + " ro_pages: %lu\n total_pages: %lu\n",
> + text_pages, ro_pages,total_pages);
> +
> + /* Set text and RO data as RO */
> + DEBUGP( " RO: 0x%lx %lu\n",
> + (unsigned long)mod->module_core, ro_pages);
> +
> + if(ro_pages>0)
> + set_memory_ro( (unsigned long)mod->module_core, ro_pages);
> +
> + /* Set all data as NX */
> + DEBUGP( " NX: 0x%lx %lu\n",
> + (unsigned long)mod->module_core +
> + (text_pages << PAGE_SHIFT) ,
> + total_pages - text_pages );
> +
> + if(total_pages > text_pages)
> + set_memory_nx( (unsigned long)mod->module_core +
> + (text_pages << PAGE_SHIFT) ,
> + total_pages - text_pages );
> +
> + /* calculate RO and NX regions for init */
> + total_pages = NUMBER_OF_PAGES( mod->module_init, mod->init_size);
> + text_pages = NUMBER_OF_PAGES( mod->module_init, mod->init_text_size);
> + ro_pages = NUMBER_OF_PAGES( mod->module_init, mod->init_ro_size);
> +
> + DEBUGP( "INIT:\n text_pages: %lu\n"
> + " ro_pages: %lu\n total_pages: %lu\n",
> + text_pages, ro_pages, total_pages);
> +
> + /* Set text and RO data as RO */
> + DEBUGP( " RO: 0x%lx %lu\n",
> + (unsigned long)mod->module_init, ro_pages);
> +
> + if(ro_pages>0)
> + set_memory_ro( (unsigned long)mod->module_init, ro_pages);
> +
> + /* Set all data as NX */
> + DEBUGP( " NX: 0x%lx %lu\n",
> + (unsigned long)mod->module_init + (text_pages << PAGE_SHIFT),
> + total_pages - text_pages );
> +
> + if(total_pages > text_pages)
> + set_memory_nx( (unsigned long)mod->module_init +
> + (text_pages << PAGE_SHIFT) ,
> + total_pages - text_pages );
> +#endif
> +
> /* Done! */
> return mod;
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>
--
James Morris
<[email protected]>
* James Morris <[email protected]> wrote:
> On Thu, 18 Jun 2009, Siarhei Liakh wrote:
>
> > This patch is a logical extension of the protection provided by
> > CONFIG_DEBUG_RODATA to LKMs. The protection is provided by splitting
> > module_core and module_init into three logical parts each and setting
> > appropriate page access permissions for each individual section:
> >
> > 1. Code: RO+X
> > 2. RO data: RO+NX
> > 3. RW data: RW+NX
> >
> > In order to achieve proper protection, layout_sections() have been
> > modified to align each of the three parts mentioned above onto page
> > boundary. Next, the corresponding page access permissions are set
> > right before successful exit from load_module(). Further,
> > module_free() have been modified to set module_core or module_init as
> > RW+NX right before calling vfree(). Functionality of this patch is
> > enabled only when CONFIG_DEBUG_RODATA defined at compile time.
> >
>
> This looks potentially useful to me, but I'm not an x86 expert
> (several now added to Cc:).
Pinged a few folks about this already. It looks useful, with the
main worry being:
1) the increase in effective module size (probably worth the price)
2) some uglies in the patch (fixable)
The main ugliness is the excessive use of #ifdefs - those should be
eliminated. Also, most scripts/checkpatch.pl warnings about this
patch should be taken seriously.
Ingo
On Tue, Jun 23, 2009 at 6:00 AM, Ingo Molnar<[email protected]> wrote:
>
> * James Morris <[email protected]> wrote:
>
>> On Thu, 18 Jun 2009, Siarhei Liakh wrote:
>>
>> > This patch is a logical extension of the protection provided by
>> > CONFIG_DEBUG_RODATA to LKMs. The protection is provided by splitting
>> > module_core and module_init into three logical parts each and setting
>> > appropriate page access permissions for each individual section:
>> >
>> > ? ?1. Code: RO+X
>> > ? ?2. RO data: RO+NX
>> > ? ?3. RW data: RW+NX
>> >
>> > In order to achieve proper protection, layout_sections() have been
>> > modified to align each of the three parts mentioned above onto page
>> > boundary. Next, the corresponding page access permissions are set
>> > right before successful exit from load_module(). Further,
>> > module_free() have been modified to set module_core or module_init as
>> > RW+NX right before calling vfree(). Functionality of this patch is
>> > enabled only when CONFIG_DEBUG_RODATA defined at compile time.
>> >
>>
>> This looks potentially useful to me, but I'm not an x86 expert
>> (several now added to Cc:).
>
> Pinged a few folks about this already. It looks useful, with the
> main worry being:
>
> ?1) the increase in effective module size (probably worth the price)
> ?2) some uglies in the patch (fixable)
>
> The main ugliness is the excessive use of #ifdefs - those should be
> eliminated. Also, most scripts/checkpatch.pl warnings about this
> patch should be taken seriously.
>
> ? ? ? ?Ingo
1: You are correct. This patch effectively sets the lower limit of 3
pages for a module (well, there are some modules that do not have
.text and/or .data, but we are not talking about these extremes). So,
for small (sub-one-page) modules this will create 300% overhead.
However, for large modules the overhead is not that significant.
2: Agree. Will fix.
The main reason the #ifdefs are there is to make this patch dependent
on CONFIG_DEBUG_RODATA, since not everyone may be willing to pay 300%
overhead for the modules they use. I guess I could do some code
re-factoring, but #ifdefs will still be there. Or did you really mean
to eliminate all of them, making this patch a permanent feature of the
kernel? Can you please point me into right direction on how to
eliminate the #ifdefs while allowing to exclude the patch at compile
time if necessary?
Thanks.