diff mbox

[6/6] s390: scrub registers on kernel entry and KVM exit

Message ID 1516182519-10623-7-git-send-email-schwidefsky@de.ibm.com (mailing list archive)
State New, archived
Headers show

Commit Message

Martin Schwidefsky Jan. 17, 2018, 9:48 a.m. UTC 
Clear all user space registers on entry to the kernel and all KVM guest
registers on KVM guest exit if the register does not contain either a
parameter or a result value.

Suggested-by: Christian Borntraeger <borntraeger@de.ibm.com>
Reviewed-by: Christian Borntraeger <borntraeger@de.ibm.com>
Signed-off-by: Martin Schwidefsky <schwidefsky@de.ibm.com>
---
 arch/s390/kernel/entry.S | 41 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 41 insertions(+)

Comments

Hao QingFeng Jan. 19, 2018, 6:29 a.m. UTC | #1 
在 2018/1/17 17:48, Martin Schwidefsky 写道:
> Clear all user space registers on entry to the kernel and all KVM guest
> registers on KVM guest exit if the register does not contain either a
> parameter or a result value.
I am not sure if I understand this but it will be safer?
And can we abstract the operations to be a macro like CLEAR_REG_7?
Thanks
>
> Suggested-by: Christian Borntraeger <borntraeger@de.ibm.com>
> Reviewed-by: Christian Borntraeger <borntraeger@de.ibm.com>
> Signed-off-by: Martin Schwidefsky <schwidefsky@de.ibm.com>
> ---
>   arch/s390/kernel/entry.S | 41 +++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 41 insertions(+)
>
> diff --git a/arch/s390/kernel/entry.S b/arch/s390/kernel/entry.S
> index 2a22c03..47227d3 100644
> --- a/arch/s390/kernel/entry.S
> +++ b/arch/s390/kernel/entry.S
> @@ -322,6 +322,12 @@ ENTRY(sie64a)
>   sie_exit:
>   	lg	%r14,__SF_EMPTY+8(%r15)		# load guest register save area
>   	stmg	%r0,%r13,0(%r14)		# save guest gprs 0-13
> +	xgr	%r0,%r0				# clear guest registers
> +	xgr	%r1,%r1
> +	xgr	%r2,%r2
> +	xgr	%r3,%r3
> +	xgr	%r4,%r4
> +	xgr	%r5,%r5
>   	lmg	%r6,%r14,__SF_GPRS(%r15)	# restore kernel registers
>   	lg	%r2,__SF_EMPTY+16(%r15)		# return exit reason code
>   	br	%r14
> @@ -358,6 +364,7 @@ ENTRY(system_call)
>   	UPDATE_VTIME %r8,%r9,__LC_SYNC_ENTER_TIMER
>   	BPENTER __TI_flags(%r12),_TIF_NOBP
>   	stmg	%r0,%r7,__PT_R0(%r11)
> +	xgr	%r0,%r0
>   	mvc	__PT_R8(64,%r11),__LC_SAVE_AREA_SYNC
>   	mvc	__PT_PSW(16,%r11),__LC_SVC_OLD_PSW
>   	mvc	__PT_INT_CODE(4,%r11),__LC_SVC_ILC
> @@ -640,6 +647,14 @@ ENTRY(pgm_check_handler)
>   4:	lgr	%r13,%r11
>   	la	%r11,STACK_FRAME_OVERHEAD(%r15)
>   	stmg	%r0,%r7,__PT_R0(%r11)
> +	xgr	%r0,%r0			# clear user space registers
> +	xgr	%r1,%r1
> +	xgr	%r2,%r2
> +	xgr	%r3,%r3
> +	xgr	%r4,%r4
> +	xgr	%r5,%r5
> +	xgr	%r6,%r6
> +	xgr	%r7,%r7
>   	mvc	__PT_R8(64,%r11),__LC_SAVE_AREA_SYNC
>   	stmg	%r8,%r9,__PT_PSW(%r11)
>   	mvc	__PT_INT_CODE(4,%r11),__LC_PGM_ILC
> @@ -706,6 +721,15 @@ ENTRY(io_int_handler)
>   	lmg	%r8,%r9,__LC_IO_OLD_PSW
>   	SWITCH_ASYNC __LC_SAVE_AREA_ASYNC,__LC_ASYNC_ENTER_TIMER
>   	stmg	%r0,%r7,__PT_R0(%r11)
> +	xgr	%r0,%r0			# clear user space registers
> +	xgr	%r1,%r1
> +	xgr	%r2,%r2
> +	xgr	%r3,%r3
> +	xgr	%r4,%r4
> +	xgr	%r5,%r5
> +	xgr	%r6,%r6
> +	xgr	%r7,%r7
> +	xgr	%r10,%r10
>   	mvc	__PT_R8(64,%r11),__LC_SAVE_AREA_ASYNC
>   	stmg	%r8,%r9,__PT_PSW(%r11)
>   	mvc	__PT_INT_CODE(12,%r11),__LC_SUBCHANNEL_ID
> @@ -924,6 +948,15 @@ ENTRY(ext_int_handler)
>   	lmg	%r8,%r9,__LC_EXT_OLD_PSW
>   	SWITCH_ASYNC __LC_SAVE_AREA_ASYNC,__LC_ASYNC_ENTER_TIMER
>   	stmg	%r0,%r7,__PT_R0(%r11)
> +	xgr	%r0,%r0			# clear user space registers
> +	xgr	%r1,%r1
> +	xgr	%r2,%r2
> +	xgr	%r3,%r3
> +	xgr	%r4,%r4
> +	xgr	%r5,%r5
> +	xgr	%r6,%r6
> +	xgr	%r7,%r7
> +	xgr	%r10,%r10
>   	mvc	__PT_R8(64,%r11),__LC_SAVE_AREA_ASYNC
>   	stmg	%r8,%r9,__PT_PSW(%r11)
>   	lghi	%r1,__LC_EXT_PARAMS2
> @@ -1133,6 +1166,14 @@ ENTRY(mcck_int_handler)
>   .Lmcck_skip:
>   	lghi	%r14,__LC_GPREGS_SAVE_AREA+64
>   	stmg	%r0,%r7,__PT_R0(%r11)
> +	xgr	%r0,%r0			# clear user space registers
> +	xgr	%r2,%r2
> +	xgr	%r3,%r3
> +	xgr	%r4,%r4
> +	xgr	%r5,%r5
> +	xgr	%r6,%r6
> +	xgr	%r7,%r7
> +	xgr	%r10,%r10
>   	mvc	__PT_R8(64,%r11),0(%r14)
>   	stmg	%r8,%r9,__PT_PSW(%r11)
>   	xc	__PT_FLAGS(8,%r11),__PT_FLAGS(%r11)
Christian Borntraeger Jan. 19, 2018, 7:57 a.m. UTC | #2 
On 01/19/2018 07:29 AM, QingFeng Hao wrote:
> 
> 
> 在 2018/1/17 17:48, Martin Schwidefsky 写道:
>> Clear all user space registers on entry to the kernel and all KVM guest
>> registers on KVM guest exit if the register does not contain either a
>> parameter or a result value.
> I am not sure if I understand this but it will be safer?

It ist similar to commit 0cb5b30698fd ("kvm: vmx: Scrub hardware GPRs at VM-exit").
The idea is to minimize potential payload channels.

> And can we abstract the operations to be a macro like CLEAR_REG_7?

No, please.
xgr %r7,%r7
is absolutely clear what it does, a MACRO often is not.
Hao QingFeng Jan. 19, 2018, 8:27 a.m. UTC | #3 
在 2018/1/19 15:57, Christian Borntraeger 写道:
>
> On 01/19/2018 07:29 AM, QingFeng Hao wrote:
>>
>> 在 2018/1/17 17:48, Martin Schwidefsky 写道:
>>> Clear all user space registers on entry to the kernel and all KVM guest
>>> registers on KVM guest exit if the register does not contain either a
>>> parameter or a result value.
>> I am not sure if I understand this but it will be safer?
> It ist similar to commit 0cb5b30698fd ("kvm: vmx: Scrub hardware GPRs at VM-exit").
> The idea is to minimize potential payload channels.
Got it! thanks for your explanation!
>
>> And can we abstract the operations to be a macro like CLEAR_REG_7?
> No, please.
> xgr %r7,%r7
> is absolutely clear what it does, a MACRO often is not.
nod, this makes sense!
diff mbox

Patch

diff --git a/arch/s390/kernel/entry.S b/arch/s390/kernel/entry.S
index 2a22c03..47227d3 100644
--- a/arch/s390/kernel/entry.S
+++ b/arch/s390/kernel/entry.S
@@ -322,6 +322,12 @@  ENTRY(sie64a)
 sie_exit:
 	lg	%r14,__SF_EMPTY+8(%r15)		# load guest register save area
 	stmg	%r0,%r13,0(%r14)		# save guest gprs 0-13
+	xgr	%r0,%r0				# clear guest registers
+	xgr	%r1,%r1
+	xgr	%r2,%r2
+	xgr	%r3,%r3
+	xgr	%r4,%r4
+	xgr	%r5,%r5
 	lmg	%r6,%r14,__SF_GPRS(%r15)	# restore kernel registers
 	lg	%r2,__SF_EMPTY+16(%r15)		# return exit reason code
 	br	%r14
@@ -358,6 +364,7 @@  ENTRY(system_call)
 	UPDATE_VTIME %r8,%r9,__LC_SYNC_ENTER_TIMER
 	BPENTER __TI_flags(%r12),_TIF_NOBP
 	stmg	%r0,%r7,__PT_R0(%r11)
+	xgr	%r0,%r0
 	mvc	__PT_R8(64,%r11),__LC_SAVE_AREA_SYNC
 	mvc	__PT_PSW(16,%r11),__LC_SVC_OLD_PSW
 	mvc	__PT_INT_CODE(4,%r11),__LC_SVC_ILC
@@ -640,6 +647,14 @@  ENTRY(pgm_check_handler)
 4:	lgr	%r13,%r11
 	la	%r11,STACK_FRAME_OVERHEAD(%r15)
 	stmg	%r0,%r7,__PT_R0(%r11)
+	xgr	%r0,%r0			# clear user space registers
+	xgr	%r1,%r1
+	xgr	%r2,%r2
+	xgr	%r3,%r3
+	xgr	%r4,%r4
+	xgr	%r5,%r5
+	xgr	%r6,%r6
+	xgr	%r7,%r7
 	mvc	__PT_R8(64,%r11),__LC_SAVE_AREA_SYNC
 	stmg	%r8,%r9,__PT_PSW(%r11)
 	mvc	__PT_INT_CODE(4,%r11),__LC_PGM_ILC
@@ -706,6 +721,15 @@  ENTRY(io_int_handler)
 	lmg	%r8,%r9,__LC_IO_OLD_PSW
 	SWITCH_ASYNC __LC_SAVE_AREA_ASYNC,__LC_ASYNC_ENTER_TIMER
 	stmg	%r0,%r7,__PT_R0(%r11)
+	xgr	%r0,%r0			# clear user space registers
+	xgr	%r1,%r1
+	xgr	%r2,%r2
+	xgr	%r3,%r3
+	xgr	%r4,%r4
+	xgr	%r5,%r5
+	xgr	%r6,%r6
+	xgr	%r7,%r7
+	xgr	%r10,%r10
 	mvc	__PT_R8(64,%r11),__LC_SAVE_AREA_ASYNC
 	stmg	%r8,%r9,__PT_PSW(%r11)
 	mvc	__PT_INT_CODE(12,%r11),__LC_SUBCHANNEL_ID
@@ -924,6 +948,15 @@  ENTRY(ext_int_handler)
 	lmg	%r8,%r9,__LC_EXT_OLD_PSW
 	SWITCH_ASYNC __LC_SAVE_AREA_ASYNC,__LC_ASYNC_ENTER_TIMER
 	stmg	%r0,%r7,__PT_R0(%r11)
+	xgr	%r0,%r0			# clear user space registers
+	xgr	%r1,%r1
+	xgr	%r2,%r2
+	xgr	%r3,%r3
+	xgr	%r4,%r4
+	xgr	%r5,%r5
+	xgr	%r6,%r6
+	xgr	%r7,%r7
+	xgr	%r10,%r10
 	mvc	__PT_R8(64,%r11),__LC_SAVE_AREA_ASYNC
 	stmg	%r8,%r9,__PT_PSW(%r11)
 	lghi	%r1,__LC_EXT_PARAMS2
@@ -1133,6 +1166,14 @@  ENTRY(mcck_int_handler)
 .Lmcck_skip:
 	lghi	%r14,__LC_GPREGS_SAVE_AREA+64
 	stmg	%r0,%r7,__PT_R0(%r11)
+	xgr	%r0,%r0			# clear user space registers
+	xgr	%r2,%r2
+	xgr	%r3,%r3
+	xgr	%r4,%r4
+	xgr	%r5,%r5
+	xgr	%r6,%r6
+	xgr	%r7,%r7
+	xgr	%r10,%r10
 	mvc	__PT_R8(64,%r11),0(%r14)
 	stmg	%r8,%r9,__PT_PSW(%r11)
 	xc	__PT_FLAGS(8,%r11),__PT_FLAGS(%r11)