Messages in this thread | | | Date | Fri, 10 Aug 2012 10:26:12 -0700 | From | Randy Dunlap <> | Subject | Re: [PATCH] lib/parser.c: avoid overflow in match_number() |
| |
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@inktank.com>
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
| |