Sysctl for set_kernel_text_r[wo]

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

Sysctl for set_kernel_text_r[wo]

David Windsor
Hi,

I am looking into adding a sysctl that enables toggling of
set_kernel_text_rw, set_kernel_text_ro.  It appears that the only
caller of these methods is ftrace, which can rather easily be disabled
when these methods are unavailable.

I'm afraid I'm overlooking something major here.  It seems that such a
control would have been added much earlier if it was actually as
simple as adding a guard variable, mutable via a sysctl, allowing
access to this interface.

Thanks,
David Windsor

--
PGP: 6141 5FFD 11AE 9844 153E  F268 7C98 7268 6B19 6CC9

--
ubuntu-hardened mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/ubuntu-hardened
Reply | Threaded
Open this post in threaded view
|

Re: Sysctl for set_kernel_text_r[wo]

Kees Cook-5
Hi David,

On Sun, Sep 18, 2011 at 09:42:59PM -0400, David Windsor wrote:
> I am looking into adding a sysctl that enables toggling of
> set_kernel_text_rw, set_kernel_text_ro.  It appears that the only
> caller of these methods is ftrace, which can rather easily be disabled
> when these methods are unavailable.

It would be really nice to be able to wipe these functions out. I really
dislike that they are available as such perfect ROP targets.

> I'm afraid I'm overlooking something major here.  It seems that such a
> control would have been added much earlier if it was actually as
> simple as adding a guard variable, mutable via a sysctl, allowing
> access to this interface.

I haven't spent too much time looking into it, but I was under the
impression that the module loader used some of the underlying functions
too. Have you checked those code paths?

-Kees

--
Kees Cook

--
ubuntu-hardened mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/ubuntu-hardened
Reply | Threaded
Open this post in threaded view
|

Re: Sysctl for set_kernel_text_r[wo]

David Windsor
Hi Kees,

On Mon, Sep 19, 2011 at 1:12 AM, Kees Cook <[hidden email]> wrote:

> Hi David,
>
> On Sun, Sep 18, 2011 at 09:42:59PM -0400, David Windsor wrote:
>> I am looking into adding a sysctl that enables toggling of
>> set_kernel_text_rw, set_kernel_text_ro.  It appears that the only
>> caller of these methods is ftrace, which can rather easily be disabled
>> when these methods are unavailable.
>
> It would be really nice to be able to wipe these functions out. I really
> dislike that they are available as such perfect ROP targets.
>

I agree.  The only caller of set_kernel_text_r[wo] is ftrace, and only
dynamic ftrace at that.  When CONFIG_DYNAMIC_FTRACE is enabled, ftrace
modifies an in-core table of function pointers to mcount call sites.
AFAICS, when tracing filters are used, ftrace walks this table and
sets mcount call sites in functions not being traced to nops, so as to
minimize execution time penalties for non-traced code paths.
Similarly, in cases where CONFIG_DYNAMIC_FTRACE is set but tracing
isn't currently being performed, performance penalties are mitigated
by making all members of the mcount call site table a nop.

Both of these operations require kernel text to be writable, but since
ftrace is the only user of the set_kernel_text_r[wo] and
set_all_modules_text_r[wo], why don't we at least guard the definition
of these interfaces with #ifdefs for CONFIG_DYNAMIC_FTRACE?  At least
this way, users for whom security is a concern (who also will not be
using ftrace), won't have these interfaces available for ROP attacks.
Currently, calls to these functions are guarded by #ifdefs but their
definition isn't.

I've included a patch that guards the definition of
set_kernel_text_r[wo] and set_all_modules_text_r[wo] with #ifdefs for
CONFIG_DYNAMIC_FTRACE.  I have not tested this patch.

Thanks,
David

 arch/x86/include/asm/cacheflush.h |    2 ++
 arch/x86/mm/init_32.c             |    2 ++
 arch/x86/mm/init_64.c             |    2 ++
 include/linux/module.h            |    2 +-
 kernel/module.c                   |    2 ++
 5 files changed, 9 insertions(+), 1 deletions(-)

diff --git a/arch/x86/include/asm/cacheflush.h
b/arch/x86/include/asm/cacheflush.h
index 4e12668..ff4e7a0 100644
--- a/arch/x86/include/asm/cacheflush.h
+++ b/arch/x86/include/asm/cacheflush.h
@@ -146,12 +146,14 @@ void clflush_cache_range(void *addr, unsigned int size);
 void mark_rodata_ro(void);
 extern const int rodata_test_data;
 extern int kernel_set_to_readonly;
+#ifdef CONFIG_DYNAMIC_FTRACE
 void set_kernel_text_rw(void);
 void set_kernel_text_ro(void);
 #else
 static inline void set_kernel_text_rw(void) { }
 static inline void set_kernel_text_ro(void) { }
 #endif
+#endif

 #ifdef CONFIG_DEBUG_RODATA_TEST
 int rodata_test(void);
diff --git a/arch/x86/mm/init_32.c b/arch/x86/mm/init_32.c
index 29f7c6d..7e79965 100644
--- a/arch/x86/mm/init_32.c
+++ b/arch/x86/mm/init_32.c
@@ -888,6 +888,7 @@ EXPORT_SYMBOL_GPL(rodata_test_data);

 int kernel_set_to_readonly __read_mostly;

+#ifdef CONFIG_DYNAMIC_FTRACE
 void set_kernel_text_rw(void)
 {
  unsigned long start = PFN_ALIGN(_text);
@@ -915,6 +916,7 @@ void set_kernel_text_ro(void)

  set_pages_ro(virt_to_page(start), size >> PAGE_SHIFT);
 }
+#endif

 static void mark_nxdata_nx(void)
 {
diff --git a/arch/x86/mm/init_64.c b/arch/x86/mm/init_64.c
index bbaaa00..4a446d9 100644
--- a/arch/x86/mm/init_64.c
+++ b/arch/x86/mm/init_64.c
@@ -733,6 +733,7 @@ EXPORT_SYMBOL_GPL(rodata_test_data);

 int kernel_set_to_readonly;

+#ifdef CONFIG_DYNAMIC_FTRACE
 void set_kernel_text_rw(void)
 {
  unsigned long start = PFN_ALIGN(_text);
@@ -768,6 +769,7 @@ void set_kernel_text_ro(void)
  */
  set_memory_ro(start, (end - start) >> PAGE_SHIFT);
 }
+#endif

 void mark_rodata_ro(void)
 {
diff --git a/include/linux/module.h b/include/linux/module.h
index 1c30087..493d26c 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -721,7 +721,7 @@ extern int module_sysfs_initialized;

 #define __MODULE_STRING(x) __stringify(x)

-#ifdef CONFIG_DEBUG_SET_MODULE_RONX
+#ifdef CONFIG_DEBUG_SET_MODULE_RONX && CONFIG_DYNAMIC_FTRACE
 extern void set_all_modules_text_rw(void);
 extern void set_all_modules_text_ro(void);
 #else
diff --git a/kernel/module.c b/kernel/module.c
index 04379f92..edbedcb 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -1667,6 +1667,7 @@ static void unset_module_init_ro_nx(struct module *mod)
  set_memory_rw);
 }

+#ifdef CONFIG_DYNAMIC_FTRACE
 /* Iterate through all modules and set each module's text as RW */
 void set_all_modules_text_rw(void)
 {
@@ -1708,6 +1709,7 @@ void set_all_modules_text_ro(void)
  }
  mutex_unlock(&module_mutex);
 }
+#endif
 #else
 static inline void set_section_ro_nx(void *base, unsigned long
text_size, unsigned long ro_size, unsigned long total_size) { }
 static void unset_module_core_ro_nx(struct module *mod) { }

--
ubuntu-hardened mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/ubuntu-hardened