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]

Message-ID: <dbfec527-b995-e382-dafa-c3459e1e45ed@linux.ibm.com>
Date:   Thu, 20 Jan 2022 09:34:05 +0100
From:   Janis Schoetterl-Glausch <scgl@...ux.ibm.com>
To:     Heiko Carstens <hca@...ux.ibm.com>
Cc:     Vasily Gorbik <gor@...ux.ibm.com>,
        Christian Borntraeger <borntraeger@...ux.ibm.com>,
        Sven Schnelle <svens@...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/19/22 14:20, Heiko Carstens wrote:
> On Wed, Jan 19, 2022 at 12:02:34PM +0100, Janis Schoetterl-Glausch wrote:
>>> That's a lot of code churn... I would have expected that the existing
>>> functions will be renamed, get an additional key parameter, and the
>>> current API is implemented by defines which map copy_to_user() &
>>> friends to the new functions, and add a zero key.
>>
>> I don't think I understand you. I can implement raw_copy_from/to_user
>> in terms of raw_copy_from/to_user_with_key, which does save a few lines,
>> but that's it, isn't it?
> 
> Right you are. I only looked at your patch, and forgot about that all
> the wrapping is nowadays done in common code.
> 
> So what I really don't like about this approach is that we get an arch
> specific copy_to_user() implementation back. This means that all those
> extra calls like might_fault(), instrument_copy_to_user(), and friends
> now have to be kept in sync by us again, if new instrumentation or
> security options are added to common code.
> 
> Given that this is manual work / monitoring I'm sure this will not
> work in the mid or long term, like it has been proven several times in
> the past for other features. We need something better, which works
> out-of-the-box wrt common code changes / enhancements.

What are our options?

1. Tooling
   1.1 Automatic monitoring
   1.2 ?
2. Implementation changes
   2.1 Modify common code
   2.2 Don't modify common code, pass key argument via well known location
   2.3 ?

Neither of 2.1 and 2.2 seem great.
How might 1.1 work? A build error if they are out of sync?