> -----Original Message-----
> From: Enrico Jorns <ejo@xxxxxxxxxxxxxx>
> Sent: Tuesday, May 23, 2023 12:53 AM
> To: linux-mmc@xxxxxxxxxxxxxxx
> Cc: Avri Altman <Avri.Altman@xxxxxxx>; Ulf Hansson
> <ulf.hansson@xxxxxxxxxx>; ejo@xxxxxxxxxxxxxx
> 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@xxxxxxxxxxxxxx>
> ---
> 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
