skynet icon indicating copy to clipboard operation
skynet copied to clipboard

fix *** buffer overflow detected *** problem with gcc-13+ or -D_FORTIFY_SOURCE=3

Open EfveZombie opened this issue 9 months ago • 8 comments

gcc-13 及以上版本会默认开启 -D_FORTIFY_SOURCE=3 (原配置为 -D_FORTIFY_SOURCE=2), 在开启 O1 以上编译优化的时候编译器可能对 memcpy 插入检查, 发生越界误判 (malloc_usable_size(ptr) 返回的大小要大于实际 malloc 大小)

崩溃堆栈: image

最小复现代码:

// test.c

#include <malloc.h>
#include <stdint.h>
#include <string.h>

size_t size = 32;

struct mem_cookie {
	uint32_t handle;
};

int main(void) {
    void *ptr = malloc (size);
    size_t usable_size = malloc_usable_size(ptr); // bigger than size

    struct mem_cookie *p = ptr + usable_size - sizeof(struct mem_cookie);

    uint32_t handle = 1234;
    printf("size:%lu usable_size:%lu p:%p\n", size, usable_size, p);

    // *** buffer overflow detected ***: terminated
    memcpy(&p->handle, &handle, sizeof(handle));
    printf("handle %d\n", p->handle);
    return 0;
}

编译方式:

gcc-13 -O2 -o test test.c

或者

clang -O2 -o test test.c -D_FORTIFY_SOURCE=3

相关: https://bugs.launchpad.net/ubuntu/+source/gcc-13/+bug/2012440

EfveZombie avatar May 09 '24 07:05 EfveZombie

这里不可以把 memcpy 改为变量赋值。因为这里计算出来的地址未必是对齐的,对不对齐的地址写数据,并非所有平台都是正确的。

另外,如果你用的平台 malloc_usable_size 行为和预期不一致,这里的修改也没有修复你说的问题。你只是把检查关掉了而已。

skynet 这里的代码使用的是 je_malloc_usable_size ,它的行为应该是确定的。

cloudwu avatar May 09 '24 09:05 cloudwu

我试了一下上面的 test.c ,在我的系统上并没有报告错。

$ gcc --version
gcc (GCC) 13.2.1 20230801

$ gcc -O2 -o test test.c
$ ./test 
size:32 usable_size:40 p:0x564c56de42c4
handle 1234


cloudwu avatar May 09 '24 09:05 cloudwu

我这边出错的平台是 WSL2 下的 Ubuntu 20.04 和 Ubuntu 24.04, 上面这个正常运行的结果是在 win 上试出来的吗, 看起来 malloc_usable_size 返回的大小是 32 和 malloc 时传入的大小时一致的所以没有导致报错吧, 我这边 alloc(32) 再 malloc_usable_size 拿到的 size 是 40

补充一下相关材料: https://bugs.launchpad.net/ubuntu/+source/gcc-13/+bug/2012440

EfveZombie avatar May 09 '24 12:05 EfveZombie

单从 skynet 内看的话这边应该都是内存对齐的, 不过可能外部调用就没有保障了?

感觉比较合理的改法应该是不依赖 je_malloc_usable_size, 像这个函数名 fill_perfix 描述的那样把 handle 填到头部去, 不过也不好处理这么改之后导致的内存对齐问题...

或者我这里可以先加个 volatile 避免编译器的优化

EfveZombie avatar May 09 '24 12:05 EfveZombie

我在 manjaro 上。而且 size:32 usable_size:40 就是不一致的。而且我认为 malloc_usable_size 的语义就是可以使用的内存,所以我认为你碰到了别的问题。

而且 je_malloc_usable_size 是 skynet 编译的 jemalloc 内部函数,它并不是 malloc_usable_size 不受 crt/libc 的影响。

如果这个 usable_size 实际被 crt 占用,那么你这么改也会破坏它。如果它其实是可用的,那么就是 crt 的误报,属于 gcc or crt 的 bug 。

cloudwu avatar May 09 '24 13:05 cloudwu

我不清楚你是怎么弄出这个问题出来的,实际上这里 skynet 改写了 malloc ,是自己的实现,理论上不应该受 libc 的检查约束。因为这块内存本身就不是由 libc 直接管理的。

而你如果想直接用 CRT 的 malloc ,那么应该定义宏 NOUSE_JEMALLOC 。这样,以上整个函数都不会被编译进去。

所以,我怀疑你是在编译链接环节出了问题,导致链接了多份不同的 malloc 。

cloudwu avatar May 09 '24 13:05 cloudwu

我看 man 上写

This function is intended to only be used for diagnostics and statistics; writing to the excess memory without first calling realloc(3) to resize the allocation is not supported.

我想想怎么去掉。

cloudwu avatar May 09 '24 14:05 cloudwu

看看是不是改好了。

cloudwu avatar May 09 '24 15:05 cloudwu

skynet_calloc 里 fill_prefix 的第二个参数应该是 (nmemb + cookie_n) * size 吧

BTW, update_xmalloc_stat_alloc 和 update_xmalloc_stat_free 是不是还用 je_malloc_usable_size 统计会比较好?

EfveZombie avatar May 10 '24 03:05 EfveZombie

https://github.com/cloudwu/skynet/commit/6a7043cfac0c64d505351283957c9854165f22db 已改。

我觉得完全去掉 je_malloc_usable_size 更干净。至于统计,这个值本来就是一个参考值,使用 je_malloc_usable_size 也没有准确多少。如果是想查询真实占用,应该使用 jemalloc 提供的 info 信息,目前也可以在 skynet console 查到。

cloudwu avatar May 10 '24 03:05 cloudwu

不应该是 fill_prefix(ptr, n * size, cookie_n * size) 吗 @_@

EfveZombie avatar May 10 '24 04:05 EfveZombie

不应该是 fill_prefix(ptr, n * size, cookie_n * size) 吗 @_@

对。抱歉,在写别的程序,刚才没仔细看。

cloudwu avatar May 10 '24 04:05 cloudwu