Is this macro safe for copying a string?

Question

Is this macro safe or should I make sure alloca never returns NULL?

#define DO_COPY(x) strcpy(alloca(strlen((x)) + 1), x)

alloca()? if it's malloc, I would say yes, you must check it. — जलजनक, Oct 18 '12 at 02:59
See http://stackoverflow.com/questions/1018853/why-is-alloca-not-considered-good-practice - if you use alloca you need to be pretty sure about what you are doing... — John3136, Oct 18 '12 at 02:59
It entirely depends on the context in which you are using it. — Jim Buck, Oct 18 '12 at 07:32

Art · Accepted Answer · 2012-10-18T07:34:11.800

If the string is user controlled I'd say that alloca is not safe. The way alloca is implemented in many compilers it doesn't do any kind of sanity checking on the amount it subtracts from (or adds to if your stack grows that way) the stack pointer. Even with large red zones around the stack it would be relatively easy to make the alloca():ed string to point way outside the stack.

Especially in a threaded environment the thread stacks can be quite small and close to each other.

On the linux machines I could test this on it would require a 10MB string to do start scribbling on some other threads stack with this. On MacOS 512kB seems to be enough.

Here's a quick hack to see how close you can end up (notice that this doesn't really tell you much if the stack allocation is done using some randomizing allocator like in OpenBSD or some other system that takes their allocator safety seriously).

#include <stdlib.h>
#include <stdio.h>
#include <pthread.h>
#include <stdint.h>

void *
x(void *v)
{
    int x;

    return &x;
}

int
main(int argc, char **argv)
{
    pthread_t ta, tb;
    char *va, *vb;

    pthread_create(&ta, NULL, x, NULL);
    pthread_create(&tb, NULL, x, NULL);

    pthread_join(ta, (void **)&va);
    pthread_join(tb, (void **)&vb);

    printf("diff: %d\n", abs((intptr_t)vb-(intptr_t)va));

    return 0;
}

And here is what strcpy(alloca(strlen(s) + 1), s) gets compiled to:

    movq    %rbx, %rdi
    call    _strlen
    addq    $31, %rax
    andq    $-16, %rax
    subq    %rax, %rsp
    movq    %rsp, %rdi
    movq    %rbx, %rsi
    call    _strcpy

Notice how there's no sanity checking other than quick alignment before subtracting the return value from strlen (in %rax) from the stack pointer.

I asked a question related to your concern earlier on StackOverflow: http://stackoverflow.com/questions/5543330/ . The answers there led me to believe that for desktop OSes and proper compilers, the situation is not as bleak as you say. Is there any reason the compiler does not generate code that accesses the newly stack-allocated pages in order together with `alloca()`? — Pascal Cuoq, Oct 18 '12 at 07:31
@PascalCuoq I just checked both gcc and clang on MacOS and two different flavors of Linux and all of them skip any kind of sanity checking. There might be flags to enable checking, but safety features in compilers are useless if they aren't enabled by default (and hard to disable). — Art, Oct 18 '12 at 07:47
Could I easily replace my code to use malloc instead? Most people I talked to tell me to avoid using alloca at all costs. — user1621581, Oct 18 '12 at 10:01

Is this macro safe for copying a string?

1 Answers1