Re: [PATCH] tpm: factor out the user space mm from tpm_vtpm_set_locality()

[Jarkko Sakkinen]

On Wed, 2023-05-31 at 11:20 -0400, Stefan Berger wrote:
> 
> On 5/30/23 16:50, Jarkko Sakkinen wrote:
> > From: Jarkko Sakkinen <jarkko.sakkinen@xxxxxxx>
> > 
> > vtpm_proxy_fops_set_locality() causes kernel buffers to be passed to
> > copy_from_user() and copy_to_user().
> 
> And what is the problem with that? Is it not working?

It is API contract and also clearly documented in the kernel documentation.

This should be obvious even if you have've consulted that documentation because
both functions have 'user' suffix, and also the pointer is __user tagged.

To make things worse it is architecture specific. I'm worried that it will
break in one of the 23 microarchitectures. Have you actually ever checked it
does not?

I'm not also an expert of how all the possible CPUs in the world empower
Linux to further restrict the move between different memory spaces. I'm
quite sure that this does conflict neither with SMAP or SMEP on x86
(because I know x86 pretty well), but who knows what they add in the
future to the microarchitecture.

> > Factor out the crippled code away with help of an internal API for
> > managing struct proxy_dev instances.
> 
> What is crippled code?

Code that behaves badly, i.e. does not meat the expectations. Illegit use of
in-kernel functions easily fits to the definition of crippled code.

Bad API behavior put aside, it is very inefficient implementation because it
unnecessarily recurses tpm_transmit(), which makes extending the driver to
any direction so much involved process, but we don't really need this as a
rationale.

This needs to be fixed in a way or another. That is dictated by the API
cotract so for that I do not really even need feedback because it is
force majeure. I'm cool with alternatives or suggestions to the current
fact, so please focus on that instead of asking question that kernel
documentation provides you already all the answers.

BR, Jarkko