问题描述
- 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 会将任意用户代码置于临界区,若回调阻塞或崩溃会造成同样的锁悬挂问题。
建议修复
- 在 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;
- 在 so/data-map.cpp 顶部添加必要的 includes 与常量:
- #include
- #include
- static const size_t MAX_TMP_COPY = 32 * 1024 * 1024; // 32MB cap(可配置)
- 实现 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;
}
- 替换 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 非法或回调崩溃导致持锁进程崩溃并把共享自旋锁挂住
问题描述
代码位置
文件:
./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; }触发条件
预期行为
实际行为
建议修复
影响范围
可能因用户 buf 非法或回调崩溃导致持锁进程崩溃并把共享自旋锁挂住