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;
Date:   Wed, 19 Jan 2022 14:20:09 +0100
From:   Heiko Carstens <hca@linux.ibm.com>
To:     Janis Schoetterl-Glausch <scgl@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
Subject: Re: [RFC PATCH v1 01/10] s390/uaccess: Add storage key checked
 access to user memory
Message-ID: <YegQCTqEsiFTUZ2R@osiris>
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>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <422595a5-b24b-8760-ff0e-112322142de7@linux.ibm.com>
Precedence: bulk

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.