Le 19/01/2021 à 03:11, Michael Ellerman a écrit :
"Christopher M. Riedl" <c...@codefail.de> writes:
On Mon Jan 11, 2021 at 7:22 AM CST, Christophe Leroy wrote:
Le 09/01/2021 à 04:25, Christopher M. Riedl a écrit :
Implement raw_copy_from_user_allowed() which assumes that userspace read
access is open. Use this new function to implement raw_copy_from_user().
Finally, wrap the new function to follow the usual "unsafe_" convention
of taking a label argument.
I think there is no point implementing raw_copy_from_user_allowed(), see
https://github.com/linuxppc/linux/commit/4b842e4e25b1 and
https://patchwork.ozlabs.org/project/linuxppc-dev/patch/8c74fc9ce8131cabb10b3e95dc0e430f396ee83e.1610369143.git.christophe.le...@csgroup.eu/
You should simply do:
#define unsafe_copy_from_user(d, s, l, e) \
unsafe_op_wrap(__copy_tofrom_user((__force void __user *)d, s, l), e)
I gave this a try and the signal ops decreased by ~8K. Now, to be
honest, I am not sure what an "acceptable" benchmark number here
actually is - so maybe this is ok? Same loss with both radix and hash:
| | hash | radix |
| ------------------------------------ | ------ | ------ |
| linuxppc/next | 118693 | 133296 |
| linuxppc/next w/o KUAP+KUEP | 228911 | 228654 |
| unsafe-signal64 | 200480 | 234067 |
| unsafe-signal64 (__copy_tofrom_user) | 192467 | 225119 |
To put this into perspective, prior to KUAP and uaccess flush, signal
performance in this benchmark was ~290K on hash.
If I'm doing the math right 8K is ~4% of the best number.
It seems like 4% is worth a few lines of code to handle these constant
sizes. It's not like we have performance to throw away.
Or, we should chase down where the call sites are that are doing small
constant copies with copy_to/from_user() and change them to use
get/put_user().
I have built pmac32_defconfig and ppc64_defconfig with a BUILD_BUG_ON(__builtin_constant_p(n) && (n
== 1 || n == 2 || n == 4 || n == 8) in raw_copy_from_user() and raw_copy_to_user():
On pmac32_defconfig, no hit.
On ppc64_defconfig, two hits:
- copies of sigset_t in signal64. This problem is only on linux/next. On next-test we don't have
this problem anymore thanks to the series from Christopher.
- in pkey_set() in arch/powerpc/kernel/ptrace/ptrace-view.c, in the copy of new_amr. This is not a
hot path I think so we can live with it.
Christophe
