Le 18/04/2020 à 13:55, Eric W. Biederman a écrit :
Christophe Leroy <christophe.leroy@xxxxxx> writes:
Le 17/04/2020 à 23:09, Eric W. Biederman a écrit :
To remove the use of set_fs in the coredump code there needs to be a
way to convert a kernel siginfo to a userspace compat siginfo.
Call that function copy_siginfo_to_compat and factor it out of
copy_siginfo_to_user32.
I find it a pitty to do that.
The existing function could have been easily converted to using
user_access_begin() + user_access_end() and use unsafe_put_user() to copy to
userspace to avoid copying through a temporary structure on the stack.
With your change, it becomes impossible to do that.
I don't follow. You don't like temporary structures in the coredump
code or temporary structures in copy_siginfo_to_user32?
In copy_siginfo_to_user32()
A temporary structure in copy_siginfo_to_user is pretty much required
so that it can be zeroed to guarantee we don't pass a structure with
holes to userspace.
Why ? We can zeroize the user structure directly, either with
clear_user() or with some not yet existing unsafe_clear_user() or
equivalent.
The implementation of copy_siginfo_to_user32 used to use the equivalent
of user_access_begin() and user_access_end() and the code was a mess
that was very difficult to reason about. I recall their being holes
in the structure that were being copied to userspace.
Meanwhile if you are going to set all of the bytes a cache hot temporary
structure is quite cheap.
But how can we be sure it is cache hot ? As we are using memset() to
zeroize it, it won't be loaded from memory as it will use dcbz
instruction, but at some point in time it will get flushed back to
memory, that's consuming anyway. Unless we invalidate it after the copy,
but that becomes complex.
Christophe
