[2/2] mmc-utils: add error handling to 'extcsd write' input value parsing

So far, any value was silently accepted, even a

  mmc extcsd write 0x100 yeah /dev/mmcblk1

which resulted in write of value 0x0 at offset 0x0 since the parsing
error of 'yeah' was ignored and the returned value of 0 was taken
unconditionally.
The 0x100 was then implicitly casted down to the expected __u8 input for
the offset and also ended up as 0.

This is not the behavior one would expect when dealing with eMMC
registers that might, depending on which we write to, be writable only
once in the eMMC's lifetime.

This introduces the str_to_u8() helper function that checks if input
values are parsable and have a proper size. Internally it also uses
strtoul() instead of strtol() since input is always expected to be
non-negative. Also, use proper input variables (unsigned long instead
of int) to remove another layer of implicit down casts.

Signed-off-by: Enrico Jorns <ejo@pengutronix.de>
---
 mmc_cmds.c | 24 +++++++++++++++++++++---
 1 file changed, 21 insertions(+), 3 deletions(-)

> -----Original Message-----
> From: Enrico Jorns <ejo@pengutronix.de>
> Sent: Tuesday, May 23, 2023 12:53 AM
> To: linux-mmc@vger.kernel.org
> Cc: Avri Altman <Avri.Altman@wdc.com>; Ulf Hansson
> <ulf.hansson@linaro.org>; ejo@pengutronix.de
> Subject: [PATCH 2/2] mmc-utils: add error handling to 'extcsd write' input
> value parsing
> 
> CAUTION: This email originated from outside of Western Digital. Do not click
> on links or open attachments unless you recognize the sender and know that
> the content is safe.
> 
> 
> So far, any value was silently accepted, even a
> 
>   mmc extcsd write 0x100 yeah /dev/mmcblk1
> 
> which resulted in write of value 0x0 at offset 0x0 since the parsing error of
> 'yeah' was ignored and the returned value of 0 was taken unconditionally.
> The 0x100 was then implicitly casted down to the expected __u8 input for the
> offset and also ended up as 0.
The largest writable offset is 191 - maybe use this for valid offset.
There are writable fields that are more than a single byte in size.
Are you still allowing writing those?

Thanks,
Avri


> 
> This is not the behavior one would expect when dealing with eMMC registers
> that might, depending on which we write to, be writable only once in the
> eMMC's lifetime.
> 
> This introduces the str_to_u8() helper function that checks if input values are
> parsable and have a proper size. Internally it also uses
> strtoul() instead of strtol() since input is always expected to be non-negative.
> Also, use proper input variables (unsigned long instead of int) to remove
> another layer of implicit down casts.
> 
> Signed-off-by: Enrico Jorns <ejo@pengutronix.de>
> ---
>  mmc_cmds.c | 24 +++++++++++++++++++++---
>  1 file changed, 21 insertions(+), 3 deletions(-)
> 
> diff --git a/mmc_cmds.c b/mmc_cmds.c
> index 154020e..e348651 100644
> --- a/mmc_cmds.c
> +++ b/mmc_cmds.c
> @@ -1981,10 +1981,28 @@ out_free:
>         return ret;
>  }
> 
> +__u8 str_to_u8(const char* input)
> +{
> +       unsigned long parsedval;
> +       char *endptr;
> +
> +       parsedval = strtoul(input, &endptr, 0);
> +       if (*endptr != '\0') {
> +               fprintf(stderr, "Invalid input: %s. Cannot parse to 8 bit integer.\n",
> input);
> +               exit(1);
> +       }
> +       if (parsedval >= UINT8_MAX) {
> +               fprintf(stderr, "Invalid input: Value %s too large 8 bit integer.\n",
> input);
> +               exit(1);
> +       }
> +
> +       return (__u8) parsedval;
> +}
> +
>  int do_write_extcsd(int nargs, char **argv)  {
>         int fd, ret;
> -       int offset, value;
> +       __u8 offset, value;
>         char *device;
>         int verify = 0;
> 
> @@ -1993,8 +2011,8 @@ int do_write_extcsd(int nargs, char **argv)
>                 exit(1);
>         }
> 
> -       offset = strtol(argv[1], NULL, 0);
> -       value  = strtol(argv[2], NULL, 0);
> +       offset = str_to_u8(argv[1]);
> +       value = str_to_u8(argv[2]);
>         device = argv[3];
> 
>         if (nargs == 5) {
> --
> 2.39.2