* [PATCH] powerpc/64: Initialise thread_info for emergency stacks
@ 2017-06-20 13:58 Nicholas Piggin
2017-06-21 2:01 ` Michael Ellerman
2017-06-21 9:25 ` Abdul Haleem
0 siblings, 2 replies; 4+ messages in thread
From: Nicholas Piggin @ 2017-06-20 13:58 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Nicholas Piggin, Michael Ellerman, akpm, Stephen Rothwell
Emergency stacks have their thread_info mostly uninitialised, which in
particular means garbage preempt_count values.
Emergency stack code runs with interrupts disabled entirely, and is
used very rarely, so this has been unnoticed so far. It was found by a
proposed new powerpc watchdog that takes a soft-NMI directly from the
masked_interrupt handler and using the emergency stack. That crashed at
BUG_ON(in_nmi()) in nmi_enter(). preempt_count()s were found to be
garbage.
Reported-by: Abdul Haleem <abdhalee@linux.vnet.ibm.com>
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
FYI, this bug looks to be breaking linux-next on some powerpc
boxes due to interaction with a proposed new powerpc watchdog
driver Andrew has in his tree:
http://marc.info/?l=linuxppc-embedded&m=149794320519941&w=2
arch/powerpc/include/asm/thread_info.h | 19 +++++++++++++++++++
arch/powerpc/kernel/setup_64.c | 6 +++---
2 files changed, 22 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/include/asm/thread_info.h b/arch/powerpc/include/asm/thread_info.h
index a941cc6fc3e9..5995e4b2996d 100644
--- a/arch/powerpc/include/asm/thread_info.h
+++ b/arch/powerpc/include/asm/thread_info.h
@@ -54,6 +54,7 @@ struct thread_info {
.task = &tsk, \
.cpu = 0, \
.preempt_count = INIT_PREEMPT_COUNT, \
+ .local_flags = 0, \
.flags = 0, \
}
@@ -62,6 +63,24 @@ struct thread_info {
#define THREAD_SIZE_ORDER (THREAD_SHIFT - PAGE_SHIFT)
+/*
+ * Emergency stacks are used for a range of things, from asynchronous
+ * NMIs (system reset, machine check) to synchronous, process context.
+ * Set HARDIRQ_OFFSET because we don't know exactly what context we
+ * come from or if it had a valid stack, which is about the best we
+ * can do.
+ * TODO: what to do with accounting?
+ */
+#define emstack_init_thread_info(ti, c) \
+do { \
+ (ti)->task = NULL; \
+ (ti)->cpu = (c); \
+ (ti)->preempt_count = HARDIRQ_OFFSET; \
+ (ti)->local_flags = 0; \
+ (ti)->flags = 0; \
+ klp_init_thread_info(ti); \
+} while (0)
+
/* how to get the thread information struct from C */
static inline struct thread_info *current_thread_info(void)
{
diff --git a/arch/powerpc/kernel/setup_64.c b/arch/powerpc/kernel/setup_64.c
index f35ff9dea4fb..54c4336655f8 100644
--- a/arch/powerpc/kernel/setup_64.c
+++ b/arch/powerpc/kernel/setup_64.c
@@ -639,18 +639,18 @@ void __init emergency_stack_init(void)
for_each_possible_cpu(i) {
struct thread_info *ti;
ti = __va(memblock_alloc_base(THREAD_SIZE, THREAD_SIZE, limit));
- klp_init_thread_info(ti);
+ emstack_init_thread_info(ti, i);
paca[i].emergency_sp = (void *)ti + THREAD_SIZE;
#ifdef CONFIG_PPC_BOOK3S_64
/* emergency stack for NMI exception handling. */
ti = __va(memblock_alloc_base(THREAD_SIZE, THREAD_SIZE, limit));
- klp_init_thread_info(ti);
+ emstack_init_thread_info(ti, i);
paca[i].nmi_emergency_sp = (void *)ti + THREAD_SIZE;
/* emergency stack for machine check exception handling. */
ti = __va(memblock_alloc_base(THREAD_SIZE, THREAD_SIZE, limit));
- klp_init_thread_info(ti);
+ emstack_init_thread_info(ti, i);
paca[i].mc_emergency_sp = (void *)ti + THREAD_SIZE;
#endif
}
--
2.11.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] powerpc/64: Initialise thread_info for emergency stacks
2017-06-20 13:58 [PATCH] powerpc/64: Initialise thread_info for emergency stacks Nicholas Piggin
@ 2017-06-21 2:01 ` Michael Ellerman
2017-06-21 2:41 ` Nicholas Piggin
2017-06-21 9:25 ` Abdul Haleem
1 sibling, 1 reply; 4+ messages in thread
From: Michael Ellerman @ 2017-06-21 2:01 UTC (permalink / raw)
To: Nicholas Piggin, linuxppc-dev; +Cc: Nicholas Piggin, akpm, Stephen Rothwell
Nicholas Piggin <npiggin@gmail.com> writes:
> diff --git a/arch/powerpc/include/asm/thread_info.h b/arch/powerpc/include/asm/thread_info.h
> index a941cc6fc3e9..5995e4b2996d 100644
> --- a/arch/powerpc/include/asm/thread_info.h
> +++ b/arch/powerpc/include/asm/thread_info.h
> @@ -62,6 +63,24 @@ struct thread_info {
>
> #define THREAD_SIZE_ORDER (THREAD_SHIFT - PAGE_SHIFT)
>
> +/*
> + * Emergency stacks are used for a range of things, from asynchronous
> + * NMIs (system reset, machine check) to synchronous, process context.
> + * Set HARDIRQ_OFFSET because we don't know exactly what context we
> + * come from or if it had a valid stack, which is about the best we
> + * can do.
> + * TODO: what to do with accounting?
> + */
> +#define emstack_init_thread_info(ti, c) \
> +do { \
> + (ti)->task = NULL; \
> + (ti)->cpu = (c); \
> + (ti)->preempt_count = HARDIRQ_OFFSET; \
> + (ti)->local_flags = 0; \
> + (ti)->flags = 0; \
> + klp_init_thread_info(ti); \
> +} while (0)
Why don't we just bzero() the whole thing? Like we do for the other
stacks?
cheers
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] powerpc/64: Initialise thread_info for emergency stacks
2017-06-21 2:01 ` Michael Ellerman
@ 2017-06-21 2:41 ` Nicholas Piggin
0 siblings, 0 replies; 4+ messages in thread
From: Nicholas Piggin @ 2017-06-21 2:41 UTC (permalink / raw)
To: Michael Ellerman; +Cc: linuxppc-dev, akpm, Stephen Rothwell
On Wed, 21 Jun 2017 12:01:37 +1000
Michael Ellerman <mpe@ellerman.id.au> wrote:
> Nicholas Piggin <npiggin@gmail.com> writes:
>
> > diff --git a/arch/powerpc/include/asm/thread_info.h b/arch/powerpc/include/asm/thread_info.h
> > index a941cc6fc3e9..5995e4b2996d 100644
> > --- a/arch/powerpc/include/asm/thread_info.h
> > +++ b/arch/powerpc/include/asm/thread_info.h
> > @@ -62,6 +63,24 @@ struct thread_info {
> >
> > #define THREAD_SIZE_ORDER (THREAD_SHIFT - PAGE_SHIFT)
> >
> > +/*
> > + * Emergency stacks are used for a range of things, from asynchronous
> > + * NMIs (system reset, machine check) to synchronous, process context.
> > + * Set HARDIRQ_OFFSET because we don't know exactly what context we
> > + * come from or if it had a valid stack, which is about the best we
> > + * can do.
> > + * TODO: what to do with accounting?
> > + */
> > +#define emstack_init_thread_info(ti, c) \
> > +do { \
> > + (ti)->task = NULL; \
> > + (ti)->cpu = (c); \
> > + (ti)->preempt_count = HARDIRQ_OFFSET; \
> > + (ti)->local_flags = 0; \
> > + (ti)->flags = 0; \
> > + klp_init_thread_info(ti); \
> > +} while (0)
>
> Why don't we just bzero() the whole thing? Like we do for the other
> stacks?
Wouldn't hurt to.
Thanks,
Nick
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] powerpc/64: Initialise thread_info for emergency stacks
2017-06-20 13:58 [PATCH] powerpc/64: Initialise thread_info for emergency stacks Nicholas Piggin
2017-06-21 2:01 ` Michael Ellerman
@ 2017-06-21 9:25 ` Abdul Haleem
1 sibling, 0 replies; 4+ messages in thread
From: Abdul Haleem @ 2017-06-21 9:25 UTC (permalink / raw)
To: Nicholas Piggin; +Cc: linuxppc-dev, akpm, Stephen Rothwell
On Tue, 2017-06-20 at 23:58 +1000, Nicholas Piggin wrote:
> Emergency stacks have their thread_info mostly uninitialised, which in
> particular means garbage preempt_count values.
>
> Emergency stack code runs with interrupts disabled entirely, and is
> used very rarely, so this has been unnoticed so far. It was found by a
> proposed new powerpc watchdog that takes a soft-NMI directly from the
> masked_interrupt handler and using the emergency stack. That crashed at
> BUG_ON(in_nmi()) in nmi_enter(). preempt_count()s were found to be
> garbage.
>
> Reported-by: Abdul Haleem <abdhalee@linux.vnet.ibm.com>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
>
> FYI, this bug looks to be breaking linux-next on some powerpc
> boxes due to interaction with a proposed new powerpc watchdog
> driver Andrew has in his tree:
>
> http://marc.info/?l=linuxppc-embedded&m=149794320519941&w=2
>
> arch/powerpc/include/asm/thread_info.h | 19 +++++++++++++++++++
> arch/powerpc/kernel/setup_64.c | 6 +++---
> 2 files changed, 22 insertions(+), 3 deletions(-)
Hi Nicholas,
Thanks for the patch, Verified on next-20170621 and PowerPC bare-metal
boots fine with your patch
Tested-by: Abdul Haleem <abdhalee@linux.vnet.ibm.com>
Thanks for all your support.
--
Regard's
Abdul Haleem
IBM Linux Technology Centre
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2017-06-21 9:25 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-06-20 13:58 [PATCH] powerpc/64: Initialise thread_info for emergency stacks Nicholas Piggin
2017-06-21 2:01 ` Michael Ellerman
2017-06-21 2:41 ` Nicholas Piggin
2017-06-21 9:25 ` Abdul Haleem
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.