Skip to content

Fix /proc/<pid>/stat parsing when the command name contains whitespace #2410

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.

Sign up for GitHub

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

Open
NopeCharon wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Fix /proc/<pid>/stat parsing when the command name contains whitespace #2410

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
143 changes: 70 additions & 73 deletions src/common/system/proc_parser.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ constexpr int kProcNetDevTxDropField = 12;
constexpr int kProcStatNumFields = 52;

constexpr int kProcStatPIDField = 0;
constexpr int kProcStatCommandField = 1;

constexpr int kProcStatMinorFaultsField = 9;
constexpr int kProcStatMajorFaultsField = 11;
Expand All @@ -96,6 +97,45 @@ constexpr int kProcStatStartTimeField = 21;
constexpr int kProcStatVSizeField = 22;
constexpr int kProcStatRSSField = 23;

namespace {

/**
* Splits the contents of a /proc/<pid>/stat file into its fields, so that the kProcStat*Field
* constants above can be used as indices into the result.
*
* The command field is the executable name surrounded by parentheses. The name can contain any
* character, including spaces, newlines and parentheses, so it can't be split on whitespace like
* the other fields. Instead, it extends from the first '(' to the last ')' in the file, since none
* of the subsequent fields can contain a ')'. The command field is returned without parentheses.
*/
StatusOr<std::vector<std::string_view>> SplitProcPIDStat(std::string_view content,
const std::filesystem::path& fpath) {
const size_t open_paren_idx = content.find('(');
const size_t close_paren_idx = content.rfind(')');
if (open_paren_idx == std::string_view::npos || close_paren_idx == std::string_view::npos ||
close_paren_idx < open_paren_idx) {
return error::Internal("Invalid command name in file: $0.", fpath.string());
}

std::vector<std::string_view> fields;
fields.push_back(absl::StripAsciiWhitespace(content.substr(0, open_paren_idx)));
fields.push_back(content.substr(open_paren_idx + 1, close_paren_idx - open_paren_idx - 1));
for (std::string_view field : absl::StrSplit(content.substr(close_paren_idx + 1),
absl::ByAnyChar(" \n"), absl::SkipEmpty())) {
fields.push_back(field);
}

// We check less than in case more fields are added later.
if (fields.size() < kProcStatNumFields) {
return error::Internal("Unexpected number of fields: $0, in file: $1.", fields.size(),
fpath.string());
}

return fields;
}

} // namespace

Status ProcParser::ParseNetworkStatAccumulateIFaceData(
const std::vector<std::string_view>& dev_stat_record, NetworkStats* out) {
DCHECK(out != nullptr);
Expand Down Expand Up @@ -208,54 +248,29 @@ Status ProcParser::ParseProcPIDStat(int32_t pid, int64_t page_size_bytes,
*/
DCHECK(out != nullptr);
const auto fpath = ProcPidPath(pid, "stat");
std::ifstream ifs;
ifs.open(fpath);
if (!ifs) {
return error::Internal("Failed to open file: $0.", fpath.string());
}

std::string line;
bool ok = true;
if (std::getline(ifs, line)) {
std::vector<std::string_view> split = absl::StrSplit(line, " ", absl::SkipWhitespace());
// We check less than in case more fields are added later.
if (split.size() < kProcStatNumFields) {
return error::Unknown("Incorrect number of fields in stat file: $0.", fpath.string());
}
PX_ASSIGN_OR_RETURN(std::string content, ReadFileToString(fpath));
PX_ASSIGN_OR_RETURN(std::vector<std::string_view> fields, SplitProcPIDStat(content, fpath));

// The name is surrounded by (). We remove it first.
size_t open_paren_idx = line.find_first_of('(');
size_t close_paren_idx = line.find_last_of(')');
if (open_paren_idx == std::string::npos || close_paren_idx == std::string::npos) {
return error::Internal("Invalid command name in file $0.", fpath.string());
}
out->process_name = line.substr(open_paren_idx + 1, close_paren_idx - open_paren_idx - 1);
out->process_name = std::string(fields[kProcStatCommandField]);

// When split_size > kProcStatNumFields, there are spaces in the command we need to handle.
// command_offset adjusts index for the number of spaces in the command field.
int command_offset = std::count(out->process_name.begin(), out->process_name.end(), ' ');

ok &= absl::SimpleAtoi(split[kProcStatPIDField], &out->pid);
bool ok = true;
ok &= absl::SimpleAtoi(fields[kProcStatPIDField], &out->pid);

ok &= absl::SimpleAtoi(split[kProcStatMinorFaultsField + command_offset], &out->minor_faults);
ok &= absl::SimpleAtoi(split[kProcStatMajorFaultsField + command_offset], &out->major_faults);
ok &= absl::SimpleAtoi(fields[kProcStatMinorFaultsField], &out->minor_faults);
ok &= absl::SimpleAtoi(fields[kProcStatMajorFaultsField], &out->major_faults);

ok &= absl::SimpleAtoi(split[kProcStatUTimeField + command_offset], &out->utime_ns);
ok &= absl::SimpleAtoi(split[kProcStatKTimeField + command_offset], &out->ktime_ns);
// The kernel tracks utime and ktime in kernel ticks.
out->utime_ns *= kernel_tick_time_ns;
out->ktime_ns *= kernel_tick_time_ns;
ok &= absl::SimpleAtoi(fields[kProcStatUTimeField], &out->utime_ns);
ok &= absl::SimpleAtoi(fields[kProcStatKTimeField], &out->ktime_ns);
// The kernel tracks utime and ktime in kernel ticks.
out->utime_ns *= kernel_tick_time_ns;
out->ktime_ns *= kernel_tick_time_ns;

ok &= absl::SimpleAtoi(split[kProcStatNumThreadsField + command_offset], &out->num_threads);
ok &= absl::SimpleAtoi(split[kProcStatVSizeField + command_offset], &out->vsize_bytes);
ok &= absl::SimpleAtoi(std::string(split[kProcStatRSSField + command_offset]), &out->rss_bytes);
ok &= absl::SimpleAtoi(fields[kProcStatNumThreadsField], &out->num_threads);
ok &= absl::SimpleAtoi(fields[kProcStatVSizeField], &out->vsize_bytes);
ok &= absl::SimpleAtoi(std::string(fields[kProcStatRSSField]), &out->rss_bytes);

// RSS is in pages.
out->rss_bytes *= page_size_bytes;

} else {
return error::Internal("Failed to read proc stat file: $0.", fpath.string());
}
// RSS is in pages.
out->rss_bytes *= page_size_bytes;

if (!ok) {
// This should never happen since it requires the file to be ill-formed
Expand Down Expand Up @@ -714,7 +729,7 @@ Status ProcParser::ReadNSPid(pid_t pid, std::vector<std::string>* ns_pids) const

StatusOr<int64_t> GetPIDStartTimeTicks(const std::filesystem::path& proc_pid_path) {
const std::filesystem::path proc_pid_stat_path = proc_pid_path / "stat";
std::string line;
std::string content;

// It's usually a big no-no to ifdef based on compilation mode,
// but this is a workaround for an ASAN bug that causes crashes and flakiness
Expand All @@ -732,43 +747,25 @@ StatusOr<int64_t> GetPIDStartTimeTicks(const std::filesystem::path& proc_pid_pat
// A few quick runs show that /proc/<pid>/stat is <200 characters on my machine,
// but a lot of those numbers are 0. If they were non-zero, the string could be much
// longer. Hopefully a 4KB block keeps us safe. This is for ASAN only anyways.
// The whole file is read, rather than a single line, because the command name may
// contain newlines.
constexpr int kMaxSupportedProcPIDStatLength = 4096;
line.resize(kMaxSupportedProcPIDStatLength);
if (std::fgets(line.data(), line.size(), fp) == nullptr) {
return error::Internal("Could not get line from file: $0.", proc_pid_stat_path.string());
content.resize(kMaxSupportedProcPIDStatLength);
const size_t bytes_read = std::fread(content.data(), 1, content.size(), fp);
std::fclose(fp);
if (bytes_read == 0) {
return error::Internal("Could not read file: $0.", proc_pid_stat_path.string());
}
content.resize(bytes_read);
#else
std::ifstream ifs;
ifs.open(proc_pid_stat_path);
if (!ifs) {
return error::Internal("Could not open file: $0.", proc_pid_stat_path.string());
}

if (!std::getline(ifs, line)) {
return error::Internal("Could not get line from file: $0.", proc_pid_stat_path.string());
}
PX_ASSIGN_OR_RETURN(content, ReadFileToString(proc_pid_stat_path));
#endif

size_t open_paren_idx = line.find_first_of('(');
size_t close_paren_idx = line.find_last_of(')');
if (open_paren_idx == std::string::npos || close_paren_idx == std::string::npos) {
return error::Internal("Invalid command name in file: $0.", proc_pid_stat_path.string());
}

// When split_size > kProcStatNumFields, there are spaces in the command we need to handle.
// command_offset adjusts index for the number of spaces in the command field.
int command_offset =
std::count(line.begin() + open_paren_idx, line.begin() + close_paren_idx, ' ');

std::vector<std::string_view> split = absl::StrSplit(line, " ", absl::SkipWhitespace());
// We check less than in case more fields are added later.
if (split.size() < kProcStatNumFields) {
return error::Internal("Unexpected number of columns: $0, in file: $1.", split.size(),
proc_pid_stat_path.string());
}
PX_ASSIGN_OR_RETURN(std::vector<std::string_view> fields,
SplitProcPIDStat(content, proc_pid_stat_path));

int64_t start_time_ticks;
if (!absl::SimpleAtoi(split[kProcStatStartTimeField + command_offset], &start_time_ticks)) {
if (!absl::SimpleAtoi(fields[kProcStatStartTimeField], &start_time_ticks)) {
return error::Internal("Time value does not parse in file: $0.", proc_pid_stat_path.string());
}

Expand Down
53 changes: 53 additions & 0 deletions src/common/system/proc_parser_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@
#include <memory>
#include <sstream>

#include <absl/strings/str_cat.h>

#include "src/common/fs/fs_wrapper.h"
#include "src/common/testing/test_environment.h"
#include "src/common/testing/testing.h"
Expand Down Expand Up @@ -277,6 +279,57 @@ TEST_F(ProcParserTest, read_pid_start_time) {
ASSERT_OK_AND_EQ(parser_->GetPIDStartTimeTicks(123), 14329);
}

// The command name in /proc/<pid>/stat may contain any character, including runs of spaces,
// newlines and parentheses. None of these should affect the parsing of the subsequent fields.
class ProcParserCommandNameTest : public ProcParserTest,
public ::testing::WithParamInterface<std::string> {
protected:
void SetUp() override {
ProcParserTest::SetUp();
const std::filesystem::path pid_path = temp_dir_.path() / "4602";
ASSERT_TRUE(std::filesystem::create_directory(pid_path));
ASSERT_OK(WriteFileFromString(
pid_path / "stat",
absl::StrCat("4602 (", GetParam(),
") S 3260 4602 3260 34818 4602 1077936128 1799 174589 55 68 8 23 106 72 20 0 "
"13 0 14329 114384896 2577 18446744073709551615 4194304 7917379 "
"140730842479232 0 0 0 1006254592 0 2143420159 0 0 0 17 3 0 0 3 0 0 "
"12193792 12432192 34951168 140730842488151 140730842488200 "
"140730842488200 140730842492896 0\n")));
}

testing::TempDir temp_dir_;
};

TEST_P(ProcParserCommandNameTest, ParsePidStat) {
PX_SET_FOR_SCOPE(FLAGS_proc_path, temp_dir_.path().string());
ProcParser::ProcessStats stats;
ASSERT_OK(parser_->ParseProcPIDStat(4602, bytes_per_page_, kernel_tick_time_ns_, &stats));

EXPECT_EQ(GetParam(), stats.process_name);
EXPECT_EQ(4602, stats.pid);

EXPECT_EQ(800, stats.utime_ns);
EXPECT_EQ(2300, stats.ktime_ns);
EXPECT_EQ(13, stats.num_threads);

EXPECT_EQ(55, stats.major_faults);
EXPECT_EQ(1799, stats.minor_faults);

EXPECT_EQ(114384896, stats.vsize_bytes);
EXPECT_EQ(2577 * bytes_per_page_, stats.rss_bytes);
}

TEST_P(ProcParserCommandNameTest, ReadPIDStartTime) {
PX_SET_FOR_SCOPE(FLAGS_proc_path, temp_dir_.path().string());
ASSERT_OK_AND_EQ(parser_->GetPIDStartTimeTicks(4602), 14329);
}

INSTANTIATE_TEST_SUITE_P(CommandNames, ProcParserCommandNameTest,
::testing::Values("npm", "", "npm start", "npm start", " npm ",
"npm\nstart", "npm\n", "npm (start)", "npm) (start",
") 1 2 3 ("));

TEST_F(ProcParserTest, read_pid_cmdline) {
PX_SET_FOR_SCOPE(FLAGS_proc_path, GetPathToTestDataFile("testdata/proc"));
EXPECT_THAT("/usr/lib/slack/slack --force-device-scale-factor=1.5 --high-dpi-support=1",
Expand Down
Loading