Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
wants to merge 1 commit into from

Conversation

EfveZombie
Copy link
Contributor

@EfveZombie EfveZombie commented May 9, 2024

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

@cloudwu
Copy link
Owner

cloudwu commented May 9, 2024

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

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

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

@cloudwu
Copy link
Owner

cloudwu commented May 9, 2024

我试了一下上面的 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


@EfveZombie
Copy link
Contributor Author

我这边出错的平台是 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
Copy link
Contributor Author

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

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

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

@cloudwu
Copy link
Owner

cloudwu commented May 9, 2024

我在 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
Copy link
Owner

cloudwu commented May 9, 2024

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

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

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

@cloudwu
Copy link
Owner

cloudwu commented May 9, 2024

我看 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 added a commit that referenced this pull request May 9, 2024
@cloudwu
Copy link
Owner

cloudwu commented May 9, 2024

看看是不是改好了。

@EfveZombie
Copy link
Contributor Author

EfveZombie commented May 10, 2024

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

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

@cloudwu
Copy link
Owner

cloudwu commented May 10, 2024

6a7043c 已改。

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

@EfveZombie
Copy link
Contributor Author

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

@cloudwu
Copy link
Owner

cloudwu commented May 10, 2024

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

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants