* [PATCH v2 1/3] tools subcmd: Tighten the filename size in check_if_command_finished
@ 2025-07-17 15:08 Ian Rogers
2025-07-17 15:08 ` [PATCH v2 2/3] perf pmu: Switch FILENAME_MAX to NAME_MAX Ian Rogers
` (2 more replies)
0 siblings, 3 replies; 4+ messages in thread
From: Ian Rogers @ 2025-07-17 15:08 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Namhyung Kim, Mark Rutland, Alexander Shishkin, Jiri Olsa,
Ian Rogers, Adrian Hunter, Kan Liang, James Clark, linux-kernel,
linux-perf-users
FILENAME_MAX is often PATH_MAX (4kb), far more than needed for the
/proc path. Make the buffer size sufficient for the maximum integer
plus "/proc/" and "/status" with a '\0' terminator.
Fixes: 5ce42b5de461 ("tools subcmd: Add non-waitpid check_if_command_finished()")
Signed-off-by: Ian Rogers <irogers@google.com>
---
v2: Fix precedence issue reported by the kernel test robot.
---
tools/lib/subcmd/run-command.c | 15 +++++++++++++--
1 file changed, 13 insertions(+), 2 deletions(-)
diff --git a/tools/lib/subcmd/run-command.c b/tools/lib/subcmd/run-command.c
index 0a764c25c384..b7510f83209a 100644
--- a/tools/lib/subcmd/run-command.c
+++ b/tools/lib/subcmd/run-command.c
@@ -5,6 +5,7 @@
#include <ctype.h>
#include <fcntl.h>
#include <string.h>
+#include <linux/compiler.h>
#include <linux/string.h>
#include <errno.h>
#include <sys/wait.h>
@@ -216,10 +217,20 @@ static int wait_or_whine(struct child_process *cmd, bool block)
return result;
}
+/*
+ * Conservative estimate of number of characaters needed to hold an a decoded
+ * integer, assume each 3 bits needs a character byte and plus a possible sign
+ * character.
+ */
+#ifndef is_signed_type
+#define is_signed_type(type) (((type)(-1)) < (type)1)
+#endif
+#define MAX_STRLEN_TYPE(type) (sizeof(type) * 8 / 3 + (is_signed_type(type) ? 1 : 0))
+
int check_if_command_finished(struct child_process *cmd)
{
#ifdef __linux__
- char filename[FILENAME_MAX + 12];
+ char filename[6 + MAX_STRLEN_TYPE(typeof(cmd->pid)) + 7 + 1];
char status_line[256];
FILE *status_file;
@@ -227,7 +238,7 @@ int check_if_command_finished(struct child_process *cmd)
* Check by reading /proc/<pid>/status as calling waitpid causes
* stdout/stderr to be closed and data lost.
*/
- sprintf(filename, "/proc/%d/status", cmd->pid);
+ sprintf(filename, "/proc/%u/status", cmd->pid);
status_file = fopen(filename, "r");
if (status_file == NULL) {
/* Open failed assume finish_command was called. */
--
2.50.0.727.gbf7dc18ff4-goog
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH v2 2/3] perf pmu: Switch FILENAME_MAX to NAME_MAX
2025-07-17 15:08 [PATCH v2 1/3] tools subcmd: Tighten the filename size in check_if_command_finished Ian Rogers
@ 2025-07-17 15:08 ` Ian Rogers
2025-07-17 15:08 ` [PATCH v2 3/3] perf ui scripts: " Ian Rogers
2025-07-23 17:30 ` [PATCH v2 1/3] tools subcmd: Tighten the filename size in check_if_command_finished Namhyung Kim
2 siblings, 0 replies; 4+ messages in thread
From: Ian Rogers @ 2025-07-17 15:08 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Namhyung Kim, Mark Rutland, Alexander Shishkin, Jiri Olsa,
Ian Rogers, Adrian Hunter, Kan Liang, James Clark, linux-kernel,
linux-perf-users
FILENAME_MAX is the same as PATH_MAX (4kb) in glibc rather than
NAME_MAX's 255. Switch to using NAME_MAX and ensure the '\0' is
accounted for in the path's buffer size.
Fixes: 754baf426e09 ("perf pmu: Change aliases from list to hashmap")
Signed-off-by: Ian Rogers <irogers@google.com>
---
tools/perf/util/pmu.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index b09b2ea2407a..f3da6e27bfcb 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -453,7 +453,7 @@ static struct perf_pmu_alias *perf_pmu__find_alias(struct perf_pmu *pmu,
{
struct perf_pmu_alias *alias;
bool has_sysfs_event;
- char event_file_name[FILENAME_MAX + 8];
+ char event_file_name[NAME_MAX + 8];
if (hashmap__find(pmu->aliases, name, &alias))
return alias;
@@ -752,7 +752,7 @@ static int pmu_aliases_parse(struct perf_pmu *pmu)
static int pmu_aliases_parse_eager(struct perf_pmu *pmu, int sysfs_fd)
{
- char path[FILENAME_MAX + 7];
+ char path[NAME_MAX + 8];
int ret, events_dir_fd;
scnprintf(path, sizeof(path), "%s/events", pmu->name);
--
2.50.0.727.gbf7dc18ff4-goog
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH v2 3/3] perf ui scripts: Switch FILENAME_MAX to NAME_MAX
2025-07-17 15:08 [PATCH v2 1/3] tools subcmd: Tighten the filename size in check_if_command_finished Ian Rogers
2025-07-17 15:08 ` [PATCH v2 2/3] perf pmu: Switch FILENAME_MAX to NAME_MAX Ian Rogers
@ 2025-07-17 15:08 ` Ian Rogers
2025-07-23 17:30 ` [PATCH v2 1/3] tools subcmd: Tighten the filename size in check_if_command_finished Namhyung Kim
2 siblings, 0 replies; 4+ messages in thread
From: Ian Rogers @ 2025-07-17 15:08 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Namhyung Kim, Mark Rutland, Alexander Shishkin, Jiri Olsa,
Ian Rogers, Adrian Hunter, Kan Liang, James Clark, linux-kernel,
linux-perf-users
FILENAME_MAX is the same as PATH_MAX (4kb) in glibc rather than
NAME_MAX's 255. Switch to using NAME_MAX and ensure the '\0' is
accounted for in the path's buffer size.
Signed-off-by: Ian Rogers <irogers@google.com>
---
tools/perf/ui/browsers/scripts.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/perf/ui/browsers/scripts.c b/tools/perf/ui/browsers/scripts.c
index 2d04ece833aa..1e8c2c2f952d 100644
--- a/tools/perf/ui/browsers/scripts.c
+++ b/tools/perf/ui/browsers/scripts.c
@@ -94,7 +94,7 @@ static int check_ev_match(int dir_fd, const char *scriptname, struct perf_sessio
FILE *fp;
{
- char filename[FILENAME_MAX + 5];
+ char filename[NAME_MAX + 5];
int fd;
scnprintf(filename, sizeof(filename), "bin/%s-record", scriptname);
--
2.50.0.727.gbf7dc18ff4-goog
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v2 1/3] tools subcmd: Tighten the filename size in check_if_command_finished
2025-07-17 15:08 [PATCH v2 1/3] tools subcmd: Tighten the filename size in check_if_command_finished Ian Rogers
2025-07-17 15:08 ` [PATCH v2 2/3] perf pmu: Switch FILENAME_MAX to NAME_MAX Ian Rogers
2025-07-17 15:08 ` [PATCH v2 3/3] perf ui scripts: " Ian Rogers
@ 2025-07-23 17:30 ` Namhyung Kim
2 siblings, 0 replies; 4+ messages in thread
From: Namhyung Kim @ 2025-07-23 17:30 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Mark Rutland, Alexander Shishkin, Jiri Olsa, Adrian Hunter,
Kan Liang, James Clark, linux-kernel, linux-perf-users,
Ian Rogers
On Thu, 17 Jul 2025 08:08:53 -0700, Ian Rogers wrote:
> FILENAME_MAX is often PATH_MAX (4kb), far more than needed for the
> /proc path. Make the buffer size sufficient for the maximum integer
> plus "/proc/" and "/status" with a '\0' terminator.
>
>
Applied to perf-tools-next, thanks!
Best regards,
Namhyung
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2025-07-23 17:30 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-07-17 15:08 [PATCH v2 1/3] tools subcmd: Tighten the filename size in check_if_command_finished Ian Rogers
2025-07-17 15:08 ` [PATCH v2 2/3] perf pmu: Switch FILENAME_MAX to NAME_MAX Ian Rogers
2025-07-17 15:08 ` [PATCH v2 3/3] perf ui scripts: " Ian Rogers
2025-07-23 17:30 ` [PATCH v2 1/3] tools subcmd: Tighten the filename size in check_if_command_finished Namhyung Kim
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).