glibc malloc guard byte wrapper

I'm trying to add a guard char at the very end of each allocated chunk so that free() can abort() if it does not find it. Why are these function preloads not working? I realize this is not a portable method but I was curious why it is not working.

gcc -shared -fPIC -std=gnu99 -O2 -o wrapper.so wrapper.c

LD_PRELOAD=/path/to/wrapper.so programname

I have one function for each: valloc, realloc, pvalloc, posix_memalign, aligned_alloc, memalign, malloc and calloc.

#define PREV_INUSE 0x1
#define IS_MMAPPED 0x2
#define NON_MAIN_ARENA 0x4
#define SIZE_BITS (PREV_INUSE|IS_MMAPPED|NON_MAIN_ARENA)


extern void *__libc_malloc(size_t size);
void *malloc(size_t size){
    size = size + 1; // add char for guard byte '@'

    void *p = __libc_malloc(size);

    if(p != NULL){
        size_t *q = p;
        q--;
        size_t s = *q & ~(SIZE_BITS); // get allocated bytes subtracting info bits
        char *z = p;
        memset(z, 0, s); // zero memory
        z[s - 1] = '@'; // place guard char
    }

    return p;
}

extern void *__libc_free(void *ptr);
void free(void *ptr){
    if(ptr != NULL){
        size_t *p = ptr;
        p--;
        size_t s = *p & ~(SIZE_BITS);
        char *z = ptr;
        if(z[s - 1] != '@') // if guard char not found, abort()
        {
            abort();
        }
        memset(z, 0, s); // zero memory
    }

    __libc_free(ptr);
}

Solution

You are using the size_t located just before the allocated area as the length available. However, it includes the size_t itself. Therefore, here:

    if (p != NULL) {
        size_t *q = p;
        q--;
        size_t s = *q & ~(SIZE_BITS); // get allocated bytes subtracting info bits
        char *z = p;
        memset(z, 0, s); // zero memory
        z[s - 1] = '@'; // place guard char
    }

you end up overwriting the length of the next region, partially, by your guard char. The solution is to substract the length of the length field in bytes, i.e. use const s = (((size_t *)p)[-1] & (~(size_t)SIZE_BITS)) - sizeof (size_t); instead.

(I verified this works on Embedded GNU C Library 2.15-0ubuntu10.15 on x86-64, for both 64 and 32-bit code (with different size_t sizes).)

I recommend you add at least minimal abstraction, so that porting your code to a different C library or a newer version of GNU C library in the future is not futile. (Version checking would be good, but I was too lazy to find out which versions of GNU C library actually use this layout.)

#include <string.h>
#include <limits.h>
#ifdef __GLIBC__

/* GLIBC stuffs the length just prior to the returned pointer,
 * with flags in the least significant three bits. It includes
 * the length field itself. */
#define   USER_LEN(ptr) ( ( ((size_t *)(ptr))[-1] & (~((size_t)7)) ) - sizeof (size_t))

#else
#error This C library is not supported (yet).
#endif

extern void  abort(void);
extern void *__libc_malloc(size_t);
extern void *__libc_realloc(void *, size_t);
extern void  __libc_free(void *);


#define CANARY_LEN 1

static void canary_set(void *const ptr, const size_t len)
{
    ((unsigned char *)ptr)[len - CANARY_LEN] = '@';
}

static int canary_ok(const void *const ptr, const size_t len)
{
    return ((const unsigned char *)ptr)[len - CANARY_LEN] == '@';
}


void *malloc(size_t size)
{
    void *ptr;
    ptr = __libc_malloc(size + CANARY_LEN);
    if (ptr) {
        const size_t len = USER_LEN(ptr);
        memset(ptr, 0, len);
        canary_set(ptr, len);
    }
    return ptr;
}

void *realloc(void *ptr, size_t size)
{
    void *newptr;

    if (!ptr)
        return malloc(size);

    if (!canary_ok(ptr, USER_LEN(ptr)))
        abort();

    newptr = __libc_realloc(ptr, size + CANARY_LEN);
    if (!newptr)
        return newptr;

    canary_set(newptr, USER_LEN(ptr));

    return newptr;
}

void free(void *ptr)
{
    if (ptr) {
        const size_t len = USER_LEN(ptr);

        if (!canary_ok(ptr, len))
            abort();

        memset(ptr, 0, len);

        __libc_free(ptr);
    }
}

Hope this helps.