From: Volodymyr Babchuk <volodymyr_babchuk@epam.com>
To: Julien Grall <julien.grall@arm.com>, xen-devel@lists.xen.org
Cc: Marc Zyngier <marc.zyngier@arm.com>, sstabellini@kernel.org
Subject: Re: [PATCH 2/6] xen/arm: smccc-1.1: Handle function result as parameters
Date: Mon, 27 Aug 2018 17:05:02 +0300 [thread overview]
Message-ID: <e3e1b4dd-fada-2cff-db16-387ba5a3acef@epam.com> (raw)
In-Reply-To: <20180824165820.32620-3-julien.grall@arm.com>
Hi,
On 24.08.18 19:58, Julien Grall wrote:
> From: Marc Zyngier <marc.zyngier@arm.com>
>
> If someone has the silly idea to write something along those lines:
>
> extern u64 foo(void);
>
> void bar(struct arm_smccc_res *res)
> {
> arm_smccc_1_1_smc(0xbad, foo(), res);
> }
>
> they are in for a surprise, as this gets compiled as:
>
> 0000000000000588 <bar>:
> 588: a9be7bfd stp x29, x30, [sp, #-32]!
> 58c: 910003fd mov x29, sp
> 590: f9000bf3 str x19, [sp, #16]
> 594: aa0003f3 mov x19, x0
> 598: aa1e03e0 mov x0, x30
> 59c: 94000000 bl 0 <_mcount>
> 5a0: 94000000 bl 0 <foo>
> 5a4: aa0003e1 mov x1, x0
> 5a8: d4000003 smc #0x0
> 5ac: b4000073 cbz x19, 5b8 <bar+0x30>
> 5b0: a9000660 stp x0, x1, [x19]
> 5b4: a9010e62 stp x2, x3, [x19, #16]
> 5b8: f9400bf3 ldr x19, [sp, #16]
> 5bc: a8c27bfd ldp x29, x30, [sp], #32
> 5c0: d65f03c0 ret
> 5c4: d503201f nop
>
> The call to foo "overwrites" the x0 register for the return value,
> and we end up calling the wrong secure service.
>
> A solution is to evaluate all the parameters before assigning
> anything to specific registers, leading to the expected result:
>
> 0000000000000588 <bar>:
> 588: a9be7bfd stp x29, x30, [sp, #-32]!
> 58c: 910003fd mov x29, sp
> 590: f9000bf3 str x19, [sp, #16]
> 594: aa0003f3 mov x19, x0
> 598: aa1e03e0 mov x0, x30
> 59c: 94000000 bl 0 <_mcount>
> 5a0: 94000000 bl 0 <foo>
> 5a4: aa0003e1 mov x1, x0
> 5a8: d28175a0 mov x0, #0xbad
> 5ac: d4000003 smc #0x0
> 5b0: b4000073 cbz x19, 5bc <bar+0x34>
> 5b4: a9000660 stp x0, x1, [x19]
> 5b8: a9010e62 stp x2, x3, [x19, #16]
> 5bc: f9400bf3 ldr x19, [sp, #16]
> 5c0: a8c27bfd ldp x29, x30, [sp], #32
> 5c4: d65f03c0 ret
>
> Reported-by: Stefano Stabellini <stefanos@xilinx.com>
> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
Reviewed-by: Volodymyr Babchuk <volodymr_babchuk@epam.com>
> ---
> xen/include/asm-arm/smccc.h | 30 ++++++++++++++++++++----------
> 1 file changed, 20 insertions(+), 10 deletions(-)
>
> diff --git a/xen/include/asm-arm/smccc.h b/xen/include/asm-arm/smccc.h
> index a31d67a1de..648bef28bd 100644
> --- a/xen/include/asm-arm/smccc.h
> +++ b/xen/include/asm-arm/smccc.h
> @@ -125,41 +125,51 @@ struct arm_smccc_res {
> register unsigned long r3 asm("r3")
>
> #define __declare_arg_1(a0, a1, res) \
> + typeof(a1) __a1 = a1; \
> struct arm_smccc_res *___res = res; \
> register unsigned long r0 asm("r0") = (uint32_t)a0;\
> - register unsigned long r1 asm("r1") = a1; \
> + register unsigned long r1 asm("r1") = __a1; \
> register unsigned long r2 asm("r2"); \
> register unsigned long r3 asm("r3")
>
> #define __declare_arg_2(a0, a1, a2, res) \
> + typeof(a1) __a1 = a1; \
> + typeof(a2) __a2 = a2; \
> struct arm_smccc_res *___res = res; \
> register unsigned long r0 asm("r0") = (uint32_t)a0;\
> - register unsigned long r1 asm("r1") = a1; \
> - register unsigned long r2 asm("r2") = a2; \
> + register unsigned long r1 asm("r1") = __a1; \
> + register unsigned long r2 asm("r2") = __a2; \
> register unsigned long r3 asm("r3")
>
> #define __declare_arg_3(a0, a1, a2, a3, res) \
> + typeof(a1) __a1 = a1; \
> + typeof(a2) __a2 = a2; \
> + typeof(a3) __a3 = a3; \
> struct arm_smccc_res *___res = res; \
> register unsigned long r0 asm("r0") = (uint32_t)a0;\
> - register unsigned long r1 asm("r1") = a1; \
> - register unsigned long r2 asm("r2") = a2; \
> - register unsigned long r3 asm("r3") = a3
> + register unsigned long r1 asm("r1") = __a1; \
> + register unsigned long r2 asm("r2") = __a2; \
> + register unsigned long r3 asm("r3") = __a3
>
> #define __declare_arg_4(a0, a1, a2, a3, a4, res) \
> + typeof(a4) __a4 = a4; \
> __declare_arg_3(a0, a1, a2, a3, res); \
> - register unsigned long r4 asm("r4") = a4
> + register unsigned long r4 asm("r4") = __a4
>
> #define __declare_arg_5(a0, a1, a2, a3, a4, a5, res) \
> + typeof(a5) __a5 = a5; \
> __declare_arg_4(a0, a1, a2, a3, a4, res); \
> - register typeof(a5) r5 asm("r5") = a5
> + register typeof(a5) r5 asm("r5") = __a5
>
> #define __declare_arg_6(a0, a1, a2, a3, a4, a5, a6, res) \
> + typeof(a6) __a6 = a6; \
> __declare_arg_5(a0, a1, a2, a3, a4, a5, res); \
> - register typeof(a6) r6 asm("r6") = a6
> + register typeof(a6) r6 asm("r6") = __a6
>
> #define __declare_arg_7(a0, a1, a2, a3, a4, a5, a6, a7, res) \
> + typeof(a7) __a7 = a7; \
> __declare_arg_6(a0, a1, a2, a3, a4, a5, a6, res); \
> - register typeof(a7) r7 asm("r7") = a7
> + register typeof(a7) r7 asm("r7") = __a7
>
> #define ___declare_args(count, ...) __declare_arg_ ## count(__VA_ARGS__)
> #define __declare_args(count, ...) ___declare_args(count, __VA_ARGS__)
>
--
Volodymyr Babchuk
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
next prev parent reply other threads:[~2018-08-27 14:05 UTC|newest]
Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-08-24 16:58 [PATCH 0/6] xen/arm: SMCCC fixup and improvement Julien Grall
2018-08-24 16:58 ` [PATCH 1/6] xen/arm: smccc-1.1: Make return values unsigned long Julien Grall
2018-08-27 14:03 ` Volodymyr Babchuk
2018-08-27 15:23 ` Julien Grall
2018-08-27 16:33 ` Volodymyr Babchuk
2018-08-24 16:58 ` [PATCH 2/6] xen/arm: smccc-1.1: Handle function result as parameters Julien Grall
2018-08-27 14:05 ` Volodymyr Babchuk [this message]
2018-08-24 16:58 ` [PATCH 3/6] xen/arm: add SMC wrapper that is compatible with SMCCC v1.0 Julien Grall
2018-08-24 16:58 ` [PATCH 4/6] xen/arm: cpufeature: Add helper to check constant caps Julien Grall
2018-08-30 17:43 ` Volodymyr Babchuk
2018-09-25 16:53 ` Julien Grall
2018-08-24 16:58 ` [PATCH 5/6] xen/arm: smccc: Add wrapper to automatically select the calling convention Julien Grall
2018-08-27 14:15 ` Volodymyr Babchuk
2018-08-27 15:29 ` Julien Grall
2018-08-27 16:50 ` Volodymyr Babchuk
2018-08-28 14:05 ` Julien Grall
2018-08-28 14:40 ` Volodymyr Babchuk
2018-08-28 14:43 ` Julien Grall
2018-08-28 15:10 ` Volodymyr Babchuk
2018-08-28 15:27 ` Julien Grall
2018-08-28 15:50 ` Volodymyr Babchuk
2018-08-28 15:57 ` Julien Grall
2018-08-30 17:45 ` Volodymyr Babchuk
2018-08-24 16:58 ` [PATCH 6/6] xen/arm: Replace call_smc with arm_smccc_smc Julien Grall
2018-08-27 13:53 ` Volodymyr Babchuk
2018-08-30 16:41 ` [PATCH 0/6] xen/arm: SMCCC fixup and improvement Volodymyr Babchuk
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=e3e1b4dd-fada-2cff-db16-387ba5a3acef@epam.com \
--to=volodymyr_babchuk@epam.com \
--cc=julien.grall@arm.com \
--cc=marc.zyngier@arm.com \
--cc=sstabellini@kernel.org \
--cc=xen-devel@lists.xen.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.