Skip to content

Commit

Permalink
Draft: Add parameter to check if files are mapped in processes not pa…
Browse files Browse the repository at this point in the history
…rt of the recording.
  • Loading branch information
bernhardu committed Sep 9, 2024
1 parent 21f051b commit 589b266
Show file tree
Hide file tree
Showing 8 changed files with 135 additions and 9 deletions.
1 change: 1 addition & 0 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -1553,6 +1553,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"
" --lsof Try to identify files that are mapped outside\n"
" of the trace and migth 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 lsof;

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),
lsof(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, "lsof", 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.lsof = 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.lsof);
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 lsof) {
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, lsof));
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 lsof)
: 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),
lsof_(lsof) {
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 lsof = 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 lsof() { return lsof_; }
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 lsof);

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 lsof_;
};

} // namespace rr
Expand Down
41 changes: 41 additions & 0 deletions src/record_syscall.cc
Original file line number Diff line number Diff line change
Expand Up @@ -6006,6 +6006,43 @@ 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 might cause diversion when replaying, "
"because pid=%d has mapped it outside of the recording.\n",
tracee_file_name.c_str(), pid);
}
}
}
}
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 +6163,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().lsof()) {
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="--lsof"
record mmap_shared_extern$bitness $workdir/map_file 0 1
mv record.out record.out.1
mv record.err record.err.1
grep "might cause diversion" $workdir/record.out.1 > /dev/null
if [[ $? != 1 ]]; then
failed "\"might 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="--lsof"
record mmap_shared_extern$bitness $workdir/map_file 1 0
mv record.out record.out.2
mv record.err record.err.2
grep "might cause diversion" $workdir/record.out.2 > /dev/null
if [[ $? != 0 ]]; then
failed "\"might cause diversion\" was not returned when some processes are outside the recording."
fi

0 comments on commit 589b266

Please sign in to comment.