All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
To: Thomas Gleixner <tglx@linutronix.de>
Cc: Tvrtko Ursulin <tursulin@ursulin.net>,
	linux-kernel@vger.kernel.org, Ingo Molnar <mingo@kernel.org>,
	Peter Zijlstra <peterz@infradead.org>,
	Sebastian Andrzej Siewior <bigeasy@linutronix.de>,
	Boris Ostrovsky <boris.ostrovsky@oracle.com>
Subject: Re: [PATCH] cpu/hotplug: Reset node state after operation
Date: Sat, 21 Oct 2017 19:49:55 -0700	[thread overview]
Message-ID: <20171022024955.GA3521@linux.vnet.ibm.com> (raw)
In-Reply-To: <alpine.DEB.2.20.1710211606130.3213@nanos>

On Sat, Oct 21, 2017 at 04:06:52PM +0200, Thomas Gleixner wrote:
> The recent rework of the cpu hotplug internals changed the usage of the per
> cpu state->node field, but missed to clean it up after usage.
> 
> So subsequent hotplug operations use the stale pointer from a previous
> operation and hand it into the callback functions. The callbacks then
> dereference a pointer which either belongs to a different facility or
> points to freed and potentially reused memory. In either case data
> corruption and crashes are the obvious consequence.
> 
> Reset the node and the last pointers in the per cpu state to NULL after the
> operation which set them has completed.
> 
> Fixes: 96abb968549c ("smp/hotplug: Allow external multi-instance rollback")
> Reported-by: Tvrtko Ursulin <tursulin@ursulin.net>
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>

This does fine with rcutorture, though it does not fix the lost
(or egregiously delayed) timer problem that is still eluding me.
Nevertheless, for whatever it is worth:

Tested-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>

> ---
>  kernel/cpu.c |    5 +++++
>  1 file changed, 5 insertions(+)
> 
> --- a/kernel/cpu.c
> +++ b/kernel/cpu.c
> @@ -632,6 +632,11 @@ cpuhp_invoke_ap_callback(int cpu, enum c
>  		__cpuhp_kick_ap(st);
>  	}
> 
> +	/*
> +	 * Clean up the leftovers so the next hotplug operation wont use stale
> +	 * data.
> +	 */
> +	st->node = st->last = NULL;
>  	return ret;
>  }
> 
> 

  parent reply	other threads:[~2017-10-22  2:50 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-10-21 14:06 [PATCH] cpu/hotplug: Reset node state after operation Thomas Gleixner
2017-10-21 14:16 ` [tip:smp/urgent] " tip-bot for Thomas Gleixner
2017-10-22  2:49 ` Paul E. McKenney [this message]
2017-10-22  9:22 ` [PATCH] " Peter Zijlstra
2017-10-23  8:01 ` Tvrtko Ursulin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20171022024955.GA3521@linux.vnet.ibm.com \
    --to=paulmck@linux.vnet.ibm.com \
    --cc=bigeasy@linutronix.de \
    --cc=boris.ostrovsky@oracle.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=peterz@infradead.org \
    --cc=tglx@linutronix.de \
    --cc=tursulin@ursulin.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.