The atoi() function does not perform error checking, which can lead to
undefined behavior when parsing invalid or out-of-range strings. This
can cause issues when parsing user-provided numerical inputs, such as
signal numbers, PIDs, or CPU lists.
To address this, introduce a new strtoi() helper function that safely
converts a string to an integer. This function validates the input and
checks for overflows, returning a boolean to indicate success or failure.
Replace all calls to atoi() with the new strtoi() function and add
proper error handling to make the parsing more robust and prevent
potential issues.
Signed-off-by: Wander Lairson Costa <wander@redhat.com>
---
tools/tracing/rtla/src/actions.c | 6 +++--
tools/tracing/rtla/src/utils.c | 40 ++++++++++++++++++++++++++++----
tools/tracing/rtla/src/utils.h | 2 ++
3 files changed, 41 insertions(+), 7 deletions(-)
diff --git a/tools/tracing/rtla/src/actions.c b/tools/tracing/rtla/src/actions.c
index efa17290926da..e23d4f1c5a592 100644
--- a/tools/tracing/rtla/src/actions.c
+++ b/tools/tracing/rtla/src/actions.c
@@ -199,12 +199,14 @@ actions_parse(struct actions *self, const char *trigger, const char *tracefn)
/* Takes two arguments, num (signal) and pid */
while (token != NULL) {
if (strlen(token) > 4 && strncmp(token, "num=", 4) == 0) {
- signal = atoi(token + 4);
+ if(!strtoi(token + 4, &signal))
+ return -1;
} else if (strlen(token) > 4 && strncmp(token, "pid=", 4) == 0) {
if (strncmp(token + 4, "parent", 7) == 0)
pid = -1;
else
- pid = atoi(token + 4);
+ if (!strtoi(token + 4, &pid))
+ return -1;
} else {
/* Invalid argument */
return -1;
diff --git a/tools/tracing/rtla/src/utils.c b/tools/tracing/rtla/src/utils.c
index d6ab15dcb4907..4cb765b94feec 100644
--- a/tools/tracing/rtla/src/utils.c
+++ b/tools/tracing/rtla/src/utils.c
@@ -17,6 +17,7 @@
#include <fcntl.h>
#include <sched.h>
#include <stdio.h>
+#include <limits.h>
#include "utils.h"
@@ -112,16 +113,18 @@ int parse_cpu_set(char *cpu_list, cpu_set_t *set)
nr_cpus = sysconf(_SC_NPROCESSORS_CONF);
for (p = cpu_list; *p; ) {
- cpu = atoi(p);
- if (cpu < 0 || (!cpu && *p != '0') || cpu >= nr_cpus)
+ if (!strtoi(p, &cpu))
+ goto err;
+ if (cpu < 0 || cpu >= nr_cpus)
goto err;
while (isdigit(*p))
p++;
if (*p == '-') {
p++;
- end_cpu = atoi(p);
- if (end_cpu < cpu || (!end_cpu && *p != '0') || end_cpu >= nr_cpus)
+ if (!strtoi(p, &end_cpu))
+ goto err;
+ if (end_cpu < cpu || end_cpu >= nr_cpus)
goto err;
while (isdigit(*p))
p++;
@@ -322,6 +325,7 @@ int set_comm_sched_attr(const char *comm_prefix, struct sched_attr *attr)
struct dirent *proc_entry;
DIR *procfs;
int retval;
+ int pid;
if (strlen(comm_prefix) >= MAX_PATH) {
err_msg("Command prefix is too long: %d < strlen(%s)\n",
@@ -341,8 +345,12 @@ int set_comm_sched_attr(const char *comm_prefix, struct sched_attr *attr)
if (!retval)
continue;
+ if (!strtoi(proc_entry->d_name, &pid)) {
+ err_msg("'%s' is not a valid pid", proc_entry->d_name);
+ goto out_err;
+ }
/* procfs_is_workload_pid confirmed it is a pid */
- retval = __set_sched_attr(atoi(proc_entry->d_name), attr);
+ retval = __set_sched_attr(pid, attr);
if (retval) {
err_msg("Error setting sched attributes for pid:%s\n", proc_entry->d_name);
goto out_err;
@@ -959,3 +967,25 @@ int auto_house_keeping(cpu_set_t *monitored_cpus)
return 1;
}
+
+/*
+ * strtoi - convert string to integer with error checking
+ *
+ * Returns true on success, false if conversion fails or result is out of int range.
+ */
+bool strtoi(const char *s, int *res)
+{
+ char *end_ptr;
+ long lres;
+
+ if (!*s)
+ return false;
+
+ errno = 0;
+ lres = strtol(s, &end_ptr, 0);
+ if (errno || *end_ptr || lres > INT_MAX || lres < INT_MIN)
+ return false;
+
+ *res = (int) lres;
+ return true;
+}
diff --git a/tools/tracing/rtla/src/utils.h b/tools/tracing/rtla/src/utils.h
index a2a6f89f342d0..160491f5de91c 100644
--- a/tools/tracing/rtla/src/utils.h
+++ b/tools/tracing/rtla/src/utils.h
@@ -3,6 +3,7 @@
#include <stdint.h>
#include <time.h>
#include <sched.h>
+#include <stdbool.h>
/*
* '18446744073709551615\0'
@@ -80,6 +81,7 @@ static inline int set_deepest_cpu_idle_state(unsigned int cpu, unsigned int stat
static inline int have_libcpupower_support(void) { return 0; }
#endif /* HAVE_LIBCPUPOWER_SUPPORT */
int auto_house_keeping(cpu_set_t *monitored_cpus);
+bool strtoi(const char *s, int *res);
#define ns_to_usf(x) (((double)x/1000))
#define ns_to_per(total, part) ((part * 100) / (double)total)
--
2.51.1On Mon, 17 Nov 2025 at 20:55, Wander Lairson Costa <wander@redhat.com> wrote: > To address this, introduce a new strtoi() helper function that safely > converts a string to an integer. This function validates the input and > checks for overflows, returning a boolean to indicate success or failure. Why not use sscanf() for this purpose instead of adding a new utility function? Also, using a boolean to return success or failure does not conform to POSIX standards and is confusing in Linux/POSIX code. Costa
On Tue, Nov 25, 2025 at 10:35:39AM +0200, Costa Shulyupin wrote:
> On Mon, 17 Nov 2025 at 20:55, Wander Lairson Costa <wander@redhat.com> wrote:
> > To address this, introduce a new strtoi() helper function that safely
> > converts a string to an integer. This function validates the input and
> > checks for overflows, returning a boolean to indicate success or failure.
>
> Why not use sscanf() for this purpose instead of adding a new utility function?
>
The strtoi implementation properly detects:
1. Empty strings - via the !*s check
2. Conversion errors - via errno from strtol
3. Trailing garbage - via *end_ptr check ensuring entire string was consumed
4. Integer overflow/underflow - via explicit lres > INT_MAX || lres < INT_MIN
bounds checking
sscanf has the following limitations:
1. Trailing garbage is silently ignored
int val;
sscanf("123abc", "%d", &val); /* Returns 1 (success), val=123, "abc" ignored */
While you could use "%d%n" with character count checking, this becomes
cumbersome and defeats the purpose of simplification.
2. Integer overflow has undefined behavior
sscanf with %d doesn't guarantee overflow detection and may silently wrap
values (e.g., 2147483648 -> -2147483648). There's no standard way to detect
this has occurred.
3. No detailed error reporting (this is minor, though)
sscanf only returns match count, not error type. You cannot distinguish
"bad format" from "overflow" from "trailing garbage".
> Also, using a boolean to return success or failure does not conform to
> POSIX standards and is confusing in Linux/POSIX code.
>
Ok, I will change it.
> Costa
>
On Mon, 2025-11-17 at 15:41 -0300, Wander Lairson Costa wrote:
>
> diff --git a/tools/tracing/rtla/src/actions.c b/tools/tracing/rtla/src/actions.c
> index efa17290926da..e23d4f1c5a592 100644
> --- a/tools/tracing/rtla/src/actions.c
> +++ b/tools/tracing/rtla/src/actions.c
> @@ -199,12 +199,14 @@ actions_parse(struct actions *self, const char *trigger, const char *tracefn)
> /* Takes two arguments, num (signal) and pid */
> while (token != NULL) {
> if (strlen(token) > 4 && strncmp(token, "num=", 4) == 0) {
> - signal = atoi(token + 4);
> + if(!strtoi(token + 4, &signal))
> + return -1;
if (
> } else if (strlen(token) > 4 && strncmp(token, "pid=", 4) == 0) {
> if (strncmp(token + 4, "parent", 7) == 0)
> pid = -1;
> else
> - pid = atoi(token + 4);
> + if (!strtoi(token + 4, &pid))
> + return -1;
else if (
Please run the patches through checkpatch.pl
> @@ -959,3 +967,25 @@ int auto_house_keeping(cpu_set_t *monitored_cpus)
>
> return 1;
> }
> +
> +/*
> + * strtoi - convert string to integer with error checking
> + *
> + * Returns true on success, false if conversion fails or result is out of int range.
> + */
> +bool strtoi(const char *s, int *res)
Could use __attribute__((__warn_unused_result__)) like kstrtoint().
BTW, it's pretty annoying that we need to reinvent the wheel on all this
stuff just because it's userspace. From some of the other tools it
looks like we can at least include basic kernel headers like compiler.h;
maybe we should have a tools/-wide common util area as well? Even
better if some of the code can be shared with the kernel itself.
Not saying that should in any way be a blocker for these patches, just
something to think about.
-Crystal
On Mon, Nov 24, 2025 at 9:46 PM Crystal Wood <crwood@redhat.com> wrote:
>
> On Mon, 2025-11-17 at 15:41 -0300, Wander Lairson Costa wrote:
>
> >
> > diff --git a/tools/tracing/rtla/src/actions.c b/tools/tracing/rtla/src/actions.c
> > index efa17290926da..e23d4f1c5a592 100644
> > --- a/tools/tracing/rtla/src/actions.c
> > +++ b/tools/tracing/rtla/src/actions.c
> > @@ -199,12 +199,14 @@ actions_parse(struct actions *self, const char *trigger, const char *tracefn)
> > /* Takes two arguments, num (signal) and pid */
> > while (token != NULL) {
> > if (strlen(token) > 4 && strncmp(token, "num=", 4) == 0) {
> > - signal = atoi(token + 4);
> > + if(!strtoi(token + 4, &signal))
> > + return -1;
>
> if (
>
> > } else if (strlen(token) > 4 && strncmp(token, "pid=", 4) == 0) {
> > if (strncmp(token + 4, "parent", 7) == 0)
> > pid = -1;
> > else
> > - pid = atoi(token + 4);
> > + if (!strtoi(token + 4, &pid))
> > + return -1;
>
> else if (
>
> Please run the patches through checkpatch.pl
>
Good catch, thanks.
> > @@ -959,3 +967,25 @@ int auto_house_keeping(cpu_set_t *monitored_cpus)
> >
> > return 1;
> > }
> > +
> > +/*
> > + * strtoi - convert string to integer with error checking
> > + *
> > + * Returns true on success, false if conversion fails or result is out of int range.
> > + */
> > +bool strtoi(const char *s, int *res)
>
> Could use __attribute__((__warn_unused_result__)) like kstrtoint().
>
Sure, I will do it in v2.
> BTW, it's pretty annoying that we need to reinvent the wheel on all this
> stuff just because it's userspace. From some of the other tools it
> looks like we can at least include basic kernel headers like compiler.h;
> maybe we should have a tools/-wide common util area as well? Even
> better if some of the code can be shared with the kernel itself.
>
> Not saying that should in any way be a blocker for these patches, just
> something to think about.
>
I thought the same thing some time ago.
>
> -Crystal
>
© 2016 - 2025 Red Hat, Inc.