All of lore.kernel.org
 help / color / mirror / Atom feed
From: greg@kroah.com (Greg KH)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] arm: Remove early stack deallocation from restore_user_regs
Date: Fri, 9 Jan 2015 11:16:47 -0800	[thread overview]
Message-ID: <20150109191647.GA1933@kroah.com> (raw)
In-Reply-To: <20150109172035.GZ12302@n2100.arm.linux.org.uk>

On Fri, Jan 09, 2015 at 05:20:35PM +0000, Russell King - ARM Linux wrote:
> On Fri, Jan 09, 2015 at 05:06:54PM +0000, Daniel Thompson wrote:
> > On 09/01/15 16:46, Russell King - ARM Linux wrote:
> > > On Mon, Jan 05, 2015 at 03:12:38PM +0000, Daniel Thompson wrote:
> > >> Currently restore_user_regs deallocates the SVC stack early in
> > >> its execution and relies on no exception being taken between
> > >> the deallocation and the registers being restored. The introduction
> > >> of a default FIQ handler that also uses the SVC stack breaks this
> > >> assumption and can result in corrupted register state.
> > >>
> > >> This patch works around the problem by removing the early
> > >> stack deallocation and using r2 as a temporary instead. I have
> > >> not found a way to do this without introducing an extra mov
> > >> instruction to the macro.
> > >>
> > >> Signed-off-by: Daniel Thompson <daniel.thompson@linaro.org>
> > >> ---
> > > 
> > > Please put it in the patch system, thanks.
> > 
> > Will do.
> > 
> > 
> > > I think we should queue
> > > this one for stable too, as I think we need this for v3.18
> > > (as a result of c0e7f7ee717e2b4c5791e7422424c96b5008c39e,
> > > ARM: 8150/3: fiq: Replace default FIQ handler)?
> > 
> > It's a close call.
> 
> I agree.
> 
> > Before 8150/3 the system would probably crash if the default FIQ handler
> > ran. After 8150/3 the system is also likely to crash since there's no
> > code hooked into the handler in v3.18 that can clear the source of FIQ
> > leaving us stuck re-entering the FIQ handler.
> > 
> > Nevertheless, this is a nasty gremlin to leave for backporters (it
> > wasn't easy to find) so I'd be very happy to Cc: stable and see what
> > they think.
> 
> Well, we could ask Greg now.  It's not a regression (as nothing makes
> use of the previously merged changes yet), but it is a nasty latent bug
> which we could easily solve.
> 
> Having thought about it some more, I'm tempted to say... leave the
> stable tag off it, and we can make the decision later - after it's had
> a chance to really prove itself to a much wider audience.  That'd be
> the lowest risk for the 3.18 stable tree.

That sounds like a good idea, you can always email stable@ to let us
know of a patch we should add at a later time.

thanks,

greg k-h

WARNING: multiple messages have this Message-ID (diff)
From: Greg KH <greg@kroah.com>
To: Russell King - ARM Linux <linux@arm.linux.org.uk>
Cc: Daniel Thompson <daniel.thompson@linaro.org>,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, patches@linaro.org,
	linaro-kernel@lists.linaro.org,
	John Stultz <john.stultz@linaro.org>,
	Sumit Semwal <sumit.semwal@linaro.org>
Subject: Re: [PATCH] arm: Remove early stack deallocation from restore_user_regs
Date: Fri, 9 Jan 2015 11:16:47 -0800	[thread overview]
Message-ID: <20150109191647.GA1933@kroah.com> (raw)
In-Reply-To: <20150109172035.GZ12302@n2100.arm.linux.org.uk>

On Fri, Jan 09, 2015 at 05:20:35PM +0000, Russell King - ARM Linux wrote:
> On Fri, Jan 09, 2015 at 05:06:54PM +0000, Daniel Thompson wrote:
> > On 09/01/15 16:46, Russell King - ARM Linux wrote:
> > > On Mon, Jan 05, 2015 at 03:12:38PM +0000, Daniel Thompson wrote:
> > >> Currently restore_user_regs deallocates the SVC stack early in
> > >> its execution and relies on no exception being taken between
> > >> the deallocation and the registers being restored. The introduction
> > >> of a default FIQ handler that also uses the SVC stack breaks this
> > >> assumption and can result in corrupted register state.
> > >>
> > >> This patch works around the problem by removing the early
> > >> stack deallocation and using r2 as a temporary instead. I have
> > >> not found a way to do this without introducing an extra mov
> > >> instruction to the macro.
> > >>
> > >> Signed-off-by: Daniel Thompson <daniel.thompson@linaro.org>
> > >> ---
> > > 
> > > Please put it in the patch system, thanks.
> > 
> > Will do.
> > 
> > 
> > > I think we should queue
> > > this one for stable too, as I think we need this for v3.18
> > > (as a result of c0e7f7ee717e2b4c5791e7422424c96b5008c39e,
> > > ARM: 8150/3: fiq: Replace default FIQ handler)?
> > 
> > It's a close call.
> 
> I agree.
> 
> > Before 8150/3 the system would probably crash if the default FIQ handler
> > ran. After 8150/3 the system is also likely to crash since there's no
> > code hooked into the handler in v3.18 that can clear the source of FIQ
> > leaving us stuck re-entering the FIQ handler.
> > 
> > Nevertheless, this is a nasty gremlin to leave for backporters (it
> > wasn't easy to find) so I'd be very happy to Cc: stable and see what
> > they think.
> 
> Well, we could ask Greg now.  It's not a regression (as nothing makes
> use of the previously merged changes yet), but it is a nasty latent bug
> which we could easily solve.
> 
> Having thought about it some more, I'm tempted to say... leave the
> stable tag off it, and we can make the decision later - after it's had
> a chance to really prove itself to a much wider audience.  That'd be
> the lowest risk for the 3.18 stable tree.

That sounds like a good idea, you can always email stable@ to let us
know of a patch we should add at a later time.

thanks,

greg k-h

  reply	other threads:[~2015-01-09 19:16 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-12-12 11:11 [RFC PATCH] arm: Remove early stack deallocation from restore_user_regs Daniel Thompson
2014-12-12 11:11 ` Daniel Thompson
2015-01-05 15:12 ` [PATCH] " Daniel Thompson
2015-01-05 15:12   ` Daniel Thompson
2015-01-09 16:46   ` Russell King - ARM Linux
2015-01-09 16:46     ` Russell King - ARM Linux
2015-01-09 17:06     ` Daniel Thompson
2015-01-09 17:06       ` Daniel Thompson
2015-01-09 17:20       ` Russell King - ARM Linux
2015-01-09 17:20         ` Russell King - ARM Linux
2015-01-09 19:16         ` Greg KH [this message]
2015-01-09 19:16           ` Greg KH

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=20150109191647.GA1933@kroah.com \
    --to=greg@kroah.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    /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.