On 9/4/20 12:35 PM, Heiko Carstens wrote:
> On Thu, Sep 03, 2020 at 09:14:35AM -0400, Janosch Frank wrote:
>> Program exception 3f (secure storage violation) can only be detected
>> when the CPU is running in SIE with a format 4 state description,
>> e.g. running a protected guest. Because of this and because user
>> space partly controls the guest memory mapping and can trigger this
>> exception, we want to send a SIGSEGV to the process running the guest
>> and not panic the kernel.
>>
>> Signed-off-by: Janosch Frank <frankja@xxxxxxxxxxxxx>
>> CC: <stable@xxxxxxxxxxxxxxx> # 5.7+
>> Fixes: 084ea4d611a3 ("s390/mm: add (non)secure page access exceptions handlers")
>> Reviewed-by: Claudio Imbrenda <imbrenda@xxxxxxxxxxxxx>
>> ---
>> arch/s390/kernel/pgm_check.S | 2 +-
>> arch/s390/mm/fault.c | 23 +++++++++++++++++++++++
>> 2 files changed, 24 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/s390/kernel/pgm_check.S b/arch/s390/kernel/pgm_check.S
>> index 2c27907a5ffc..9a92638360ee 100644
>> --- a/arch/s390/kernel/pgm_check.S
>> +++ b/arch/s390/kernel/pgm_check.S
>> @@ -80,7 +80,7 @@ PGM_CHECK(do_dat_exception) /* 3b */
>> PGM_CHECK_DEFAULT /* 3c */
>> PGM_CHECK(do_secure_storage_access) /* 3d */
>> PGM_CHECK(do_non_secure_storage_access) /* 3e */
>> -PGM_CHECK_DEFAULT /* 3f */
>> +PGM_CHECK(do_secure_storage_violation) /* 3f */
>> PGM_CHECK(monitor_event_exception) /* 40 */
>> PGM_CHECK_DEFAULT /* 41 */
>> PGM_CHECK_DEFAULT /* 42 */
>> diff --git a/arch/s390/mm/fault.c b/arch/s390/mm/fault.c
>> index 4c8c063bce5b..20abb7c5c540 100644
>> --- a/arch/s390/mm/fault.c
>> +++ b/arch/s390/mm/fault.c
>> @@ -859,6 +859,24 @@ void do_non_secure_storage_access(struct pt_regs *regs)
>> }
>> NOKPROBE_SYMBOL(do_non_secure_storage_access);
>>
>> +void do_secure_storage_violation(struct pt_regs *regs)
>> +{
>> + char buf[TASK_COMM_LEN];
>> +
>> + /*
>> + * Either KVM messed up the secure guest mapping or the same
>> + * page is mapped into multiple secure guests.
>> + *
>> + * This exception is only triggered when a guest 2 is running
>> + * and can therefore never occur in kernel context.
>> + */
>> + printk_ratelimited(KERN_WARNING
>> + "Secure storage violation in task: %s, pid %d\n",
>> + get_task_comm(buf, current), task_pid_nr(current));
>
> Why get_task_comm() and task_pid_nr() instead of simply current->comm
> and current->pid?
Normally if there are functions to get data I assume those should be used.
> Also: is the dmesg message of any value?
Yes, it's import for administrators to know that an exception caused
this segfault and not some memory shenanigans.
As the exception only occurs if a guest runs in unsupported modes like
sharing the memory between two secure guests it's a good first
indication what went wrong.
>
>> + send_sig(SIGSEGV, current, 0);
>> +}
>> +NOKPROBE_SYMBOL(do_secure_storage_violation);>
> Why is this NOKPROBE? Can this deadlock?
Right, we don't need to export this at all, this was copied over from
the export function above.
Attachment:
signature.asc
Description: OpenPGP digital signature
