Re: [PATCH] lib/parser.c: avoid overflow in match_number()

From: Randy Dunlap
Date: Fri Aug 10 2012 - 13:27:33 EST


On 08/09/2012 01:03 PM, Alex Elder wrote:

> The result of converting an integer value to another signed integer
> type that's unable to represent the original value is implementation
> defined. (See notes in section 6.3.1.3 of the C standard.)
>
> In match_number(), the result of simple_strtol() (which returns type
> long) is assigned to a value of type int.
>
> Instead, handle the result of simple_strtol() in a well-defined way,
> and return -ERANGE if the result won't fit in the int variable used
> to hold the parsed result.
>
> No current callers pay attention to the particular error value
> returned, so this additional return code shouldn't do any harm.
>
> Signed-off-by: Alex Elder <elder@xxxxxxxxxxx>


Makes sense to me, but I wonder who will merge it.

I'm Cc-ing a couple of possibilities.

> ---
> lib/parser.c | 10 ++++++++--
> 1 file changed, 8 insertions(+), 2 deletions(-)
>
> Index: b/lib/parser.c
> ===================================================================
> --- a/lib/parser.c
> +++ b/lib/parser.c
> @@ -122,13 +122,14 @@ int match_token(char *s, const match_tab
> *
> * Description: Given a &substring_t and a base, attempts to parse the substring
> * as a number in that base. On success, sets @result to the integer represented
> - * by the string and returns 0. Returns either -ENOMEM or -EINVAL on failure.
> + * by the string and returns 0. Returns -ENOMEM, -EINVAL, or -ERANGE on failure.
> */
> static int match_number(substring_t *s, int *result, int base)
> {
> char *endp;
> char *buf;
> int ret;
> + long val;
> size_t len = s->to - s->from;
>
> buf = kmalloc(len + 1, GFP_KERNEL);
> @@ -136,10 +137,15 @@ static int match_number(substring_t *s,
> return -ENOMEM;
> memcpy(buf, s->from, len);
> buf[len] = '\0';
> - *result = simple_strtol(buf, &endp, base);
> +
> ret = 0;
> + val = simple_strtol(buf, &endp, base);
> if (endp == buf)
> ret = -EINVAL;
> + else if (val < (long) INT_MIN || val > (long) INT_MAX)
> + ret = -ERANGE;
> + else
> + *result = (int) val;
> kfree(buf);
> return ret;
> }
> --


--
~Randy
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/