2021-01-21 15:19:24

by Janosch Frank

[permalink] [raw]
Subject: [PATCH v2 2/2] s390: mm: Fix secure storage access exception handling

Turns out that the bit 61 in the TEID is not always 1 and if that's
the case the address space ID and the address are
unpredictable. Without an address and its address space ID we can't
export memory and hence we can only send a SIGSEGV to the process or
panic the kernel depending on who caused the exception.

Signed-off-by: Janosch Frank <[email protected]>
Fixes: 084ea4d611a3d ("s390/mm: add (non)secure page access exceptions handlers")
Cc: [email protected]
---
arch/s390/mm/fault.c | 14 ++++++++++++++
1 file changed, 14 insertions(+)

diff --git a/arch/s390/mm/fault.c b/arch/s390/mm/fault.c
index e30c7c781172..3e8685ad938d 100644
--- a/arch/s390/mm/fault.c
+++ b/arch/s390/mm/fault.c
@@ -791,6 +791,20 @@ void do_secure_storage_access(struct pt_regs *regs)
struct page *page;
int rc;

+ /* There are cases where we don't have a TEID. */
+ if (!(regs->int_parm_long & 0x4)) {
+ /*
+ * When this happens, userspace did something that it
+ * was not supposed to do, e.g. branching into secure
+ * memory. Trigger a segmentation fault.
+ */
+ if (user_mode(regs)) {
+ send_sig(SIGSEGV, current, 0);
+ return;
+ } else
+ panic("Unexpected PGM 0x3d with TEID bit 61=0");
+ }
+
switch (get_fault_type(regs)) {
case USER_FAULT:
mm = current->mm;
--
2.25.1


2021-01-21 15:45:02

by Christian Borntraeger

[permalink] [raw]
Subject: Re: [PATCH v2 2/2] s390: mm: Fix secure storage access exception handling


On 21.01.21 16:14, Janosch Frank wrote:
> Turns out that the bit 61 in the TEID is not always 1 and if that's
> the case the address space ID and the address are
> unpredictable. Without an address and its address space ID we can't
> export memory and hence we can only send a SIGSEGV to the process or
> panic the kernel depending on who caused the exception.
>
> Signed-off-by: Janosch Frank <[email protected]>
> Fixes: 084ea4d611a3d ("s390/mm: add (non)secure page access exceptions handlers")
> Cc: [email protected]

Reviewed-by: Christian Borntraeger <[email protected]>

we should let it run in our CI for some days, though.

> ---
> arch/s390/mm/fault.c | 14 ++++++++++++++
> 1 file changed, 14 insertions(+)
>
> diff --git a/arch/s390/mm/fault.c b/arch/s390/mm/fault.c
> index e30c7c781172..3e8685ad938d 100644
> --- a/arch/s390/mm/fault.c
> +++ b/arch/s390/mm/fault.c
> @@ -791,6 +791,20 @@ void do_secure_storage_access(struct pt_regs *regs)
> struct page *page;
> int rc;
>
> + /* There are cases where we don't have a TEID. */
> + if (!(regs->int_parm_long & 0x4)) {
> + /*
> + * When this happens, userspace did something that it
> + * was not supposed to do, e.g. branching into secure
> + * memory. Trigger a segmentation fault.
> + */
> + if (user_mode(regs)) {
> + send_sig(SIGSEGV, current, 0);
> + return;
> + } else
> + panic("Unexpected PGM 0x3d with TEID bit 61=0");
> + }
> +
> switch (get_fault_type(regs)) {
> case USER_FAULT:
> mm = current->mm;
>

2021-01-22 12:47:28

by Claudio Imbrenda

[permalink] [raw]
Subject: Re: [PATCH v2 2/2] s390: mm: Fix secure storage access exception handling

On Thu, 21 Jan 2021 10:14:35 -0500
Janosch Frank <[email protected]> wrote:

> Turns out that the bit 61 in the TEID is not always 1 and if that's
> the case the address space ID and the address are
> unpredictable. Without an address and its address space ID we can't
> export memory and hence we can only send a SIGSEGV to the process or
> panic the kernel depending on who caused the exception.
>
> Signed-off-by: Janosch Frank <[email protected]>
> Fixes: 084ea4d611a3d ("s390/mm: add (non)secure page access
> exceptions handlers") Cc: [email protected]
> ---
> arch/s390/mm/fault.c | 14 ++++++++++++++
> 1 file changed, 14 insertions(+)
>
> diff --git a/arch/s390/mm/fault.c b/arch/s390/mm/fault.c
> index e30c7c781172..3e8685ad938d 100644
> --- a/arch/s390/mm/fault.c
> +++ b/arch/s390/mm/fault.c
> @@ -791,6 +791,20 @@ void do_secure_storage_access(struct pt_regs
> *regs) struct page *page;
> int rc;
>
> + /* There are cases where we don't have a TEID. */
> + if (!(regs->int_parm_long & 0x4)) {
> + /*
> + * When this happens, userspace did something that it
> + * was not supposed to do, e.g. branching into secure
> + * memory. Trigger a segmentation fault.
> + */
> + if (user_mode(regs)) {
> + send_sig(SIGSEGV, current, 0);
> + return;
> + } else
> + panic("Unexpected PGM 0x3d with TEID bit
> 61=0");
> + }
> +
> switch (get_fault_type(regs)) {
> case USER_FAULT:
> mm = current->mm;

Reviewed-by: Claudio Imbrenda <[email protected]>