Skip to content

Commit

Permalink
Warn about potential for data corruption with unaligned probes
Browse files Browse the repository at this point in the history
We should alert users to the dangers of "--unsafe" if we're going to
suggest it to them as an option.
  • Loading branch information
ajor committed Oct 7, 2024
1 parent da91c51 commit 279301d
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 9 deletions.
16 changes: 9 additions & 7 deletions src/attached_probe.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -381,6 +381,10 @@ static uint64_t resolve_offset(const std::string &path,
return bcc_sym.offset;
}

static constexpr std::string_view hint_unsafe =
"\nUse --unsafe to force attachment. WARNING: This option could lead to "
"data corruption in the target process.";

static void check_alignment(std::string &path,
std::string &symbol,
uint64_t sym_offset,
Expand All @@ -400,7 +404,7 @@ static void check_alignment(std::string &path,
if (safe_mode)
throw FatalUserException("Could not add " + probetypeName(type) +
" into middle of instruction: " + tmp +
" (use --unsafe to force attachment)");
std::string{ hint_unsafe });
else
LOG(WARNING) << "Unsafe " << type
<< " in the middle of the instruction: " << tmp;
Expand All @@ -410,7 +414,7 @@ static void check_alignment(std::string &path,
if (safe_mode)
throw FatalUserException("Failed to check if " + probetypeName(type) +
" is in proper place: " + tmp +
" (use --unsafe to force attachment)");
std::string{ hint_unsafe });
else
LOG(WARNING) << "Unchecked " << type << ": " << tmp;
break;
Expand All @@ -420,7 +424,7 @@ static void check_alignment(std::string &path,
throw FatalUserException("Can't check if " + probetypeName(type) +
" is in proper place (compiled without "
"(k|u)probe offset support): " +
tmp + " (use --unsafe to force attachment)");
tmp + std::string{ hint_unsafe });
else
LOG(WARNING) << "Unchecked " << type << " : " << tmp;
break;
Expand Down Expand Up @@ -491,12 +495,10 @@ bool AttachedProbe::resolve_offset_uprobe(bool safe_mode, bool has_multiple_aps)
msg << "Could not determine boundary for " << sym.name
<< " (symbol has size 0).";
if (probe_.orig_name == probe_.name) {
msg << " Use --unsafe to force attachment.";
msg << hint_unsafe;
throw FatalUserException(msg.str());
} else {
LOG(WARNING)
<< msg.str()
<< " Skipping attachment (use --unsafe to force attachment).";
LOG(WARNING) << msg.str() << " Skipping attachment." << hint_unsafe;
}
return false;
}
Expand Down
2 changes: 1 addition & 1 deletion src/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ void usage()
std::cerr << " -c 'CMD' run CMD and enable USDT probes on resulting process" << std::endl;
std::cerr << " --usdt-file-activation" << std::endl;
std::cerr << " activate usdt semaphores based on file path" << std::endl;
std::cerr << " --unsafe allow unsafe builtin functions" << std::endl;
std::cerr << " --unsafe allow unsafe/destructive functionality" << std::endl;
std::cerr << " -q keep messages quiet" << std::endl;
std::cerr << " --info Print information about kernel BPF support" << std::endl;
std::cerr << " -k emit a warning when a bpf helper returns an error (except read functions)" << std::endl;
Expand Down
3 changes: 2 additions & 1 deletion tests/runtime/probe
Original file line number Diff line number Diff line change
Expand Up @@ -305,7 +305,8 @@ AFTER /bin/bash -c "echo lala"; /bin/bash -c "echo lala"

NAME uprobe_zero_size
PROG uprobe:./testprogs/uprobe_test:_init { printf("arg0: %d\n", arg0); exit();}
EXPECT ERROR: Could not determine boundary for _init (symbol has size 0). Use --unsafe to force attachment.
EXPECT ERROR: Could not determine boundary for _init (symbol has size 0).
Use --unsafe to force attachment. WARNING: This option could lead to data corruption in the target process.
WILL_FAIL

NAME uprobe_zero_size_unsafe
Expand Down

0 comments on commit 279301d

Please sign in to comment.