| Message ID | 20191101221150.116536-8-samitolvanen@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v4,01/17] arm64: mm: avoid x18 in idmap_kpti_install_ng_mappings | expand |
On Fri, Nov 01, 2019 at 03:11:40PM -0700, Sami Tolvanen wrote: > Implements CONFIG_DEBUG_STACK_USAGE for shadow stacks. When enabled, > also prints out the highest shadow stack usage per process. > > Signed-off-by: Sami Tolvanen <samitolvanen@google.com> Thanks for helping me find this Kconfig. :) :) Reviewed-by: Kees Cook <keescook@chromium.org> -Kees > --- > kernel/scs.c | 39 +++++++++++++++++++++++++++++++++++++++ > 1 file changed, 39 insertions(+) > > diff --git a/kernel/scs.c b/kernel/scs.c > index 7780fc4e29ac..67c43af627d1 100644 > --- a/kernel/scs.c > +++ b/kernel/scs.c > @@ -167,6 +167,44 @@ int scs_prepare(struct task_struct *tsk, int node) > return 0; > } > > +#ifdef CONFIG_DEBUG_STACK_USAGE > +static inline unsigned long scs_used(struct task_struct *tsk) > +{ > + unsigned long *p = __scs_base(tsk); > + unsigned long *end = scs_magic(tsk); > + uintptr_t s = (uintptr_t)p; > + > + while (p < end && *p) > + p++; > + > + return (uintptr_t)p - s; > +} > + > +static void scs_check_usage(struct task_struct *tsk) > +{ > + static DEFINE_SPINLOCK(lock); > + static unsigned long highest; > + unsigned long used = scs_used(tsk); > + > + if (used <= highest) > + return; > + > + spin_lock(&lock); > + > + if (used > highest) { > + pr_info("%s: highest shadow stack usage %lu bytes\n", > + __func__, used); > + highest = used; > + } > + > + spin_unlock(&lock); > +} > +#else > +static inline void scs_check_usage(struct task_struct *tsk) > +{ > +} > +#endif > + > bool scs_corrupted(struct task_struct *tsk) > { > return *scs_magic(tsk) != SCS_END_MAGIC; > @@ -181,6 +219,7 @@ void scs_release(struct task_struct *tsk) > return; > > WARN_ON(scs_corrupted(tsk)); > + scs_check_usage(tsk); > > scs_account(tsk, -1); > task_set_scs(tsk, NULL); > -- > 2.24.0.rc1.363.gb1bccd3e3d-goog >
On Fri, Nov 01, 2019 at 03:11:40PM -0700, Sami Tolvanen wrote: > Implements CONFIG_DEBUG_STACK_USAGE for shadow stacks. When enabled, > also prints out the highest shadow stack usage per process. > > Signed-off-by: Sami Tolvanen <samitolvanen@google.com> > --- > kernel/scs.c | 39 +++++++++++++++++++++++++++++++++++++++ > 1 file changed, 39 insertions(+) > > diff --git a/kernel/scs.c b/kernel/scs.c > index 7780fc4e29ac..67c43af627d1 100644 > --- a/kernel/scs.c > +++ b/kernel/scs.c > @@ -167,6 +167,44 @@ int scs_prepare(struct task_struct *tsk, int node) > return 0; > } > > +#ifdef CONFIG_DEBUG_STACK_USAGE > +static inline unsigned long scs_used(struct task_struct *tsk) > +{ > + unsigned long *p = __scs_base(tsk); > + unsigned long *end = scs_magic(tsk); > + uintptr_t s = (uintptr_t)p; As previously, please use unsigned long for consistency. > + > + while (p < end && *p) > + p++; I think this is the only place where we legtimately access the shadow call stack directly. When using SCS and KASAN, are the compiler-generated accesses to the SCS instrumented? If not, it might make sense to make this: while (p < end && READ_ONCE_NOCKECK(*p)) ... and poison the allocation from KASAN's PoV, so that we can find unintentional accesses more easily. Mark. > + > + return (uintptr_t)p - s; > +} > + > +static void scs_check_usage(struct task_struct *tsk) > +{ > + static DEFINE_SPINLOCK(lock); > + static unsigned long highest; > + unsigned long used = scs_used(tsk); > + > + if (used <= highest) > + return; > + > + spin_lock(&lock); > + > + if (used > highest) { > + pr_info("%s: highest shadow stack usage %lu bytes\n", > + __func__, used); > + highest = used; > + } > + > + spin_unlock(&lock); > +} > +#else > +static inline void scs_check_usage(struct task_struct *tsk) > +{ > +} > +#endif > + > bool scs_corrupted(struct task_struct *tsk) > { > return *scs_magic(tsk) != SCS_END_MAGIC; > @@ -181,6 +219,7 @@ void scs_release(struct task_struct *tsk) > return; > > WARN_ON(scs_corrupted(tsk)); > + scs_check_usage(tsk); > > scs_account(tsk, -1); > task_set_scs(tsk, NULL); > -- > 2.24.0.rc1.363.gb1bccd3e3d-goog >
On Mon, Nov 4, 2019 at 4:40 AM Mark Rutland <mark.rutland@arm.com> wrote: > > +#ifdef CONFIG_DEBUG_STACK_USAGE > > +static inline unsigned long scs_used(struct task_struct *tsk) > > +{ > > + unsigned long *p = __scs_base(tsk); > > + unsigned long *end = scs_magic(tsk); > > + uintptr_t s = (uintptr_t)p; > > As previously, please use unsigned long for consistency. Ack. > > + while (p < end && *p) > > + p++; > > I think this is the only place where we legtimately access the shadow > call stack directly. There's also scs_corrupted, which checks that the end magic is intact. > When using SCS and KASAN, are the > compiler-generated accesses to the SCS instrumented? > > If not, it might make sense to make this: > > while (p < end && READ_ONCE_NOCKECK(*p)) > > ... and poison the allocation from KASAN's PoV, so that we can find > unintentional accesses more easily. Sure, that makes sense. I can poison the allocation for the non-vmalloc case, I'll just need to refactor scs_set_magic to happen before the poisoning. Sami
On Mon, Nov 04, 2019 at 01:35:28PM -0800, Sami Tolvanen wrote: > On Mon, Nov 4, 2019 at 4:40 AM Mark Rutland <mark.rutland@arm.com> wrote: > > > +#ifdef CONFIG_DEBUG_STACK_USAGE > > > +static inline unsigned long scs_used(struct task_struct *tsk) > > > +{ > > > + unsigned long *p = __scs_base(tsk); > > > + unsigned long *end = scs_magic(tsk); > > > + uintptr_t s = (uintptr_t)p; > > > > As previously, please use unsigned long for consistency. > > Ack. > > > > + while (p < end && *p) > > > + p++; > > > > I think this is the only place where we legtimately access the shadow > > call stack directly. > > There's also scs_corrupted, which checks that the end magic is intact. Ah, true. I missed that. > > When using SCS and KASAN, are the > > compiler-generated accesses to the SCS instrumented? > > > > If not, it might make sense to make this: > > > > while (p < end && READ_ONCE_NOCKECK(*p)) > > > > ... and poison the allocation from KASAN's PoV, so that we can find > > unintentional accesses more easily. > > Sure, that makes sense. I can poison the allocation for the > non-vmalloc case, I'll just need to refactor scs_set_magic to happen > before the poisoning. Sounds good! Mark.
diff --git a/kernel/scs.c b/kernel/scs.c index 7780fc4e29ac..67c43af627d1 100644 --- a/kernel/scs.c +++ b/kernel/scs.c @@ -167,6 +167,44 @@ int scs_prepare(struct task_struct *tsk, int node) return 0; } +#ifdef CONFIG_DEBUG_STACK_USAGE +static inline unsigned long scs_used(struct task_struct *tsk) +{ + unsigned long *p = __scs_base(tsk); + unsigned long *end = scs_magic(tsk); + uintptr_t s = (uintptr_t)p; + + while (p < end && *p) + p++; + + return (uintptr_t)p - s; +} + +static void scs_check_usage(struct task_struct *tsk) +{ + static DEFINE_SPINLOCK(lock); + static unsigned long highest; + unsigned long used = scs_used(tsk); + + if (used <= highest) + return; + + spin_lock(&lock); + + if (used > highest) { + pr_info("%s: highest shadow stack usage %lu bytes\n", + __func__, used); + highest = used; + } + + spin_unlock(&lock); +} +#else +static inline void scs_check_usage(struct task_struct *tsk) +{ +} +#endif + bool scs_corrupted(struct task_struct *tsk) { return *scs_magic(tsk) != SCS_END_MAGIC; @@ -181,6 +219,7 @@ void scs_release(struct task_struct *tsk) return; WARN_ON(scs_corrupted(tsk)); + scs_check_usage(tsk); scs_account(tsk, -1); task_set_scs(tsk, NULL);
Implements CONFIG_DEBUG_STACK_USAGE for shadow stacks. When enabled, also prints out the highest shadow stack usage per process. Signed-off-by: Sami Tolvanen <samitolvanen@google.com> --- kernel/scs.c | 39 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+)