Prev: Spinlocks: Factor our GENERIC_LOCKBREAK in order to avoid spin with irqs disable
Next: avoid kobject name conflict with different namespaces
From: Alexey Dobriyan on 7 May 2008 15:20
On Wed, May 07, 2008 at 11:25:29AM -0700, Harvey Harrison wrote:
> Directly code the strict string conversion functions rather than using
> defining macros. Pull out a small helper to check the strict conditions
> required at the end of a string (nul-terminated or newline).

> Add additional checks in strict_strtol and strict_strtoll for numeric
> overflow of the signed types.

C interer ranges are asymmetric.

These "strict_" functions are a farce. No amount of afterchecking will
save you if there is trivial wraparound in core function.

Alexey "kstrtonum" Dobriyan


> --- a/lib/vsprintf.c
> +++ b/lib/vsprintf.c
> @@ -128,6 +128,18 @@ long long simple_strtoll(const char *cp, char **endp, unsigned int base)
> return simple_strtoull(cp, endp, base);
> }
>
> +/*
> + * Strictly check that the string is nul terminated or has a newline
> + * immediately following len chars.
> + */
> +static int strict_checktail(size_t len, const char *cp, const char *tail)

Name simply sucks.

> +{
> + if ((*tail == '\0') ||
> + ((len == (size_t)(tail - cp) + 1) && (*tail == '\n')))
> + return 1;
> + else
> + return 0;
> +}

> @@ -165,7 +196,29 @@ int strict_strtoul(const char *cp, unsigned int base, unsigned long *res);
> * It returns 0 if conversion is successful and *res is set to the converted
> * value, otherwise it returns -EINVAL and *res is set to 0.
> */
> -int strict_strtol(const char *cp, unsigned int base, long *res);
> +int strict_strtol(const char *cp, unsigned int base, long *res)
> +{
> + int ret;
> + unsigned long tmp;
> +
> + if (*cp == '-')
> + ret = strict_strtoul(cp + 1, base, &tmp);
> + else
> + ret = strict_strtoul(cp, base, &tmp);
> +
> + if (!ret || tmp > LONG_MAX) {
> + *res = 0;
> + return -EINVAL;
> + }
> +
> + if (*cp == '-')
> + *res = -tmp;
> + else
> + *res = tmp;
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(strict_strtol);
>
> /**
> * strict_strtoull - convert a string to an unsigned long long strictly

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Harvey Harrison on 7 May 2008 15:30

On Wed, 2008-05-07 at 12:20 -0700, Harvey Harrison wrote:
> On Thu, 2008-05-08 at 00:09 +0400, Alexey Dobriyan wrote:
> > On Wed, May 07, 2008 at 11:25:29AM -0700, Harvey Harrison wrote:
> > > Directly code the strict string conversion functions rather than using
> > > defining macros. Pull out a small helper to check the strict conditions
> > > required at the end of a string (nul-terminated or newline).
> >
> > > Add additional checks in strict_strtol and strict_strtoll for numeric
> > > overflow of the signed types.
> >
> > C interer ranges are asymmetric.
>
> Yes, and LLONG_MAX = -LLONG_MIN + 1...so it will reject string values
> of LLONG_MIN...easily fixed if we want it I guess.


Missed the braces above obviously....but the point stands.

Harvey

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Harvey Harrison on 7 May 2008 15:30
On Thu, 2008-05-08 at 00:09 +0400, Alexey Dobriyan wrote:
> On Wed, May 07, 2008 at 11:25:29AM -0700, Harvey Harrison wrote:
> > Directly code the strict string conversion functions rather than using
> > defining macros. Pull out a small helper to check the strict conditions
> > required at the end of a string (nul-terminated or newline).
>
> > Add additional checks in strict_strtol and strict_strtoll for numeric
> > overflow of the signed types.
>
> C interer ranges are asymmetric.

Yes, and LLONG_MAX = -LLONG_MIN + 1...so it will reject string values
of LLONG_MIN...easily fixed if we want it I guess.

>
> These "strict_" functions are a farce. No amount of afterchecking will
> save you if there is trivial wraparound in core function.
>

That's also true, I guess it depends on how far we want to go. And
after that point, just stick a WARN_ON and return -EINVAL.

I'm not sure where the balance is, but I think my patch is still on
the useful side of it.

> Alexey "kstrtonum" Dobriyan
>
> > + */
> > +static int strict_checktail(size_t len, const char *cp, const char *tail)
>
> Name simply sucks.

Sure does, care to suggest something better, at least it had a
comment ;-)

Anyways, thanks for taking a look.

Harvey

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
 | 
Pages: 1
Prev: Spinlocks: Factor our GENERIC_LOCKBREAK in order to avoid spin with irqs disable
Next: avoid kobject name conflict with different namespaces