Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Message-ID: <dbfec527-b995-e382-dafa-c3459e1e45ed@linux.ibm.com>
Date:   Thu, 20 Jan 2022 09:34:05 +0100
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.4.0
Subject: Re: [RFC PATCH v1 01/10] s390/uaccess: Add storage key checked access
 to user memory
Content-Language: en-US
To:     Heiko Carstens <hca@linux.ibm.com>
Cc:     Vasily Gorbik <gor@linux.ibm.com>,
        Christian Borntraeger <borntraeger@linux.ibm.com>,
        Sven Schnelle <svens@linux.ibm.com>,
        Nico Boehr <nrb@linux.ibm.com>,
        Alexander Gordeev <agordeev@linux.ibm.com>,
        linux-s390@vger.kernel.org, linux-kernel@vger.kernel.org
References: <20220118095210.1651483-1-scgl@linux.ibm.com>
 <20220118095210.1651483-2-scgl@linux.ibm.com> <YefeakONMN4PLlml@osiris>
 <422595a5-b24b-8760-ff0e-112322142de7@linux.ibm.com>
 <YegQCTqEsiFTUZ2R@osiris>
From:   Janis Schoetterl-Glausch <scgl@linux.ibm.com>
In-Reply-To: <YegQCTqEsiFTUZ2R@osiris>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk

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?