From 1c8581e006cb1470f3a09f50253e2db7caa45aa7 Mon Sep 17 00:00:00 2001 From: Dariusz Sosnowski Date: Sat, 5 Sep 2020 01:30:41 +0200 Subject: [PATCH 1/5] record_syscall: Empty stub for prepare_rdma_verbs_ioctl --- src/record_syscall.cc | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/src/record_syscall.cc b/src/record_syscall.cc index 4e40a1d5f3e..720e2f1e2c6 100644 --- a/src/record_syscall.cc +++ b/src/record_syscall.cc @@ -1587,6 +1587,26 @@ template void prepare_ethtool_ioctl(RecordTask* t, TaskSyscallSt syscall_state.after_syscall_action(record_page_below_stack_ptr); } +template +static void prepare_rdma_verbs_ioctl(__attribute__((unused)) RecordTask* t, + __attribute__((unused)) TaskSyscallState& syscall_state) +{ + LOG(warn) << "called prepare_rdma_verbs_ioctl()"; + + auto hdrp = syscall_state.reg_parameter(3, IN); + bool ok = true; + auto hdr = t->read_mem(hdrp, &ok); + if (!ok) { + syscall_state.expect_errno = EFAULT; + return; + } + + LOG(warn) << " hdr->length = " << hdr.length; + LOG(warn) << " hdr->object_id = " << hdr.object_id; + LOG(warn) << " hdr->method_id = " << hdr.method_id; + LOG(warn) << " hdr->num_attrs = " << hdr.num_attrs; +} + template static Switchable prepare_ioctl(RecordTask* t, TaskSyscallState& syscall_state) { @@ -1761,7 +1781,7 @@ static Switchable prepare_ioctl(RecordTask* t, */ case RDMA_VERBS_IOCTL: // TODO: ioctl argument size is dynamically calculated. How to do this in rr? - syscall_state.reg_parameter(3); + prepare_rdma_verbs_ioctl(t, syscall_state); return PREVENT_SWITCH; case SG_IO: From f1fd5a69206c8bd24ef1dbd4413ba7e83be06850 Mon Sep 17 00:00:00 2001 From: Dariusz Sosnowski Date: Sat, 5 Sep 2020 14:01:53 +0200 Subject: [PATCH 2/5] record_syscall: Store ib_uverbs_ioctl_hdr in scratch struct ib_uverbs_ioctl_hdr size is defined dynamically, so on RDMA_VERBS_IOCTL command before registering IN_OUT parameter rr has to lookup .length field. --- src/record_syscall.cc | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/src/record_syscall.cc b/src/record_syscall.cc index 720e2f1e2c6..ad7fac6b186 100644 --- a/src/record_syscall.cc +++ b/src/record_syscall.cc @@ -1593,7 +1593,16 @@ static void prepare_rdma_verbs_ioctl(__attribute__((unused)) RecordTask* t, { LOG(warn) << "called prepare_rdma_verbs_ioctl()"; - auto hdrp = syscall_state.reg_parameter(3, IN); + auto ®s = syscall_state.syscall_entry_registers; + remote_ptr argp = regs.arg(3); + auto hdrp = argp.cast(); + if (hdrp.is_null()) { + LOG(fatal) << "arg3 is NULL."; + // TODO(sodar): How to handle it correctly? + syscall_state.expect_errno = EFAULT; + return; + } + bool ok = true; auto hdr = t->read_mem(hdrp, &ok); if (!ok) { @@ -1601,6 +1610,15 @@ static void prepare_rdma_verbs_ioctl(__attribute__((unused)) RecordTask* t, return; } + auto size = hdr.length; + auto hdrv = syscall_state.reg_parameter(3, size, IN_OUT); + if (hdrv.is_null()) { + LOG(fatal) << "hdrv is NULL."; + // TODO(sodar): How to handle it correctly? + syscall_state.expect_errno = EFAULT; + return; + } + LOG(warn) << " hdr->length = " << hdr.length; LOG(warn) << " hdr->object_id = " << hdr.object_id; LOG(warn) << " hdr->method_id = " << hdr.method_id; From de89417eb9dbf1048efb3fc810c36a608e1de227 Mon Sep 17 00:00:00 2001 From: Dariusz Sosnowski Date: Sat, 5 Sep 2020 14:10:43 +0200 Subject: [PATCH 3/5] record_syscall: Removes WARN logs from prepare_rdma_verbs_ioctl --- src/record_syscall.cc | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/record_syscall.cc b/src/record_syscall.cc index ad7fac6b186..ada352e2f15 100644 --- a/src/record_syscall.cc +++ b/src/record_syscall.cc @@ -1618,11 +1618,6 @@ static void prepare_rdma_verbs_ioctl(__attribute__((unused)) RecordTask* t, syscall_state.expect_errno = EFAULT; return; } - - LOG(warn) << " hdr->length = " << hdr.length; - LOG(warn) << " hdr->object_id = " << hdr.object_id; - LOG(warn) << " hdr->method_id = " << hdr.method_id; - LOG(warn) << " hdr->num_attrs = " << hdr.num_attrs; } template From 4df0bc66c8476a545285992916d0329b416480fb Mon Sep 17 00:00:00 2001 From: Dariusz Sosnowski Date: Sat, 5 Sep 2020 16:14:50 +0200 Subject: [PATCH 4/5] record_syscall: Store RDMA_VERBS_IOCTL attrs in scratch `struct ib_uverbs_ioctl_hdr` as an VLA of `struct ib_uverbs_attr`. Each of this can hold a pointer to userspace buffer or a piece of data. Kernel driver distinguish between data/pointer based on internal kernel data, so here memory buffer is recorded in scratch if: - data length is longer than 8 (then it is most certainly a buffer); - or data length is equal to 8 and data addr points to a valid tracee memory (however it might not be accurate, data length might be 8 and can be treated as data, but store scratch anyway). On top of that attr struct is always stored in scratch. --- src/record_syscall.cc | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/src/record_syscall.cc b/src/record_syscall.cc index ada352e2f15..43f46c7a56c 100644 --- a/src/record_syscall.cc +++ b/src/record_syscall.cc @@ -1587,6 +1587,8 @@ template void prepare_ethtool_ioctl(RecordTask* t, TaskSyscallSt syscall_state.after_syscall_action(record_page_below_stack_ptr); } +template struct TD; + template static void prepare_rdma_verbs_ioctl(__attribute__((unused)) RecordTask* t, __attribute__((unused)) TaskSyscallState& syscall_state) @@ -1618,6 +1620,40 @@ static void prepare_rdma_verbs_ioctl(__attribute__((unused)) RecordTask* t, syscall_state.expect_errno = EFAULT; return; } + + // TODO(sodar): Do using real structs... + // TD td; + auto attrs_field_p = hdrp.template cast() + sizeof(typename Arch::ib_uverbs_ioctl_hdr); + auto attrs_p = attrs_field_p.template cast(); + for (unsigned int i = 0; i < hdr.num_attrs; ++i) { + auto attr_p = attrs_p + i; + auto attr = t->read_mem(attr_p, &ok); + ASSERT(t, ok) << "failed to read attrs[" << i << "]"; + + ssize_t data_len = attr.len; + uintptr_t data_addr = attr.data; + if (data_len > 8) { + // Heuristic - assume that any data longer than 8 bytes, is pointer pointing to a buffer and + // attr is PTR_OUT. + ASSERT(t, data_addr != 0) << "p is NULL, but data_len = " << data_len << "for arrts[" << i << "]"; + // remote_ptr data_p = data_addr; + auto data_p = REMOTE_PTR_FIELD(attr_p, data); + auto p = syscall_state.mem_ptr_parameter(data_p, data_len, IN_OUT); + ASSERT(t, !p.is_null()) << "p to arrts[" << i << "] is null"; + } else if (data_len == 8) { + // Heuristic - if data can fit a pointer check if pointer is valid; if not, it is probably ENUM_IN. + // Otherwise PTR_IN and/or PTR_OUT. + // ASSERT(t, data_addr != 0) << "p is NULL, but data_len = " << data_len << "for arrts[" << i << "]"; + // remote_ptr data_p = data_addr; + auto data_p = REMOTE_PTR_FIELD(attr_p, data); + if (t->vm()->has_mapping(data_addr)) { + auto p = syscall_state.mem_ptr_parameter(data_p, data_len, IN_OUT); + ASSERT(t, !p.is_null()) << "p to arrts[" << i << "] is null"; + } + } else { + // Already stored in VLA. + } + } } template From 87fe972260e0ebe0bf43c199c985d3862c997c27 Mon Sep 17 00:00:00 2001 From: Dariusz Sosnowski Date: Tue, 8 Sep 2020 00:09:24 +0200 Subject: [PATCH 5/5] record_syscall: Properly handle INVOKE_WRITE RDMA verb --- src/record_syscall.cc | 35 +++++++++++++++++++++++++++++++++-- 1 file changed, 33 insertions(+), 2 deletions(-) diff --git a/src/record_syscall.cc b/src/record_syscall.cc index 43f46c7a56c..4c928dcc144 100644 --- a/src/record_syscall.cc +++ b/src/record_syscall.cc @@ -30,6 +30,7 @@ #include #include #include +#include #include #include #include @@ -1593,8 +1594,6 @@ template static void prepare_rdma_verbs_ioctl(__attribute__((unused)) RecordTask* t, __attribute__((unused)) TaskSyscallState& syscall_state) { - LOG(warn) << "called prepare_rdma_verbs_ioctl()"; - auto ®s = syscall_state.syscall_entry_registers; remote_ptr argp = regs.arg(3); auto hdrp = argp.cast(); @@ -1612,6 +1611,12 @@ static void prepare_rdma_verbs_ioctl(__attribute__((unused)) RecordTask* t, return; } + LOG(debug) + << "hdr->length=" << hdr.length + << " hdr->object_id=" << hdr.object_id + << " hdr->method_id=" << hdr.method_id + << " hdr->num_attrs=" << hdr.num_attrs; + auto size = hdr.length; auto hdrv = syscall_state.reg_parameter(3, size, IN_OUT); if (hdrv.is_null()) { @@ -1630,6 +1635,32 @@ static void prepare_rdma_verbs_ioctl(__attribute__((unused)) RecordTask* t, auto attr = t->read_mem(attr_p, &ok); ASSERT(t, ok) << "failed to read attrs[" << i << "]"; + if (hdr.object_id == UVERBS_OBJECT_DEVICE && hdr.method_id == UVERBS_METHOD_INVOKE_WRITE) { + switch (attr.attr_id) { + case UVERBS_ATTR_CORE_IN: + case UVERBS_ATTR_UHW_IN: { + if (attr.len > sizeof(uint64_t)) { + auto data_p = REMOTE_PTR_FIELD(attr_p, data); + syscall_state.mem_ptr_parameter(data_p, attr.len, IN_OUT); + } + break; + } + case UVERBS_ATTR_CORE_OUT: + case UVERBS_ATTR_UHW_OUT: { + auto data_p = REMOTE_PTR_FIELD(attr_p, data); + syscall_state.mem_ptr_parameter(data_p, attr.len, IN_OUT); + break; + } + case UVERBS_ATTR_WRITE_CMD: { + // Should be inside attr struct. + break; + } + default: + ASSERT(t, false) << "unknown attr_id for INVOKE_WRITE verb"; + } + continue; + } + ssize_t data_len = attr.len; uintptr_t data_addr = attr.data; if (data_len > 8) {