Skip to content

Bug: int DataMap::find中如果buf传入非法地址,导致异常访问段错误时, 共享自旋锁没有解锁。 #69

@yuKing123-king

Description

@yuKing123-king

问题描述

  • DataMap::find 实现里,当前代码在持有跨进程共享自旋锁(m_lock)期间直接把 ring-buffer 中的数据 memcpy 到调用者提供的 buf,或直接在锁内调用用户回调。这会导致两类严重问题
    • 如果 memcpy 写入非法地址(调用者传入错误的 buf)会触发 SIGSEGV 导致持锁进程崩溃,从而共享自旋锁长期处于“已持有”状态,阻塞其他进程 。
    • 在锁内调用用户回调(m_user_cb)会把任意用户代码运行在锁保护区内,可能阻塞或崩溃,同样造成锁悬挂风险。
  • 当 cnt > idx 时 st 为负值,隐式转换为 ulong 导致回绕为接近 ULONG_MAX 的大值,循环条件与数组访问会异常,可能访问 m_entrys 的越界内存或产生难以调试的行为(DEBUG 打印也会误导,因为使用 %lu 打印 long)。

代码位置

文件: ./so/data-map.cpp
函数: DataMap::find()
行号: 525-548

{
	/**
	 * TODO: buf传入非法地址,导致异常访问段错误时,
	 * 共享自旋锁没有解锁。
	 */
	int ret;
	SpinLockGuard lock_util_exit(m_lock);
	ret = unsafe_find(hash, lifetime, buf, bsz);
	if (ret > 0)
	{
		return ret;
	}
	/**
	 * update data and try again
	 */
	ret = update(KEY_DT(hash));
	if (ret != 0)
	{
		return ret;
	}
	ret = unsafe_find(hash, lifetime, buf, bsz);
	return ret;
}

触发条件

  • 用户态调用 read/find,并传入非法 buf 指针(未映射或已 munmap)。
  • 用户回调(m_user_cb)在锁内执行并阻塞或崩溃。
  • 在并发/竞态情形下,读到半写入的数据导致 dh->dsz 不可靠,也会触发错误行为。

预期行为

  • 任何情况下都不应在持有跨进程共享自旋锁(m_lock)时访问用户空间指针或执行用户代码。
  • 参数校验:若 buf == nullptr 且 m_user_cb == nullptr 则返回 -EINVAL。
  • 缓冲不足:若合并后数据总大小 total_sz > bsz 返回 -ENOBUFS(不在锁内做部分写入)。
  • 单次收集过大(防止 OOM):若采集总大小超过可配置上限(例如 MAX_TMP_COPY),返回 -E2BIG 或 -ENOMEM,并记录警告日志。
  • 若找不到匹配条目,返回 -ENOENT。

实际行为

  • 获取 SpinLockGuard lock_util_exit(m_lock) 后直接调用 unsafe_find() / sub_iterator();这些函数在锁保护下会直接用 memcpy(buf, dh, dh->dsz) 将数据写入调用者提供的 buf,或在锁内调用 m_user_cb。
  • 若 buf 为非法地址或已被 munmap,memcpy 会触发 SIGSEGV 导致持锁进程崩溃。由于 m_lock 是放在共享内存的自旋锁(不是 robust mutex),崩溃的进程不会把锁正确释放,从而使其它进程在尝试获取该锁时永久阻塞,产生系统级 DoS。
  • 在锁内执行 m_user_cb 会将任意用户代码置于临界区,若回调阻塞或崩溃会造成同样的锁悬挂问题。

建议修复

  1. 在 DataMap 类(so/data-map.h)新增私有 helper 声明:
    • int collect_entries(ulong hash, ulong lifetime, std::vector<std::vector<uint8_t>> &out_entries, size_t &total_sz) const;
    • int sub_iterator_collect(ulong si, std::vector<std::vector<uint8_t>> &out_entries, size_t &total_sz) const;
  2. 在 so/data-map.cpp 顶部添加必要的 includes 与常量:
    • #include
    • #include
    • static const size_t MAX_TMP_COPY = 32 * 1024 * 1024; // 32MB cap(可配置)
  3. 实现 collect_entries/sub_iterator_collect:
    • 在锁内把 ringbuffer 中的每个匹配条目(含 DataHdr)复制到 std::vector<uint8_t> 并 push 到 out_entries;
    • 若累计大小超过 MAX_TMP_COPY,则返回 -E2BIG;
    • 对 ringbuffer 有效性/过期检查保持不变。
    • 实现代码示例
int DataMap::sub_iterator_collect(ulong idx, std::vector<std::vector<uint8_t>> &out_entries, size_t &total_sz) const
{
    idx &= (m_ent_cnt - 1);
    idx += m_ent_cnt;
    const AddrEntry &entry = m_entrys[idx];
    ulong cnt = entry.dsz;
    DKapture::DataType dt = KEY_DT(entry.hash);
    long st = idx - cnt;
    total_sz = 0;
    for (ulong i = st; i < idx; i++)
    {
        if (KEY_DT(m_entrys[i].hash) != dt)
            continue;
        DKapture::DataHdr *dh = (typeof(dh))m_bpf_rb->buf(m_entrys[i].data_idx);
        if (!dh)
            continue;
        if (total_sz + dh->dsz > MAX_TMP_COPY)
        {
            pr_error("collect size exceed limit %zu", MAX_TMP_COPY);
            return -E2BIG;
        }
        std::vector<uint8_t> item;
        item.reserve(dh->dsz);
        uint8_t *p = (uint8_t *)dh;
        item.insert(item.end(), p, p + dh->dsz);
        out_entries.emplace_back(std::move(item));
        total_sz += dh->dsz;
    }
    return (int)total_sz;
}
int DataMap::collect_entries(ulong hash, ulong lifetime, std::vector<std::vector<uint8_t>> &out_entries, size_t &total_sz) const
{
    // 确保在锁保护下调用
    assert(m_lock->try_lock() == false);
    long sidx = get_round_idx();
    long eidx = sidx + m_ent_cnt - 1;
    lifetime *= 1000000UL;
    struct timespec ts = {};
    DKapture::DataHdr *dh;
    clock_gettime(CLOCK_MONOTONIC, &ts);
    ulong now = TIME_ns(ts);
    ulong bpf_bsz = m_bpf_rb->get_bsz();
    ulong prod_idx = m_bpf_rb->get_producer_index();
    total_sz = 0;
    for (long i = eidx; i >= sidx; i--)
    {
        if (m_entrys[i].hash == 0)
            continue;
        ulong rb_idx = m_entrys[i].data_idx;
        if (unlikely(rb_idx + bpf_bsz < prod_idx))
        {
            DEBUG(0, "expired data idx: %lu, prod_idx: %lu, bpf_bsz: %lu", rb_idx, prod_idx, bpf_bsz);
            break;
        }
        if (likely(m_entrys[i].hash != hash))
            continue;
        if (m_entrys[i].time + lifetime < now)
        {
            DEBUG(0, "data out of date");
            return -ETIME;
        }
        if (KEY_PID(hash) == 0)
        {
            int ret = sub_iterator_collect(i, out_entries, total_sz);
            return ret;
        }
        dh = (typeof(dh))m_bpf_rb->buf(m_entrys[i].data_idx);
        if (!dh) continue;
        if (total_sz + dh->dsz > MAX_TMP_COPY)
        {
            pr_error("collect_entries: total size exceed limit %zu", MAX_TMP_COPY);
            return -E2BIG;
        }
        std::vector<uint8_t> item;
        item.reserve(dh->dsz);
        uint8_t *p = (uint8_t *)dh;
        item.insert(item.end(), p, p + dh->dsz);
        out_entries.emplace_back(std::move(item));
        total_sz += dh->dsz;
         Keep the behavior of the previous per-pid read: stop when it matches to the first one
        break;
    }
    return out_entries.empty() ? -ENOENT : (int)total_sz;
}
  1. 替换 DataMap::find
    • 参数校验(buf != nullptr 或 m_user_cb 非空,且若 buf 非空 bsz>0);
    • 在 SpinLockGuard 作用域内调用 collect_entries(...) 将数据收为 out_entries;
    • 退出锁作用域(释放锁);
    • 在锁外:若 buf 非空,检查 total_sz <= bsz 并 memcpy 出来的内容;否则在锁外逐条调用 m_user_cb(将回调结果透传)。
    • 实现代码示例
int DataMap::find(ulong hash, ulong lifetime, void *buf, size_t bsz)
{
    if (buf == nullptr && m_user_cb == nullptr)
        return -EINVAL;
    if (buf != nullptr && bsz == 0)
        return -EINVAL;
    std::vector<std::vector<uint8_t>> out_entries;
    size_t total_sz = 0;
    {
        SpinLockGuard lock_util_exit(m_lock);
        int ret = collect_entries(hash, lifetime, out_entries, total_sz);
        if (ret < 0)
            return ret;
    } // lock released here
    if (out_entries.empty())
        return -ENOENT;
    if (buf)
    {
        if (total_sz > bsz)
        {
            pr_error("dkapture::read: buffer size is too small, %zu needed, %lu provided", total_sz, bsz);
            return -ENOBUFS;
        }
        size_t off = 0;
        for (const auto &item : out_entries)
        {
            memcpy((char *)buf + off, item.data(), item.size());
            off += item.size();
        }
        return (ssize_t)total_sz;
    }
    if (m_user_cb)
    {
        for (const auto &item : out_entries)
        {
            int cbret = m_user_cb(m_user_ctx, (void *)item.data(), item.size());
            if (cbret != 0)
                return cbret;
        }
        return (ssize_t)total_sz;
    }
    return -EINVAL;
}

影响范围

可能因用户 buf 非法或回调崩溃导致持锁进程崩溃并把共享自旋锁挂住

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions