linux-kernel - Re: [PATCH 1/2] Introduce copy_user_handle

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 02 Jul 2008 17:40:49 +0200
From:	Andi Kleen <andi@...stfloor.org>
To:	Vitaly Mayatskikh <v.mayatskih@...il.com>
CC:	linux-kernel@...r.kernel.org,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: [PATCH 1/2] Introduce copy_user_handle_tail routine

Vitaly Mayatskikh wrote:
> Andi Kleen <andi@...stfloor.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


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/