Skip to content

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

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 非法或回调崩溃导致持锁进程崩溃并把共享自旋锁挂住

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions