LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH RESEND] firmware: qcom: scm: Fix crash in qcom_scm_call_atomic1()
@ 2018-04-26 11:55 Niklas Cassel
  2018-04-26 18:01 ` Bjorn Andersson
  0 siblings, 1 reply; 4+ messages in thread
From: Niklas Cassel @ 2018-04-26 11:55 UTC (permalink / raw)
  To: andy.gross, david.brown
  Cc: bjorn.andersson, sboyd, linux-arm-msm, linux-soc, linux-kernel,
	Niklas Cassel

qcom_scm_call_atomic1() can crash with a NULL pointer dereference at
qcom_scm_call_atomic1+0x30/0x48.

disassembly of qcom_scm_call_atomic1():
...
<0xc08d73b0 <+12>: ldr r3, [r12]
... (no instruction explicitly modifies r12)
0xc08d73cc <+40>: smc 0
... (no instruction explicitly modifies r12)
0xc08d73d4 <+48>: ldr r3, [r12] <- crashing instruction
...

Since the first ldr is successful, and since r12 isn't explicitly
modified by any instruction between the first and the second ldr,
it must have been modified by the smc call, which is ok,
since r12 is caller save according to the AAPCS.

Add r12 to the clobber list so that the compiler knows that the
callee potentially overwrites the value in r12.
Clobber descriptions may not in any way overlap with an input or
output operand.

Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
Reviewed-by: Stephen Boyd <sboyd@kernel.org>
Signed-off-by: Niklas Cassel <niklas.cassel@linaro.org>
---
 drivers/firmware/qcom_scm-32.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/firmware/qcom_scm-32.c b/drivers/firmware/qcom_scm-32.c
index dfbd894d5bb7..4e24e591ae74 100644
--- a/drivers/firmware/qcom_scm-32.c
+++ b/drivers/firmware/qcom_scm-32.c
@@ -147,7 +147,7 @@ static u32 smc(u32 cmd_addr)
 			"smc	#0	@ switch to secure world\n"
 			: "=r" (r0)
 			: "r" (r0), "r" (r1), "r" (r2)
-			: "r3");
+			: "r3", "r12");
 	} while (r0 == QCOM_SCM_INTERRUPTED);
 
 	return r0;
@@ -263,7 +263,7 @@ static s32 qcom_scm_call_atomic1(u32 svc, u32 cmd, u32 arg1)
 			"smc    #0      @ switch to secure world\n"
 			: "=r" (r0)
 			: "r" (r0), "r" (r1), "r" (r2)
-			: "r3");
+			: "r3", "r12");
 	return r0;
 }
 
@@ -298,7 +298,7 @@ static s32 qcom_scm_call_atomic2(u32 svc, u32 cmd, u32 arg1, u32 arg2)
 			"smc    #0      @ switch to secure world\n"
 			: "=r" (r0)
 			: "r" (r0), "r" (r1), "r" (r2), "r" (r3)
-			);
+			: "r12");
 	return r0;
 }
 
@@ -328,7 +328,7 @@ u32 qcom_scm_get_version(void)
 			"smc	#0	@ switch to secure world\n"
 			: "=r" (r0), "=r" (r1)
 			: "r" (r0), "r" (r1)
-			: "r2", "r3");
+			: "r2", "r3", "r12");
 	} while (r0 == QCOM_SCM_INTERRUPTED);
 
 	version = r1;
-- 
2.14.3

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH RESEND] firmware: qcom: scm: Fix crash in qcom_scm_call_atomic1()
  2018-04-26 11:55 [PATCH RESEND] firmware: qcom: scm: Fix crash in qcom_scm_call_atomic1() Niklas Cassel
@ 2018-04-26 18:01 ` Bjorn Andersson
  2018-04-26 19:25   ` Niklas Cassel
  0 siblings, 1 reply; 4+ messages in thread
From: Bjorn Andersson @ 2018-04-26 18:01 UTC (permalink / raw)
  To: Niklas Cassel
  Cc: andy.gross, david.brown, sboyd, linux-arm-msm, linux-soc, linux-kernel

On Thu 26 Apr 04:55 PDT 2018, Niklas Cassel wrote:

> qcom_scm_call_atomic1() can crash with a NULL pointer dereference at
> qcom_scm_call_atomic1+0x30/0x48.
> 

Hi Niklas,

The change has been picked up for v4.18, as you can see here:
https://git.kernel.org/pub/scm/linux/kernel/git/agross/linux.git/log/?h=for-next

Regards,
Bjorn

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH RESEND] firmware: qcom: scm: Fix crash in qcom_scm_call_atomic1()
  2018-04-26 18:01 ` Bjorn Andersson
@ 2018-04-26 19:25   ` Niklas Cassel
  2018-04-26 21:13     ` Andy Gross
  0 siblings, 1 reply; 4+ messages in thread
From: Niklas Cassel @ 2018-04-26 19:25 UTC (permalink / raw)
  To: Bjorn Andersson
  Cc: andy.gross, david.brown, sboyd, linux-arm-msm, linux-soc, linux-kernel

On Thu, Apr 26, 2018 at 11:01:30AM -0700, Bjorn Andersson wrote:
> On Thu 26 Apr 04:55 PDT 2018, Niklas Cassel wrote:
> 
> > qcom_scm_call_atomic1() can crash with a NULL pointer dereference at
> > qcom_scm_call_atomic1+0x30/0x48.
> > 
> 
> Hi Niklas,
> 
> The change has been picked up for v4.18, as you can see here:
> https://git.kernel.org/pub/scm/linux/kernel/git/agross/linux.git/log/?h=for-next

Hello Björn,

I just looked at today's linux-next tag: next-20180426.
I didn't bother to check Andy's tree.

Andy's for-next branch is included in linux-next, so it
will probably be included in tomorrow's linux-next tag.

Sorry for the noise.

Kind regards,
Niklas

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH RESEND] firmware: qcom: scm: Fix crash in qcom_scm_call_atomic1()
  2018-04-26 19:25   ` Niklas Cassel
@ 2018-04-26 21:13     ` Andy Gross
  0 siblings, 0 replies; 4+ messages in thread
From: Andy Gross @ 2018-04-26 21:13 UTC (permalink / raw)
  To: Niklas Cassel
  Cc: Bjorn Andersson, david.brown, sboyd, linux-arm-msm, linux-soc,
	linux-kernel

On Thu, Apr 26, 2018 at 09:25:36PM +0200, Niklas Cassel wrote:
> On Thu, Apr 26, 2018 at 11:01:30AM -0700, Bjorn Andersson wrote:
> > On Thu 26 Apr 04:55 PDT 2018, Niklas Cassel wrote:
> > 
> > > qcom_scm_call_atomic1() can crash with a NULL pointer dereference at
> > > qcom_scm_call_atomic1+0x30/0x48.
> > > 
> > 
> > Hi Niklas,
> > 
> > The change has been picked up for v4.18, as you can see here:
> > https://git.kernel.org/pub/scm/linux/kernel/git/agross/linux.git/log/?h=for-next
> 
> Hello Björn,
> 
> I just looked at today's linux-next tag: next-20180426.
> I didn't bother to check Andy's tree.
> 
> Andy's for-next branch is included in linux-next, so it
> will probably be included in tomorrow's linux-next tag.
> 
> Sorry for the noise.

I'm pretty bad about broadcasting which patches i am taking. Sorry bout that.

Andy

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2018-04-26 21:14 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-04-26 11:55 [PATCH RESEND] firmware: qcom: scm: Fix crash in qcom_scm_call_atomic1() Niklas Cassel
2018-04-26 18:01 ` Bjorn Andersson
2018-04-26 19:25   ` Niklas Cassel
2018-04-26 21:13     ` Andy Gross

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).