Re: [PATCH 1/2] Introduce copy_user_handle_tail routine

[Andi Kleen <andi@firstfloor.org>]

Vitaly Mayatskikh wrote:
> Andi Kleen <andi@firstfloor.org> writes:
> 
>>>>>> Overall you could write it much simpler with a rep ; movs I think,
>>>>>> like traditional linux did.
>>>> rep movs can fail.
>> How? (if it's a byte copy?)
> 
> Parameter len is a number of uncopied bytes,

But that is exactly what copy_*_user wants to return

 it doesn't count bytes
> which were loaded into registers before GPF in unrolled
> loop. copy_user_handle_tail tries to do a byte copy for, possibly,
> remaining bytes, but it can fail at the first read/write, or at the
> second, etc. It doesn't know where it will fail.

The original version I wrote returned "unfaulted bytes" which was wrong.
Correct is "uncopied" as fixed by Linus. rep ; movs returns uncopied.

> 
>>>>>> I think a simple memset would be actually ok, i don't think we ever zero
>>>>>> anything that faults. That would be obviously racy anyways. If the zero
>>>>>> are supposed to override something  then a racing user thread could always
>>>>>> catch it.
>>>> Linus wanted this routine to be extremely dumb. This is the reason why tail
>>>> handling was moved from assembly to C. Yeah, my original patches were in
>>>> assembly and on the top of your realization.
>> My point was that it could be simpler because zeroing should not ever fault
>> (copy_in_user is not supposed to zero)
> 
> Why do you think that zeroing can never fail, even in userspace?

There's no zeroing in user space, only in kernel space.

The only reason kernel does it is to avoid leaking uninitialized data,
but for user space it doesn't make sense (see above)

-Andi