* [PATCH] ACPI: tools: pfrut: Check if the input of level and type is in the right numeric range
@ 2023-03-07 11:41 Chen Yu
2023-03-07 11:38 ` Rafael J. Wysocki
0 siblings, 1 reply; 3+ messages in thread
From: Chen Yu @ 2023-03-07 11:41 UTC (permalink / raw)
To: Rafael J. Wysocki, Len Brown, Robert Moore
Cc: linux-acpi, acpica-devel, linux-kernel, Chen Yu,
Hariganesh Govindarajulu
The user can provide arbitrary non-numeic value to level and type, which
brings unexpected behavior:
pfrut -h
usage: pfrut [OPTIONS]
code injection:
-l, --load
-s, --stage
-a, --activate
-u, --update [stage and activate]
-q, --query
-d, --revid
update telemetry:
-G, --getloginfo
-T, --type(0:execution, 1:history)
-L, --level(0, 1, 2, 4)
-R, --read
-D, --revid log
pfrut -T A
pfrut -G
log_level:0
log_type:0
log_revid:2
max_data_size:65536
chunk1_size:0
chunk2_size:1530
rollover_cnt:0
reset_cnt:17
Fix this by restricting the input is in the expected range.
Reported-by: Hariganesh Govindarajulu <hariganesh.govindarajulu@intel.com>
Signed-off-by: Chen Yu <yu.c.chen@intel.com>
---
tools/power/acpi/tools/pfrut/pfrut.c | 33 ++++++++++++++++++++++++++++
1 file changed, 33 insertions(+)
diff --git a/tools/power/acpi/tools/pfrut/pfrut.c b/tools/power/acpi/tools/pfrut/pfrut.c
index 52aa0351533c..ed672efef83b 100644
--- a/tools/power/acpi/tools/pfrut/pfrut.c
+++ b/tools/power/acpi/tools/pfrut/pfrut.c
@@ -22,6 +22,7 @@
#include <sys/ioctl.h>
#include <sys/mman.h>
#include <uuid/uuid.h>
+#include <ctype.h>
#include PFRUT_HEADER
char *capsule_name;
@@ -77,6 +78,18 @@ static void help(void)
progname);
}
+static int is_digit_input(char *str)
+{
+ char *scan;
+
+ for (scan = str; *scan != '\0'; scan++) {
+ if (!isdigit(*scan))
+ return 0;
+ }
+
+ return 1;
+}
+
char *option_string = "l:sauqd:GT:L:RD:h";
static struct option long_options[] = {
{"load", required_argument, 0, 'l'},
@@ -125,11 +138,31 @@ static void parse_options(int argc, char **argv)
log_getinfo = 1;
break;
case 'T':
+ if (!is_digit_input(optarg)) {
+ printf("Please provide numeric value for type (0:execution, 1:history) - Exiting.\n");
+ exit(1);
+ }
+
log_type = atoi(optarg);
+ if (log_type != 0 && log_type != 1) {
+ printf("Please provide numeric value for type (0:execution, 1:history) - Exiting.\n");
+ exit(1);
+ }
+
set_log_type = 1;
break;
case 'L':
+ if (!is_digit_input(optarg)) {
+ printf("Please provide numeric value for level (0, 1, 2, 4) - Exiting.\n");
+ exit(1);
+ }
+
log_level = atoi(optarg);
+ if (log_level > 4) {
+ printf("Please provide numeric value for level (0, 1, 2, 4) - Exiting.\n");
+ exit(1);
+ }
+
set_log_level = 1;
break;
case 'R':
--
2.25.1
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH] ACPI: tools: pfrut: Check if the input of level and type is in the right numeric range
2023-03-07 11:41 [PATCH] ACPI: tools: pfrut: Check if the input of level and type is in the right numeric range Chen Yu
@ 2023-03-07 11:38 ` Rafael J. Wysocki
2023-03-08 4:20 ` Chen Yu
0 siblings, 1 reply; 3+ messages in thread
From: Rafael J. Wysocki @ 2023-03-07 11:38 UTC (permalink / raw)
To: Chen Yu
Cc: Rafael J. Wysocki, Len Brown, Robert Moore, linux-acpi,
acpica-devel, linux-kernel, Hariganesh Govindarajulu
On Tue, Mar 7, 2023 at 4:46 AM Chen Yu <yu.c.chen@intel.com> wrote:
>
> The user can provide arbitrary non-numeic value to level and type, which
> brings unexpected behavior:
So I guess the expected behavior would be to throw an error?
> pfrut -h
> usage: pfrut [OPTIONS]
> code injection:
> -l, --load
> -s, --stage
> -a, --activate
> -u, --update [stage and activate]
> -q, --query
> -d, --revid
> update telemetry:
> -G, --getloginfo
> -T, --type(0:execution, 1:history)
> -L, --level(0, 1, 2, 4)
> -R, --read
> -D, --revid log
>
> pfrut -T A
> pfrut -G
> log_level:0
> log_type:0
> log_revid:2
> max_data_size:65536
> chunk1_size:0
> chunk2_size:1530
> rollover_cnt:0
> reset_cnt:17
>
> Fix this by restricting the input is in the expected range.
>
> Reported-by: Hariganesh Govindarajulu <hariganesh.govindarajulu@intel.com>
> Signed-off-by: Chen Yu <yu.c.chen@intel.com>
> ---
> tools/power/acpi/tools/pfrut/pfrut.c | 33 ++++++++++++++++++++++++++++
> 1 file changed, 33 insertions(+)
>
> diff --git a/tools/power/acpi/tools/pfrut/pfrut.c b/tools/power/acpi/tools/pfrut/pfrut.c
> index 52aa0351533c..ed672efef83b 100644
> --- a/tools/power/acpi/tools/pfrut/pfrut.c
> +++ b/tools/power/acpi/tools/pfrut/pfrut.c
> @@ -22,6 +22,7 @@
> #include <sys/ioctl.h>
> #include <sys/mman.h>
> #include <uuid/uuid.h>
> +#include <ctype.h>
> #include PFRUT_HEADER
>
> char *capsule_name;
> @@ -77,6 +78,18 @@ static void help(void)
> progname);
> }
>
> +static int is_digit_input(char *str)
> +{
> + char *scan;
> +
> + for (scan = str; *scan != '\0'; scan++) {
> + if (!isdigit(*scan))
> + return 0;
> + }
> +
> + return 1;
> +}
> +
> char *option_string = "l:sauqd:GT:L:RD:h";
> static struct option long_options[] = {
> {"load", required_argument, 0, 'l'},
> @@ -125,11 +138,31 @@ static void parse_options(int argc, char **argv)
> log_getinfo = 1;
> break;
> case 'T':
> + if (!is_digit_input(optarg)) {
> + printf("Please provide numeric value for type (0:execution, 1:history) - Exiting.\n");
> + exit(1);
> + }
> +
Wouldn't using strtol() instead of atoi() work?
> log_type = atoi(optarg);
> + if (log_type != 0 && log_type != 1) {
> + printf("Please provide numeric value for type (0:execution, 1:history) - Exiting.\n");
> + exit(1);
> + }
> +
> set_log_type = 1;
> break;
> case 'L':
> + if (!is_digit_input(optarg)) {
> + printf("Please provide numeric value for level (0, 1, 2, 4) - Exiting.\n");
> + exit(1);
> + }
> +
> log_level = atoi(optarg);
> + if (log_level > 4) {
> + printf("Please provide numeric value for level (0, 1, 2, 4) - Exiting.\n");
> + exit(1);
> + }
> +
> set_log_level = 1;
> break;
> case 'R':
> --
> 2.25.1
>
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] ACPI: tools: pfrut: Check if the input of level and type is in the right numeric range
2023-03-07 11:38 ` Rafael J. Wysocki
@ 2023-03-08 4:20 ` Chen Yu
0 siblings, 0 replies; 3+ messages in thread
From: Chen Yu @ 2023-03-08 4:20 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Rafael J. Wysocki, Len Brown, Robert Moore, linux-acpi,
acpica-devel, linux-kernel, Hariganesh Govindarajulu
Hi Rafael,
thanks for the review,
On 2023-03-07 at 12:38:02 +0100, Rafael J. Wysocki wrote:
> On Tue, Mar 7, 2023 at 4:46 AM Chen Yu <yu.c.chen@intel.com> wrote:
> >
> > The user can provide arbitrary non-numeic value to level and type, which
> > brings unexpected behavior:
>
> So I guess the expected behavior would be to throw an error?
>
Yes.
> > pfrut -h
> > usage: pfrut [OPTIONS]
> > code injection:
> > -l, --load
> > -s, --stage
> > -a, --activate
> > -u, --update [stage and activate]
> > -q, --query
> > -d, --revid
> > update telemetry:
> > -G, --getloginfo
> > -T, --type(0:execution, 1:history)
> > -L, --level(0, 1, 2, 4)
> > -R, --read
> > -D, --revid log
> >
> > pfrut -T A
> > pfrut -G
> > log_level:0
> > log_type:0
> > log_revid:2
> > max_data_size:65536
> > chunk1_size:0
> > chunk2_size:1530
> > rollover_cnt:0
> > reset_cnt:17
> >
> > Fix this by restricting the input is in the expected range.
> >
> > Reported-by: Hariganesh Govindarajulu <hariganesh.govindarajulu@intel.com>
> > Signed-off-by: Chen Yu <yu.c.chen@intel.com>
> > ---
> > tools/power/acpi/tools/pfrut/pfrut.c | 33 ++++++++++++++++++++++++++++
> > 1 file changed, 33 insertions(+)
> >
> > diff --git a/tools/power/acpi/tools/pfrut/pfrut.c b/tools/power/acpi/tools/pfrut/pfrut.c
> > index 52aa0351533c..ed672efef83b 100644
> > --- a/tools/power/acpi/tools/pfrut/pfrut.c
> > +++ b/tools/power/acpi/tools/pfrut/pfrut.c
> > @@ -22,6 +22,7 @@
> > #include <sys/ioctl.h>
> > #include <sys/mman.h>
> > #include <uuid/uuid.h>
> > +#include <ctype.h>
> > #include PFRUT_HEADER
> >
> > char *capsule_name;
> > @@ -77,6 +78,18 @@ static void help(void)
> > progname);
> > }
> >
> > +static int is_digit_input(char *str)
> > +{
> > + char *scan;
> > +
> > + for (scan = str; *scan != '\0'; scan++) {
> > + if (!isdigit(*scan))
> > + return 0;
> > + }
> > +
> > + return 1;
> > +}
> > +
> > char *option_string = "l:sauqd:GT:L:RD:h";
> > static struct option long_options[] = {
> > {"load", required_argument, 0, 'l'},
> > @@ -125,11 +138,31 @@ static void parse_options(int argc, char **argv)
> > log_getinfo = 1;
> > break;
> > case 'T':
> > + if (!is_digit_input(optarg)) {
> > + printf("Please provide numeric value for type (0:execution, 1:history) - Exiting.\n");
> > + exit(1);
> > + }
> > +
>
> Wouldn't using strtol() instead of atoi() work?
>
Yes, strtol() would be simpler:
c = strtol(optarg, &endptr, 0)
if (*endptr)
exit(1);
I'll change it in next version.
thanks,
Chenyu
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2023-03-08 4:21 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-03-07 11:41 [PATCH] ACPI: tools: pfrut: Check if the input of level and type is in the right numeric range Chen Yu
2023-03-07 11:38 ` Rafael J. Wysocki
2023-03-08 4:20 ` Chen Yu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox