From fa6ab22ac3914a232adcfd2b042eee875409678b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bernhard=20=C3=9Cbelacker?= Date: Mon, 9 Sep 2024 17:51:26 +0200 Subject: [PATCH] Add parameter to check if files are mapped in processes not part of the recording. --- CMakeLists.txt | 1 + src/AddressSpace.cc | 4 +++ src/RecordCommand.cc | 16 +++++++++-- src/RecordSession.cc | 11 +++++--- src/RecordSession.h | 8 ++++-- src/record_syscall.cc | 49 +++++++++++++++++++++++++++++++++ src/test/mmap_shared_extern.c | 49 +++++++++++++++++++++++++++++++++ src/test/mmap_shared_extern.run | 25 +++++++++++++++++ 8 files changed, 154 insertions(+), 9 deletions(-) create mode 100644 src/test/mmap_shared_extern.c create mode 100644 src/test/mmap_shared_extern.run diff --git a/CMakeLists.txt b/CMakeLists.txt index 17621d60694..5c6a6118332 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -1556,6 +1556,7 @@ set(TESTS_WITH_PROGRAM many_yields mmap_fd_reuse_checkpoint mmap_replace_most_mappings + mmap_shared_extern mmap_shared_prot mmap_shared_write_exec_race mmap_tmpfs diff --git a/src/AddressSpace.cc b/src/AddressSpace.cc index 5e3b94b8393..7c73b07f629 100644 --- a/src/AddressSpace.cc +++ b/src/AddressSpace.cc @@ -107,6 +107,10 @@ void KernelMapIterator::init(bool* ok) { } void KernelMapIterator::operator++() { + if (!maps_file) { + return; + } + char line[PATH_MAX * 2]; if (!fgets(line, sizeof(line), maps_file)) { fclose(maps_file); diff --git a/src/RecordCommand.cc b/src/RecordCommand.cc index d2398170b3b..6dc939e0e55 100644 --- a/src/RecordCommand.cc +++ b/src/RecordCommand.cc @@ -104,7 +104,9 @@ RecordCommand RecordCommand::singleton( " --asan Override heuristics and always enable ASAN\n" " compatibility.\n" " --tsan Override heuristics and always enable TSAN\n" - " compatibility.\n"); + " compatibility.\n" + " --check-outside-mmaps Try to identify files that are mapped outside\n" + " of the trace and could cause diversions.\n"); struct RecordFlags { vector extra_env; @@ -188,6 +190,9 @@ struct RecordFlags { with PT. */ bool intel_pt; + /* True if we should check files being mapped outside of the recording. */ + bool check_outside_mmaps; + RecordFlags() : max_ticks(Scheduler::DEFAULT_MAX_TICKS), ignore_sig(0), @@ -212,7 +217,8 @@ struct RecordFlags { unmap_vdso(false), asan(false), tsan(false), - intel_pt(false) {} + intel_pt(false), + check_outside_mmaps(false) {} }; static void parse_signal_name(ParsedOption& opt) { @@ -275,6 +281,7 @@ static bool parse_record_arg(vector& args, RecordFlags& flags) { { 17, "asan", NO_PARAMETER }, { 18, "tsan", NO_PARAMETER }, { 19, "intel-pt", NO_PARAMETER }, + { 20, "check-outside-mmaps", NO_PARAMETER }, { 'c', "num-cpu-ticks", HAS_PARAMETER }, { 'h', "chaos", NO_PARAMETER }, { 'i', "ignore-signal", HAS_PARAMETER }, @@ -491,6 +498,9 @@ static bool parse_record_arg(vector& args, RecordFlags& flags) { case 19: flags.intel_pt = true; break; + case 20: + flags.check_outside_mmaps = true; + break; case 's': flags.always_switch = true; break; @@ -681,7 +691,7 @@ static WaitStatus record(const vector& args, const RecordFlags& flags) { flags.bind_cpu, flags.output_trace_dir, flags.trace_id.get(), flags.stap_sdt, flags.unmap_vdso, flags.asan, flags.tsan, - flags.intel_pt); + flags.intel_pt, flags.check_outside_mmaps); setup_session_from_flags(*session, flags); static_session = session.get(); diff --git a/src/RecordSession.cc b/src/RecordSession.cc index 1a2d2906dc0..73c3193aecc 100644 --- a/src/RecordSession.cc +++ b/src/RecordSession.cc @@ -2386,7 +2386,8 @@ static string lookup_by_path(const string& name) { bool unmap_vdso, bool force_asan_active, bool force_tsan_active, - bool intel_pt) { + bool intel_pt, + bool check_outside_mmaps) { TraceeAttentionSet::initialize(); // The syscallbuf library interposes some critical @@ -2494,7 +2495,7 @@ static string lookup_by_path(const string& name) { new RecordSession(full_path, argv, env, disable_cpuid_features, syscallbuf, syscallbuf_desched_sig, bind_cpu, output_trace_dir, trace_id, use_audit, unmap_vdso, - intel_pt)); + intel_pt, check_outside_mmaps)); session->excluded_ranges_ = std::move(exe_info.sanitizer_exclude_memory_ranges); session->fixed_global_exclusion_range_ = std::move(exe_info.fixed_global_exclusion_range); return session; @@ -2511,7 +2512,8 @@ RecordSession::RecordSession(const std::string& exe_path, const TraceUuid* trace_id, bool use_audit, bool unmap_vdso, - bool intel_pt_enabled) + bool intel_pt_enabled, + bool check_outside_mmaps) : trace_out(argv[0], output_trace_dir, ticks_semantics_), scheduler_(*this), trace_id(trace_id), @@ -2527,7 +2529,8 @@ RecordSession::RecordSession(const std::string& exe_path, enable_chaos_(false), wait_for_all_(false), use_audit_(use_audit), - unmap_vdso_(unmap_vdso) { + unmap_vdso_(unmap_vdso), + check_outside_mmaps_(check_outside_mmaps) { set_intel_pt_enabled(intel_pt_enabled); if (intel_pt_enabled) { PerfCounters::start_pt_copy_thread(); diff --git a/src/RecordSession.h b/src/RecordSession.h index b18b3d5668d..de150d88c25 100644 --- a/src/RecordSession.h +++ b/src/RecordSession.h @@ -72,7 +72,8 @@ class RecordSession final : public Session { bool unmap_vdso = false, bool force_asan_active = false, bool force_tsan_active = false, - bool intel_pt = false); + bool intel_pt = false, + bool check_outside_mmaps = false); ~RecordSession() override; @@ -98,6 +99,7 @@ class RecordSession final : public Session { } bool use_audit() const { return use_audit_; } bool unmap_vdso() { return unmap_vdso_; } + bool check_outside_mmaps() { return check_outside_mmaps_; } uint64_t rr_signal_mask() const; enum RecordStatus { @@ -221,7 +223,8 @@ class RecordSession final : public Session { const TraceUuid* trace_id, bool use_audit, bool unmap_vdso, - bool intel_pt); + bool intel_pt, + bool check_outside_mmaps); virtual void on_create(Task* t) override; @@ -286,6 +289,7 @@ class RecordSession final : public Session { bool use_audit_; bool unmap_vdso_; + bool check_outside_mmaps_; }; } // namespace rr diff --git a/src/record_syscall.cc b/src/record_syscall.cc index 720517d8a80..3962627302c 100644 --- a/src/record_syscall.cc +++ b/src/record_syscall.cc @@ -6006,6 +6006,51 @@ static vector find_holes(RecordTask* t, int desc, uint64_t offset, ui return ret; } +static void check_outside_mappings(const KernelMapping& tracee_km, const RecordSession& session) { +#if defined(__i386__) + struct utsname buf; + if (uname(&buf) != 0) { + FATAL() << "Failed to read /proc"; + } + if (sizeof(void*) == 4 && strcmp(buf.machine, "x86_64") == 0) { + /* Running 32-bit rr at a 64-bit kernel is not going to work because + * the KernelMapIterator currently does not handle this combination. */ + FATAL() << "Cannot use --check_outside_mmaps with 32-bit rr at 64-bit kernel."; + } +#endif + DIR* proc = opendir("/proc"); + if (!proc) { + FATAL() << "Failed to read /proc"; + } + while (true) { + struct dirent* f = readdir(proc); + if (!f) { + break; + } + pid_t pid = atoi(f->d_name); + if (pid) { + auto task = session.find_task(pid); + if (task) { + continue; + } + bool ok = false; + for (KernelMapIterator it(pid, &ok); ok && !it.at_end(); ++it) { + auto km = it.current(); + if (km.device() == tracee_km.device() && + km.inode() == tracee_km.inode() && + km.prot() & PROT_WRITE) + { + printf("rr: Warning: Mapping of file %s could cause diversion when replaying, " + "because pid=%d has mapped it outside of the recording.\n", + km.fsname().c_str(), pid); + return; + } + } + } + } + closedir(proc); +} + static void process_mmap(RecordTask* t, size_t length, int prot, int flags, int fd, off64_t offset) { if (t->regs().syscall_failed()) { @@ -6126,6 +6171,10 @@ static void process_mmap(RecordTask* t, size_t length, int prot, int flags, "written by programs outside the rr tracee " "tree."; } + + if (t->session().check_outside_mmaps()) { + check_outside_mappings(km, t->session()); + } } // We don't want to patch MAP_SHARED files. In the best case we'd end crashing diff --git a/src/test/mmap_shared_extern.c b/src/test/mmap_shared_extern.c new file mode 100644 index 00000000000..2893830ff40 --- /dev/null +++ b/src/test/mmap_shared_extern.c @@ -0,0 +1,49 @@ +#include "util.h" + +#include +#include +#include +#include +#include + +int main(__attribute__((unused)) int argc, char* argv[], char* argenv[]) { + size_t page_size = sysconf(_SC_PAGESIZE); + char last = 0; + char* p; + int proc_num = argv[2][0] - '0'; + int timeout = 30000; // 30 seconds + + if (argv[3][0] > '0') { + if (fork() == 0) { + char proc_num_arg[2] = {'0' + proc_num + 1, '\0'}; + char proc_to_start_arg[2] = {argv[3][0] - 1, '\0'}; + char* execv_argv[] = {argv[0], argv[1], proc_num_arg, proc_to_start_arg, NULL}; + execve("/proc/self/exe", execv_argv, argenv); + } + } + + int fd = open(argv[1], O_RDWR|O_CREAT, 0600); + test_assert(fd >= 0); + ftruncate(fd, page_size); + p = (char*)mmap(0, page_size, PROT_READ|PROT_WRITE, MAP_SHARED, fd, 0); + assert(p != MAP_FAILED); + + while (1) { + if ((*p % 2) == proc_num) { + if ((*p)++ >= 6) { + atomic_printf("%d %d exiting.\n", getpid(), proc_num); + usleep(50000); /* first recorded process exiting kills all others, wait a little. */ + return 0; + } + } + if (last != *p) + atomic_printf("%d\n", last = *p); + usleep(50000); + timeout -= 50; + if (timeout < 0) { + atomic_printf("timeout reached.\n"); + return 0; + } + } + return 0; +} diff --git a/src/test/mmap_shared_extern.run b/src/test/mmap_shared_extern.run new file mode 100644 index 00000000000..933a3ae998e --- /dev/null +++ b/src/test/mmap_shared_extern.run @@ -0,0 +1,25 @@ +source `dirname $0`/util.sh + +skip_if_rr_32_bit_with_shell_64_bit + +rm -rf $workdir/map_file +RECORD_ARGS="--check-outside-mmaps" +record mmap_shared_extern$bitness $workdir/map_file 0 1 +mv record.out record.out.1 +mv record.err record.err.1 +grep "could cause diversion" $workdir/record.out.1 > /dev/null +if [[ $? != 1 ]]; then + failed "\"could cause diversion\" was returned when all processes are inside the recording." + exit +fi + +rm -rf $workdir/map_file +mmap_shared_extern$bitness $workdir/map_file 0 0 > /dev/null& +RECORD_ARGS="--check-outside-mmaps" +record mmap_shared_extern$bitness $workdir/map_file 1 0 +mv record.out record.out.2 +mv record.err record.err.2 +grep "could cause diversion" $workdir/record.out.2 > /dev/null +if [[ $? != 0 ]]; then + failed "\"could cause diversion\" was not returned when some processes are outside the recording." +fi