linux-kernel - Re: [RFC PATCH v1 01/10] s390/uaccess: Add storage key checked access to user memory

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 18 Jan 2022 16:52:58 +0100
From:   Janis Schoetterl-Glausch <scgl@...ux.ibm.com>
To:     Sven Schnelle <svens@...ux.ibm.com>
Cc:     Heiko Carstens <hca@...ux.ibm.com>,
        Vasily Gorbik <gor@...ux.ibm.com>,
        Christian Borntraeger <borntraeger@...ux.ibm.com>,
        Nico Boehr <nrb@...ux.ibm.com>,
        Alexander Gordeev <agordeev@...ux.ibm.com>,
        linux-s390@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [RFC PATCH v1 01/10] s390/uaccess: Add storage key checked access
 to user memory

On 1/18/22 16:37, Sven Schnelle wrote:
> Hi Janis,
> 
> Janis Schoetterl-Glausch <scgl@...ux.ibm.com> writes:
> 
>> KVM needs a mechanism to do accesses to guest memory that honor
>> storage key protection.
>> Since the copy_to/from_user implementation makes use of move
>> instructions that support having an additional access key supplied,
>> we can implement __copy_from/to_user_with_key by enhancing the
>> existing implementation.
>>
>> Signed-off-by: Janis Schoetterl-Glausch <scgl@...ux.ibm.com>
> 
> This doesn't apply to my master branch.

Maybe it's due to the prerequisite patch missing?
https://lore.kernel.org/linux-s390/YeGBmPBJ8NMi0Rkp@osiris/T/#t

> 
>> diff --git a/arch/s390/lib/uaccess.c b/arch/s390/lib/uaccess.c
>> index d3a700385875..ce7a150dd93a 100644
>> --- a/arch/s390/lib/uaccess.c
>> +++ b/arch/s390/lib/uaccess.c
>> @@ -59,11 +59,13 @@ static inline int copy_with_mvcos(void)
>>  #endif
>>  
>>  static inline unsigned long copy_from_user_mvcos(void *x, const void __user *ptr,
>> -						 unsigned long size)
>> +						 unsigned long size, char key)
>>  {
>>  	unsigned long tmp1, tmp2;
>>  	union oac spec = {
>> +		.oac2.key = key,
>>  		.oac2.as = PSW_BITS_AS_SECONDARY,
>> +		.oac2.k = 1,
>>  		.oac2.a = 1,
>>  	};
>>  
>> @@ -94,19 +96,19 @@ static inline unsigned long copy_from_user_mvcos(void *x, const void __user *ptr
>>  }
>>  
>>  static inline unsigned long copy_from_user_mvcp(void *x, const void __user *ptr,
>> -						unsigned long size)
>> +						unsigned long size, char key)
> 
> Any special reason for using 'char' as type for key here? Given the left shift
> below i would prefer 'unsigned char' to avoid having to think about
> whether this can overflow. The end result wouldn't look different,
> so more or less a cosmetic issue.

Will do.

[...]
> 
> With that minor nitpick:
> 
> Reviewed-by: Sven Schnelle <svens@...ux.ibm.com>

Thanks!