Skip to content

Commit

Permalink
Add parameter to check if files are mapped in processes not part of t…
Browse files Browse the repository at this point in the history
…he recording.
  • Loading branch information
bernhardu committed Sep 23, 2024
1 parent d44565d commit 559a4f2
Show file tree
Hide file tree
Showing 8 changed files with 136 additions and 9 deletions.
1 change: 1 addition & 0 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 4 additions & 0 deletions src/AddressSpace.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
16 changes: 13 additions & 3 deletions src/RecordCommand.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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<string> extra_env;
Expand Down Expand Up @@ -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),
Expand All @@ -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) {
Expand Down Expand Up @@ -275,6 +281,7 @@ static bool parse_record_arg(vector<string>& 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 },
Expand Down Expand Up @@ -491,6 +498,9 @@ static bool parse_record_arg(vector<string>& 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;
Expand Down Expand Up @@ -681,7 +691,7 @@ static WaitStatus record(const vector<string>& 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();
Expand Down
11 changes: 7 additions & 4 deletions src/RecordSession.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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;
Expand All @@ -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),
Expand All @@ -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();
Expand Down
8 changes: 6 additions & 2 deletions src/RecordSession.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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 {
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -286,6 +289,7 @@ class RecordSession final : public Session {

bool use_audit_;
bool unmap_vdso_;
bool check_outside_mmaps_;
};

} // namespace rr
Expand Down
42 changes: 42 additions & 0 deletions src/record_syscall.cc
Original file line number Diff line number Diff line change
Expand Up @@ -6006,6 +6006,44 @@ static vector<WriteHole> find_holes(RecordTask* t, int desc, uint64_t offset, ui
return ret;
}

static void check_outside_mappings(const string& tracee_file_name, const RecordSession& session) {
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) {
bool ok = true;
for (auto it : session.tasks()) {
if (pid == it.second->tid) {
ok = false;
break;
}
}
if (!ok) {
break;
}
for (KernelMapIterator it(pid, &ok); !it.at_end(); ++it) {
auto km = it.current();
if (km.fsname() == tracee_file_name &&
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",
tracee_file_name.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()) {
Expand Down Expand Up @@ -6126,6 +6164,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(tracee_file_name, t->session());
}
}

// We don't want to patch MAP_SHARED files. In the best case we'd end crashing
Expand Down
40 changes: 40 additions & 0 deletions src/test/mmap_shared_extern.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
#include "util.h"

#include <assert.h>
#include <fcntl.h>
#include <string.h>
#include <sys/mman.h>
#include <sys/stat.h>

int main(__attribute__((unused)) int argc, char* argv[]) {
size_t page_size = sysconf(_SC_PAGESIZE);
char last = 0;
char* p;
int proc_num = argv[2][0] - '0';

if (argv[3][0] > '0') {
char cmd[512];
sprintf(cmd, "%s %s %d %c &", argv[0], argv[1], proc_num+1, argv[3][0]-1);
system(cmd);
}

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);
}
return 0;
}
23 changes: 23 additions & 0 deletions src/test/mmap_shared_extern.run
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
source `dirname $0`/util.sh

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

0 comments on commit 559a4f2

Please sign in to comment.