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@xxxxxxxxx> 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@xxxxxxxxx>
> > Signed-off-by: Chen Yu <yu.c.chen@xxxxxxxxx>
> > ---
> > 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
