Detect illicit cross-domain calls (i386/x86_64)
PATCH 7/7 Detect illicit cross-domain calls (i386/x86_64)
[This patch is a remake of
https://mail.gna.org/public/adeos-main/2006-09/msg00037.html]
Actually, this patch does more than the subject suggests. First it gets rid of ipipe_preempt_guard by fixing the remaining invocation of preempt_xxx over non-root domains. At least it appears so for i386 and x86_64 based on simple tests.
But - here is its main contribution - this patch also comes with means to detect remaining or to-be-introduced illicit calls. It introduces ipipe_check_context(border_ipd) that checks (CONFIG_IPIPE_DEBUG_CONTEXT) if the caller runs over the same or a lower-priority domain than border_ipd. If not, the kernel will bark loudly.
There are already three major instrumentation points for Linux included:
- preempt_xxx()
- might_resched()
- raw_local_irq_save/disable()
That /should/ catch almost all evil invocations of Linux services from non root-domains without hurting performance too much.
@everyone on other archs: please port and test for remaining ipipe_preempt_guard wreckages!
Jan
--- include/linux/ipipe_base.h | 6 ++++ include/linux/kernel.h | 8 ++++-- include/linux/preempt.h | 59 ++++++++++++++++++--------------------------- kernel/ipipe/Kconfig.debug | 9 ++++++ kernel/ipipe/core.c | 31 +++++++++++++++++++++++ 5 files changed, 76 insertions(+), 37 deletions(-)
include/linux/ipipe_base.h +6 -0
@@ -75,6 +75,12 @@ void __ipipe_unstall_root(void);void __ipipe_restore_root (unsigned long x); +#ifdef CONFIG_IPIPE_DEBUG_CONTEXT +void ipipe_check_context (struct ipipe_domain *border_ipd); +#else /* !CONFIG_IPIPE_DEBUG_CONTEXT */ +static inline void ipipe_check_context (struct ipipe_domain *border_ipd) { } +#endif /* !CONFIG_IPIPE_DEBUG_CONTEXT */ +#endif /* CONFIG_IPIPE */ #endif /* !__LINUX_IPIPE_BASE_H */
include/linux/kernel.h +6 -2
@@ -14,6 +14,7 @@#include <linux/compiler.h> #include <linux/bitops.h> #include <linux/log2.h> +#include <linux/ipipe_base.h> #include <asm/byteorder.h> #include <asm/bug.h>
@@ -72,9 +73,12 @@ struct user;*/ #ifdef CONFIG_PREEMPT_VOLUNTARY extern int cond_resched (void ); -# define might_resched () cond_resched() +# define might_resched () do { \ + ipipe_check_context(ipipe_root_domain); \ + cond_resched(); \ + } while (0)#else -# define might_resched () do { } while (0) +# define might_resched () ipipe_check_context(ipipe_root_domain)#endif #ifdef CONFIG_DEBUG_SPINLOCK_SLEEP
include/linux/preempt.h +24 -35
82 lines not on this page · raw message
kernel/ipipe/Kconfig.debug +9 -0
@@ -2,6 +2,15 @@ config IPIPE_DEBUG bool "I-pipe debugging" depends on IPIPE +config IPIPE_DEBUG_CONTEXT + bool "Check for illicit cross-domain calls" + depends on IPIPE_DEBUG + ---help--- + Enable this feature to arm checkpoints in the kernel that + verify the correct invocation context. On entry of critical + Linux services a warning is issued if the caller is not + running over the root domain. + config IPIPE_TRACE bool "Latency tracing" depends on IPIPE_DEBUG
kernel/ipipe/core.c +31 -0
@@ -33,6 +33,9 @@#include <linux/proc_fs.h> #include <linux/seq_file.h> #endif /* CONFIG_PROC_FS */ +#ifdef CONFIG_IPIPE_TRACE_MCOUNT +#include <linux/ipipe_trace.h> +#endif /* CONFIG_IPIPE_TRACE_MCOUNT */ static int __ipipe_ptd_key_count;
@@ -1383,6 +1386,34 @@ void __init ipipe_init_proc(void)#endif /* CONFIG_PROC_FS */ +#ifdef CONFIG_IPIPE_DEBUG_CONTEXT +void ipipe_check_context (struct ipipe_domain *border_ipd) +{ +static int check_hit; + +if (likely (ipipe_current_domain->priority <= border_ipd->priority) || + check_hit) +return ; + + check_hit =1 ; + +#ifdef CONFIG_IPIPE_TRACE_MCOUNT +ipipe_trace_panic_freeze (); +#endif /* CONFIG_IPIPE_TRACE_MCOUNT */ +ipipe_set_printk_sync (ipipe_current_domain); +printk (KERN_ERR "I-pipe: Detected illicit call from domain '%s'\n" +KERN_ERR " into a service reserved for domain '%s' and " +"below.\n" , + ipipe_current_domain->name, border_ipd->name); +show_stack (NULL ,NULL ); +#ifdef CONFIG_IPIPE_TRACE_MCOUNT +ipipe_trace_panic_dump (); +#endif /* CONFIG_IPIPE_TRACE_MCOUNT */ +} + +EXPORT_SYMBOL (ipipe_check_context); +#endif /* CONFIG_IPIPE_DEBUG_CONTEXT */ +EXPORT_SYMBOL (ipipe_virtualize_irq);EXPORT_SYMBOL (ipipe_control_irq);EXPORT_SYMBOL (ipipe_suspend_domain);
--- include/asm-i386/i387.h | 9 +++++---- include/asm-i386/irqflags.h | 7 +++++-- 2 files changed, 10 insertions(+), 6 deletions(-)
include/asm-i386/irqflags.h +5 -2
@@ -54,6 +54,7 @@ static inline void raw_local_irq_restorestatic inline void raw_local_irq_disable (void ) {#ifdef CONFIG_IPIPE +ipipe_check_context (ipipe_root_domain);__ipipe_stall_root ();barrier ();#else
@@ -144,8 +145,10 @@ static inline unsigned long __raw_local_#define raw_local_save_flags (flags) \ do { (flags) = __raw_local_save_flags(); } while (0) -#define raw_local_irq_save (flags) \ - do { (flags) = __raw_local_irq_save(); } while (0) +#define raw_local_irq_save (flags) do { \ + ipipe_check_context(ipipe_root_domain); \ + (flags) = __raw_local_irq_save(); \ + } while (0)static inline int raw_irqs_disabled_flags (unsigned long flags) {
include/asm-i386/i387.h +5 -4
@@ -75,10 +75,11 @@ static inline void __save_init_fpu( stru }#define __unlazy_fpu ( tsk ) do { \ - if (task_thread_info(tsk)->status & TS_USEDFPU) \ - save_init_fpu( tsk ); \ - else \ - tsk->fpu_counter = 0; \ + if (task_thread_info(tsk)->status & TS_USEDFPU) { \ + __save_init_fpu(tsk); \ + stts(); \ + } else \ + tsk->fpu_counter = 0; \ } while (0)#define __clear_fpu ( tsk ) \
--- include/asm-x86_64/irqflags.h | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)
include/asm-x86_64/irqflags.h +5 -2
@@ -86,6 +86,7 @@ static inline int raw_irqs_disabled_flagstatic inline void raw_local_irq_disable (void ) {#ifdef CONFIG_IPIPE +ipipe_check_context (ipipe_root_domain);__ipipe_stall_root ();barrier ();#else
@@ -128,8 +129,10 @@ static inline unsigned long __raw_local_return flags; } -#define raw_local_irq_save (flags) \ - do { (flags) = __raw_local_irq_save(); } while (0) +#define raw_local_irq_save (flags) do { \ + ipipe_check_context(ipipe_root_domain); \ + (flags) = __raw_local_irq_save(); \ + } while (0)static inline int raw_irqs_disabled (void ) {
A message that is not in the archive (sent off-list or not yet received): [email protected]
Quoted 12 lines · Dmitry Adamushko
Quoted 2 lines · Jan Kiszka
[This patch is a remake of
https://mail.gna.org/public/adeos-main/2006-09/msg00037.html]one minor issue below..
Quoted 1 line
===================================================================
Quoted 4 lines
include/linux/preempt.h +0
-0@@ -8,6 +8,7 @@
Quoted 6 lines
include/linux/preempt.h line 11
Quoted 3 lines
include/linux/preempt.h +1
-0#include <linux/thread_info.h> #include <linux/linkage.h> +#include <linux/preempt.h> includes itself.. is it kindof a new useful trick? if so please educate me :o)
Interesting... I guess someone was trying to type "ipipe_base" while thinking of "preempt". The former seems to be included implicitly, the latter caused no harm. Anyway, corrected patch attached.
Thanks for review.
Jan
--- include/linux/ipipe_base.h | 6 ++++ include/linux/kernel.h | 8 ++++-- include/linux/preempt.h | 59 ++++++++++++++++++--------------------------- kernel/ipipe/Kconfig.debug | 9 ++++++ kernel/ipipe/core.c | 31 +++++++++++++++++++++++ 5 files changed, 76 insertions(+), 37 deletions(-)
include/linux/ipipe_base.h +6 -0
@@ -75,6 +75,12 @@ void __ipipe_unstall_root(void);void __ipipe_restore_root (unsigned long x); +#ifdef CONFIG_IPIPE_DEBUG_CONTEXT +void ipipe_check_context (struct ipipe_domain *border_ipd); +#else /* !CONFIG_IPIPE_DEBUG_CONTEXT */ +static inline void ipipe_check_context (struct ipipe_domain *border_ipd) { } +#endif /* !CONFIG_IPIPE_DEBUG_CONTEXT */ +#endif /* CONFIG_IPIPE */ #endif /* !__LINUX_IPIPE_BASE_H */
include/linux/kernel.h +6 -2
@@ -14,6 +14,7 @@#include <linux/compiler.h> #include <linux/bitops.h> #include <linux/log2.h> +#include <linux/ipipe_base.h> #include <asm/byteorder.h> #include <asm/bug.h>
@@ -72,9 +73,12 @@ struct user;*/ #ifdef CONFIG_PREEMPT_VOLUNTARY extern int cond_resched (void ); -# define might_resched () cond_resched() +# define might_resched () do { \ + ipipe_check_context(ipipe_root_domain); \ + cond_resched(); \ + } while (0)#else -# define might_resched () do { } while (0) +# define might_resched () ipipe_check_context(ipipe_root_domain)#endif #ifdef CONFIG_DEBUG_SPINLOCK_SLEEP
include/linux/preempt.h +24 -35
82 lines not on this page · raw message
kernel/ipipe/Kconfig.debug +9 -0
@@ -2,6 +2,15 @@ config IPIPE_DEBUG bool "I-pipe debugging" depends on IPIPE +config IPIPE_DEBUG_CONTEXT + bool "Check for illicit cross-domain calls" + depends on IPIPE_DEBUG + ---help--- + Enable this feature to arm checkpoints in the kernel that + verify the correct invocation context. On entry of critical + Linux services a warning is issued if the caller is not + running over the root domain. + config IPIPE_TRACE bool "Latency tracing" depends on IPIPE_DEBUG
kernel/ipipe/core.c +31 -0
@@ -33,6 +33,9 @@#include <linux/proc_fs.h> #include <linux/seq_file.h> #endif /* CONFIG_PROC_FS */ +#ifdef CONFIG_IPIPE_TRACE_MCOUNT +#include <linux/ipipe_trace.h> +#endif /* CONFIG_IPIPE_TRACE_MCOUNT */ static int __ipipe_ptd_key_count;
@@ -1383,6 +1386,34 @@ void __init ipipe_init_proc(void)#endif /* CONFIG_PROC_FS */ +#ifdef CONFIG_IPIPE_DEBUG_CONTEXT +void ipipe_check_context (struct ipipe_domain *border_ipd) +{ +static int check_hit; + +if (likely (ipipe_current_domain->priority <= border_ipd->priority) || + check_hit) +return ; + + check_hit =1 ; + +#ifdef CONFIG_IPIPE_TRACE_MCOUNT +ipipe_trace_panic_freeze (); +#endif /* CONFIG_IPIPE_TRACE_MCOUNT */ +ipipe_set_printk_sync (ipipe_current_domain); +printk (KERN_ERR "I-pipe: Detected illicit call from domain '%s'\n" +KERN_ERR " into a service reserved for domain '%s' and " +"below.\n" , + ipipe_current_domain->name, border_ipd->name); +show_stack (NULL ,NULL ); +#ifdef CONFIG_IPIPE_TRACE_MCOUNT +ipipe_trace_panic_dump (); +#endif /* CONFIG_IPIPE_TRACE_MCOUNT */ +} + +EXPORT_SYMBOL (ipipe_check_context); +#endif /* CONFIG_IPIPE_DEBUG_CONTEXT */ +EXPORT_SYMBOL (ipipe_virtualize_irq);EXPORT_SYMBOL (ipipe_control_irq);EXPORT_SYMBOL (ipipe_suspend_domain);
Quoted 5 lines · Jan Kiszka
...
There are already three major instrumentation points for Linux included:
-preempt_xxx()
-might_resched()
- raw_local_irq_save/disable()
- smp_processor_id()
The last spot used to cause quite some headache in earlier patches, and running on it over non-root domain for whatever reason should be detected as well.
Moreover, this patch revision sanitises some #ifdefs and copes the with an internal usage of local_irq_disable over non-root domains in __ipipe_handle_exception().
Jan
PS: The hunk around i386's __unlazy_fpu is already on its way into mainline.
--- include/linux/ipipe_base.h | 6 ++++ include/linux/ipipe_trace.h | 7 ++++- include/linux/kernel.h | 8 ++++- include/linux/preempt.h | 59 +++++++++++++++++--------------------------- include/linux/smp.h | 14 ++++++++-- kernel/ipipe/Kconfig.debug | 9 ++++++ kernel/ipipe/core.c | 25 ++++++++++++++++++ 7 files changed, 88 insertions(+), 40 deletions(-)
include/linux/ipipe_base.h +6 -0
@@ -75,6 +75,12 @@ void __ipipe_unstall_root(void);void __ipipe_restore_root (unsigned long x); +#ifdef CONFIG_IPIPE_DEBUG_CONTEXT +void ipipe_check_context (struct ipipe_domain *border_ipd); +#else /* !CONFIG_IPIPE_DEBUG_CONTEXT */ +static inline void ipipe_check_context (struct ipipe_domain *border_ipd) { } +#endif /* !CONFIG_IPIPE_DEBUG_CONTEXT */ +#endif /* CONFIG_IPIPE */ #endif /* !__LINUX_IPIPE_BASE_H */
include/linux/kernel.h +6 -2
@@ -14,6 +14,7 @@#include <linux/compiler.h> #include <linux/bitops.h> #include <linux/log2.h> +#include <linux/ipipe_base.h> #include <asm/byteorder.h> #include <asm/bug.h>
@@ -72,9 +73,12 @@ struct user;*/ #ifdef CONFIG_PREEMPT_VOLUNTARY extern int cond_resched (void ); -# define might_resched () cond_resched() +# define might_resched () do { \ + ipipe_check_context(ipipe_root_domain); \ + cond_resched(); \ + } while (0)#else -# define might_resched () do { } while (0) +# define might_resched () ipipe_check_context(ipipe_root_domain)#endif #ifdef CONFIG_DEBUG_SPINLOCK_SLEEP
include/linux/preempt.h +24 -35
82 lines not on this page · raw message
kernel/ipipe/Kconfig.debug +9 -0
@@ -2,6 +2,15 @@ config IPIPE_DEBUG bool "I-pipe debugging" depends on IPIPE +config IPIPE_DEBUG_CONTEXT + bool "Check for illicit cross-domain calls" + depends on IPIPE_DEBUG + ---help--- + Enable this feature to arm checkpoints in the kernel that + verify the correct invocation context. On entry of critical + Linux services a warning is issued if the caller is not + running over the root domain. + config IPIPE_TRACE bool "Latency tracing" depends on IPIPE_DEBUG
kernel/ipipe/core.c +25 -0
@@ -33,6 +33,7 @@#include <linux/proc_fs.h> #include <linux/seq_file.h> #endif /* CONFIG_PROC_FS */ +#include <linux/ipipe_trace.h> static int __ipipe_ptd_key_count;
@@ -1383,6 +1384,30 @@ void __init ipipe_init_proc(void)#endif /* CONFIG_PROC_FS */ +#ifdef CONFIG_IPIPE_DEBUG_CONTEXT +void ipipe_check_context (struct ipipe_domain *border_ipd) +{ +static int check_hit; + +if (likely (ipipe_current_domain->priority <= border_ipd->priority) || + check_hit) +return ; + + check_hit =1 ; + +ipipe_trace_panic_freeze (); +ipipe_set_printk_sync (ipipe_current_domain); +printk (KERN_ERR "I-pipe: Detected illicit call from domain '%s'\n" +KERN_ERR " into a service reserved for domain '%s' and " +"below.\n" , + ipipe_current_domain->name, border_ipd->name); +show_stack (NULL ,NULL ); +ipipe_trace_panic_dump (); +} + +EXPORT_SYMBOL (ipipe_check_context); +#endif /* CONFIG_IPIPE_DEBUG_CONTEXT */ +EXPORT_SYMBOL (ipipe_virtualize_irq);EXPORT_SYMBOL (ipipe_control_irq);EXPORT_SYMBOL (ipipe_suspend_domain);
include/linux/smp.h +12 -2
@@ -109,6 +109,8 @@ static inline int smp_call_function_sing#endif /* !SMP */ +#include <linux/ipipe_base.h> +/* * smp_processor_id(): get the current CPU ID. *
@@ -126,9 +128,17 @@ static inline int smp_call_function_sing*/ #ifdef CONFIG_DEBUG_PREEMPT extern unsigned int debug_smp_processor_id (void ); -# define smp_processor_id () debug_smp_processor_id() +# define smp_processor_id () \ + ({ \ + ipipe_check_context(ipipe_root_domain); \ + debug_smp_processor_id(); \ + })#else -# define smp_processor_id () raw_smp_processor_id() +# define smp_processor_id () \ + ({ \ + ipipe_check_context(ipipe_root_domain); \ + raw_smp_processor_id(); \ + })#endif #define get_cpu () ({ preempt_disable(); smp_processor_id(); })
include/linux/ipipe_trace.h +6 -1
@@ -39,6 +39,11 @@ int ipipe_trace_frozen_reset(void);void ipipe_trace_panic_freeze (void );void ipipe_trace_panic_dump (void ); -#endif /* CONFIG_IPIPE_TRACE */ +#else /* !CONFIG_IPIPE_TRACE */ + +static inline void ipipe_trace_panic_freeze (void ) { } +static inline void ipipe_trace_panic_dump (void ) { } + +#endif /* !CONFIG_IPIPE_TRACE */ #endif /* !__LINUX_IPIPE_H */
--- arch/i386/kernel/ipipe.c | 9 +++++++-- arch/i386/kernel/process.c | 2 +- include/asm-i386/i387.h | 9 +++++---- include/asm-i386/irqflags.h | 7 +++++-- 4 files changed, 18 insertions(+), 9 deletions(-)
include/asm-i386/irqflags.h +5 -2
@@ -54,6 +54,7 @@ static inline void raw_local_irq_restorestatic inline void raw_local_irq_disable (void ) {#ifdef CONFIG_IPIPE +ipipe_check_context (ipipe_root_domain);__ipipe_stall_root ();barrier ();#else
@@ -144,8 +145,10 @@ static inline unsigned long __raw_local_#define raw_local_save_flags (flags) \ do { (flags) = __raw_local_save_flags(); } while (0) -#define raw_local_irq_save (flags) \ - do { (flags) = __raw_local_irq_save(); } while (0) +#define raw_local_irq_save (flags) do { \ + ipipe_check_context(ipipe_root_domain); \ + (flags) = __raw_local_irq_save(); \ + } while (0)static inline int raw_irqs_disabled_flags (unsigned long flags) {
include/asm-i386/i387.h +5 -4
@@ -75,10 +75,11 @@ static inline void __save_init_fpu( stru }#define __unlazy_fpu ( tsk ) do { \ - if (task_thread_info(tsk)->status & TS_USEDFPU) \ - save_init_fpu( tsk ); \ - else \ - tsk->fpu_counter = 0; \ + if (task_thread_info(tsk)->status & TS_USEDFPU) { \ + __save_init_fpu(tsk); \ + stts(); \ + } else \ + tsk->fpu_counter = 0; \ } while (0)#define __clear_fpu ( tsk ) \
arch/i386/kernel/process.c +1 -1
@@ -636,7 +636,7 @@ struct task_struct fastcall * __switch_t {struct thread_struct *prev = &prev_p->thread, *next = &next_p->thread; -int cpu =smp_processor_id (); +int cpu =raw_smp_processor_id ();struct tss_struct *tss = &per_cpu (init_tss, cpu);/* never put a printk in __switch_to... printk() calls wake_up*() indirectly */
arch/i386/kernel/ipipe.c +7 -2
@@ -711,8 +711,13 @@ fastcall int __ipipe_handle_exception(st/* Track the hw interrupt state before calling the Linux * exception handler, replicating it into the virtual mask. */ -if (irqs_disabled_hw ()) -local_irq_disable (); +if (irqs_disabled_hw ()) { +/* Do not trigger the alarm in ipipe_check_context() by using +* plain local_irq_disable(). */ +__ipipe_stall_root (); +trace_hardirqs_off (); +barrier (); + }#ifdef CONFIG_KGDB /* catch exception KGDB is interested in over non-root domains */
--- arch/x86_64/kernel/ipipe.c | 9 +++++++-- arch/x86_64/kernel/process.c | 2 +- include/asm-x86_64/irqflags.h | 7 +++++-- 3 files changed, 13 insertions(+), 5 deletions(-)
include/asm-x86_64/irqflags.h +5 -2
@@ -86,6 +86,7 @@ static inline int raw_irqs_disabled_flagstatic inline void raw_local_irq_disable (void ) {#ifdef CONFIG_IPIPE +ipipe_check_context (ipipe_root_domain);__ipipe_stall_root ();barrier ();#else
@@ -128,8 +129,10 @@ static inline unsigned long __raw_local_return flags; } -#define raw_local_irq_save (flags) \ - do { (flags) = __raw_local_irq_save(); } while (0) +#define raw_local_irq_save (flags) do { \ + ipipe_check_context(ipipe_root_domain); \ + (flags) = __raw_local_irq_save(); \ + } while (0)static inline int raw_irqs_disabled (void ) {
arch/x86_64/kernel/ipipe.c +7 -2
@@ -611,8 +611,13 @@ asmlinkage int __ipipe_handle_exception(/* Track the hw interrupt state before calling the Linux * exception handler, replicating it into the virtual mask. */ -if (irqs_disabled_hw ()) -local_irq_disable (); +if (irqs_disabled_hw ()) { +/* Do not trigger the alarm in ipipe_check_context() by using +* plain local_irq_disable(). */ +__ipipe_stall_root (); +trace_hardirqs_off (); +barrier (); + }#ifdef CONFIG_KGDB /* catch exception KGDB is interested in over non-root domains */
arch/x86_64/kernel/process.c +1 -1
@@ -572,7 +572,7 @@ __switch_to(struct task_struct *prev_p, {struct thread_struct *prev = &prev_p->thread, *next = &next_p->thread; -int cpu =smp_processor_id (); +int cpu =raw_smp_processor_id ();struct tss_struct *tss = &per_cpu (init_tss, cpu);/* we're going to use this soon, after a few expensive things */