Re: [PATCH 8/8] exec: open code copy_string_kernel

From: Christophe Leroy
Date: Sun Apr 19 2020 - 05:44:23 EST




Le 19/04/2020 Ã 10:06, Christoph Hellwig a ÃcritÂ:
On Sat, Apr 18, 2020 at 10:15:42AM +0200, Christophe Leroy wrote:


Le 14/04/2020 Ã 09:01, Christoph Hellwig a ÃcritÂ:
Currently copy_string_kernel is just a wrapper around copy_strings that
simplifies the calling conventions and uses set_fs to allow passing a
kernel pointer. But due to the fact the we only need to handle a single
kernel argument pointer, the logic can be sigificantly simplified while
getting rid of the set_fs.


Instead of duplicating almost identical code, can you write a function that
takes whether the source is from user or from kernel, then you just do
things like:

if (from_user)
len = strnlen_user(str, MAX_ARG_STRLEN);
else
len = strnlen(str, MAX_ARG_STRLEN);


if (from_user)
copy_from_user(kaddr+offset, str, bytes_to_copy);
else
memcpy(kaddr+offset, str, bytes_to_copy);

We'll need two different str variables then with and without __user
annotations to keep type safety. And introduce a branch-y and unreadable
mess in the exec fast path instead of adding a simple and well understood
function for the kernel case that just deals with the much simpler case
of just copying a single arg vector from a kernel address.


About the branch, I was expecting GCC to inline and eliminate the unused branch.