Re: [PATCH 05/14 v6] Open-code the memcpy() at static TLS initialization time.

[Florian Weimer <fweimer at redhat dot com>]

On 06/07/2016 01:06 PM, Nix wrote:
> From: Nick Alcock <nick.alcock@oracle.com>
>
> This one is a bit nasty.  Now that we are initializing TLS earlier for
> the stack canary's sake, existing memcpy() implementations become
> problematic.  We can use the multiarch implementations, but they might
> not always be present, and even if they are present they might not always
> be in assembler, so might be compiled with stack-protection.  We cannot
> use posix/memcpy.c without marking both it and */wordcopy.c as non-stack-
> protected, which for memcpy() of all things seems like a seriously bad
> idea: if any function in glibc should be stack-protected, it's memcpy()
> (though stack-protecting the many optimized assembly versions is not done
> in this patch series).

Some of the pathnames in this explanation appear to be incorrect.

I looked at the memcpy.c and wordcopy.c implementations, and I do not 
see why these functions would need the stack protector.  They should not 
have addressable local variables, they should all be in registers.  As a 
result, the implementation should never derive a pointer from the 
current stack frame.

These functions might be passed pointers into the callers stack frame, 
but the memory looks like this (addresses increase from bottom to top, 
so the stack grows downwards on most architectures (except HPPA)):

    :                             :
    +-----------------------------+
    |  Caller stack frame         |
    |                             |
    |  (end of buf)               |
    |    :                        |
    |    :                        |
    |  char buf[64];              |
    |                             |
    +-----------------------------+
    | Return address              |
    | memcpy stack frame          |
    |                             |
    +-(current top of stack)------+
    :                             :
    +=============================+
    | XXX guard page XXXXXXXXXXXX |
    :                             :


As a result, a buffer overflow will run away from the return address 
associated with the memcpy activation frame.  To avoid that, you'd need 
a memcpy call with a destination address which points below the current 
top of stack before memcpy is called, that is an invalid, dangling 
pointer.  A random pointer with an excessive size is unlikely to work 
because the copying operation would eventually hit the guard page below 
the stack.  This is not a straight stack-based overflow anymore, and not 
something stack protector can help with.

(Attacks against runaway memcpy calls (with size arguments approaching 
SIZE_MAX) work against some memcpy implementations not because these 
implementations lack stack protector, but because they load variables 
from the stack in the inner loop, which may have been overwritten by 
attacker-controlled values.)

Anyway, if the above analysis is right, it should be safe to disable 
stack protector for functions such as memcpy (essentially doing manually 
what -fstack-protector-strong does automatically).  This would be my 
preferred approach here.

Thanks,
Florian