Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add parameter to check if files are mapped in processes not part of the recording. #3419

Merged
merged 1 commit into from
Sep 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
49 changes: 49 additions & 0 deletions src/record_syscall.cc
Original file line number Diff line number Diff line change
Expand Up @@ -6006,6 +6006,51 @@ static vector<WriteHole> 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()) {
Expand Down Expand Up @@ -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
Expand Down
49 changes: 49 additions & 0 deletions src/test/mmap_shared_extern.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
#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[], 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;
}
25 changes: 25 additions & 0 deletions src/test/mmap_shared_extern.run
Original file line number Diff line number Diff line change
@@ -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