Skip to content

Commit 18cd218

Browse files
authored
Fix acting[] array out-of-bounds access in radostrace (#87)
* Fix acting[] array out-of-bounds access in radostrace The acting[] array in client_op_v was defined with a fixed size of 6, but the loop iterating over it used a hardcoded bound of 8, causing out-of-bounds reads. Beyond fixing the mismatch, the array was too small for real Ceph deployments. Ceph caps the number of OSDs per PG at CEPH_PG_MAX_SIZE (16), defined in src/include/rados.h. This applies to both replicated pools (max size=10 enforced by OSDMonitor) and erasure-coded pools (k+m bounded by CEPH_PG_MAX_SIZE). The Linux kernel client uses the same constant to size its acting set arrays (struct ceph_osds in osdmap.h). Changes: - Define MAX_ACTING=16 macro in bpf_ceph_types.h, matching CEPH_PG_MAX_SIZE from Ceph's rados.h - Use MAX_ACTING for acting[] array size and all loop bounds - Move initialize_value() stack allocation to a .bss global (zero_val) to avoid BPF 512-byte stack limit, since client_op_v with acting[16] is ~434 bytes Signed-off-by: Seyeong Kim <seyeong.kim@canonical.com> * Fix ringbuf reserve size mismatch in uprobe_finish_op bpf_ringbuf_reserve() was using sizeof(struct op_v) but the code copies a struct client_op_v into the reserved buffer. With the acting[] array expansion this became a detectable OOB write caught by the BPF verifier. Signed-off-by: Seyeong Kim <seyeong.kim@canonical.com> --------- Signed-off-by: Seyeong Kim <seyeong.kim@canonical.com>
1 parent 4f3d470 commit 18cd218

3 files changed

Lines changed: 9 additions & 7 deletions

File tree

src/bpf_ceph_types.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@
1212
#define MSG_OSD_EC_READ 110
1313
#define MSG_OSD_EC_READ_REPLY 111
1414

15+
#define MAX_ACTING 16
16+
1517
#define CEPH_OSD_FLAG_READ 0x0010
1618
#define CEPH_OSD_FLAG_WRITE 0x0020
1719

@@ -54,7 +56,7 @@ struct client_op_v {
5456
char object_name[128];
5557
__u64 m_pool;
5658
__u32 m_seed;
57-
int acting[6];
59+
int acting[MAX_ACTING];
5860
__u64 offset;
5961
__u64 length;
6062
__u16 ops[3];

src/radostrace.bpf.c

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -39,10 +39,10 @@ const volatile __u32 CEPH_OSD_OP_BUFFER_CARRIAGE_OFFSET = 0;
3939
const volatile __u32 CEPH_OSD_OP_BUFFER_RAW_OFFSET = 0;
4040
const volatile __u32 CEPH_OSD_OP_BUFFER_DATA_OFFSET = 0;
4141

42+
static struct client_op_v zero_val = {};
43+
4244
void initialize_value(struct client_op_k key) {
43-
struct client_op_v val;
44-
memset(&val, 0, sizeof(val));
45-
bpf_map_update_elem(&ops, &key, &val, 0);
45+
bpf_map_update_elem(&ops, &key, &zero_val, 0);
4646
}
4747

4848
SEC("uprobe")
@@ -198,7 +198,7 @@ int uprobe_send_op(struct pt_regs *ctx) {
198198
return 0;
199199
}
200200

201-
for (int i = 0 ; i < 8; ++i) {
201+
for (int i = 0 ; i < MAX_ACTING; ++i) {
202202
val->acting[i] = -1;
203203
if (M_start < m_finish) {
204204
bpf_probe_read_user(&(val->acting[i]), sizeof(int), (void *)M_start);
@@ -320,7 +320,7 @@ int uprobe_finish_op(struct pt_regs *ctx) {
320320
opv->finish_stamp = bpf_ktime_get_boot_ns();
321321
opv->pid = get_pid();
322322
// submit to ringbuf
323-
struct client_op_v *e = bpf_ringbuf_reserve(&rb, sizeof(struct op_v), 0);
323+
struct client_op_v *e = bpf_ringbuf_reserve(&rb, sizeof(struct client_op_v), 0);
324324
if (NULL == e) {
325325
return 0;
326326
}

src/radostrace.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -282,7 +282,7 @@ static int handle_event(void *ctx, void *data, size_t size) {
282282
acting_osd_list << "[";
283283
{
284284
bool first = true;
285-
for (int i = 0; i < 8; ++i) {
285+
for (int i = 0; i < MAX_ACTING; ++i) {
286286
if (op_v->acting[i] < 0) break;
287287
if (!first) acting_osd_list << ",";
288288
acting_osd_list << op_v->acting[i];

0 commit comments

Comments
 (0)