Null Pointer Dereference in rt_smem_setname Function Causes System Crash

[GuoyuYin]

RT-Thread Version

2f55990

Hardware Type/Architectures

arm32

Develop Toolchain

GCC

Describe the bug

Expected Behavior:

The function rt_smem_setname in mem.c should successfully copy the thread name into the memory structure without errors. It should iterate through the name string and properly terminate the loop when encountering a null character ('\0'), ensuring no out-of-bound access or unexpected behavior.

Actual Behavior:

The function rt_smem_setname in mem.c at line 108 encounters an issue where it fails to handle the input string name properly. Specifically, the condition if (name[index] == '\0') break; does not safeguard against cases where name is NULL. This leads to a potential dereference of a null pointer, causing a system crash and halting execution. Additionally, the log indicates a mismatch in returned syscall calls (info.Calls : 22 != ncalls : 30), further suggesting undefined behavior in memory handling.

Description:

During the execution of syz_recvfrom, the system encounters a critical failure leading to a timeout.The issue originates in the rt_smem_setname function at line 108 of mem.c, which lacks a check to ensure the name pointer is non-NULL before dereferencing it. If name is NULL, the condition if (name[index] == '\0') results in undefined behavior, causing the system to crash. This problem is exacerbated by the fact that rt_smem_setname is invoked without sufficient validation of the input arguments, leading to unpredictable behavior during runtime.

The error log highlights a stack trace showing the crash in mem.c at line 108. It further indicates the crash occurred during the execution of a syscall (syz_recvfrom) and subsequent memory operations (rt_smem_free, rt_free), pointing to improper memory handling as the root cause.

Debug Logs:

#executing syz_recvfrom (0x2, 0x0, 0xffffffffffffffff, 0x0, 0xfffffffffffffffc, 0x0)
2024/09/04 12:20:18 Syscall execution is ok
2024/09/04 12:20:18 qemu run inst merger err: execution timed out
2024/09/04 12:20:18 Received stop signal, requires feedback = true
2024/09/04 12:20:18 running diagnose
2024/09/04 12:20:18 VM-0 failed reading regs: dial tcp 127.0.0.1:2991: connect: connection refused
2024/09/04 12:20:18 VM-0 failed reading regs: dial tcp 127.0.0.1:2991: connect: connection refused
2024/09/04 12:20:18 Stack frames at BUG: unexpected stop:
2024/09/04 12:20:18 Level: 1: 1611470188, /root/rtthread/rt-thread/src/mem.c : rt_smem_setname : 108 : 
2024/09/04 12:20:18 Level: 2: 1611474960, /root/rtthread/rt-thread/src/mem.c : rt_smem_free : 526 : 
2024/09/04 12:20:18 Level: 3: 1611469424, /root/rtthread/rt-thread/src/kservice.c : rt_free : 767 : 
2024/09/04 12:20:18 Level: 4: 1610767840, /root/rtthread/rt-thread/components/libc/compilers/newlib/syscalls.c : _free_r : 77 : 
2024/09/04 12:20:18 Level: 5: 1611220452, /root/rtthread/rt-thread/bsp/qemu-vexpress-a9/applications/common_freertos.h : syz_recvfrom : 861 : 
2024/09/04 12:20:18 Level: 6: 1611227220, /root/rtthread/rt-thread/bsp/qemu-vexpress-a9/applications/executor.h : execute_syscall : 365 : 
2024/09/04 12:20:18 Level: 7: 1611230600, /root/rtthread/rt-thread/bsp/qemu-vexpress-a9/applications/executor.h : fuzz_start_one : 558 : 
2024/09/04 12:20:18 Level: 8: 1611230904, /root/rtthread/rt-thread/bsp/qemu-vexpress-a9/applications/executor.h : executor_main : 594 : 
2024/09/04 12:20:18 Level: 9: 1611231056, /root/rtthread/rt-thread/bsp/qemu-vexpress-a9/applications/main.c : main : 24 : 
2024/09/04 12:20:18 [ERROR] Incorrect returned call number: info.Calls : 22 != ncalls : 30 

__Note:kcov.c main.c common_freertos.h executor.h is a file we wrote ourselves.

Steps to Reproduce:

1. Set up the Rt-Thread environment with the necessary configurations for fuzz testing.
2. Compile the RT-Thread kernel with debugging symbols enabled.
3. Use a fuzzer tool such as syzkaller configured to generate random inputs for syscall testing.
4. Execute the syz_recvfrom function with ((0x2, 0x0, 0xffffffffffffffff, 0x0, 0xfffffffffffffffc, 0x0)).

#include <rtthread.h>
#include <stdint.h>

// UDP 数据接收
long syz_recvfrom(volatile int s, volatile int mem, volatile int len, volatile int flags, volatile int from, volatile int fromlen) {
    struct sockaddr *src_addr = (struct sockaddr *)malloc(sizeof(struct sockaddr));
    if (!src_addr) return -1;
    socklen_t *addr_len = (socklen_t *)malloc(sizeof(socklen_t));
    if (!addr_len) {
        free(src_addr);
        return -1;
    }
    *addr_len = (socklen_t)fromlen;
    int result = recvfrom(s, (void *)mem, (size_t)len, flags, src_addr, addr_len);
    free(src_addr);
    free(addr_len);
    return (long)result;
}

5. Observe the behavior; if the bug persists, you should see similar logs indicating a timeout and register access failure.

Suggested Fix

To resolve this issue, modify the rt_smem_setname function to include a null pointer check for the name parameter before accessing it. This ensures the function does not dereference a null pointer, preventing crashes and undefined behavior.

Here is the updated code for rt_smem_setname:

rt_inline void rt_smem_setname(struct rt_small_mem_item *mem, const char *name)
{
    int index;

    // Check if the name pointer is NULL
    if (name == NULL)
    {
        // If NULL, fill the thread name with spaces and return
        for (index = 0; index < sizeof(mem->thread); index++)
        {
            mem->thread[index] = ' ';
        }
        return;
    }

    // Copy the name string into the thread field
    for (index = 0; index < sizeof(mem->thread); index++)
    {
        if (name[index] == '\0') break;
        mem->thread[index] = name[index];
    }

    // Fill remaining space with spaces
    for (; index < sizeof(mem->thread); index++)
    {
        mem->thread[index] = ' ';
    }
}

Additionally, review all calls to rt_smem_setname to ensure that valid pointers are passed wherever possible. This defensive programming practice will reduce the likelihood of similar issues in the future.

Other additional context

No response