Re: [Xen-devel] [PATCH v5 14/16] x86/boot: implement early command line parser in C

[Daniel Kiper <daniel.kiper@xxxxxxxxxx>]

On Wed, Aug 31, 2016 at 07:01:10AM -0600, Jan Beulich wrote:
> >>> On 30.08.16 at 21:58, <daniel.kiper@xxxxxxxxxx> wrote:
> > On Thu, Aug 25, 2016 at 07:27:21AM -0600, Jan Beulich wrote:
> >> >>> On 20.08.16 at 00:43, <daniel.kiper@xxxxxxxxxx> wrote:

[...]

> >> > +static unsigned int strtoui(const char *s, const char *stop, const char 
> >> > **next)
> >> > +{
> >> > +    char l;
> >> > +    unsigned int base = 10, ores = 0, res = 0;
> >> > +
> >> > +    if ( *s == '0' )
> >> > +      base = (tolower(*++s) == 'x') ? (++s, 16) : 8;
> >> > +
> >> > +    for ( ; *s != '\0'; ++s )
> >> > +    {
> >> > +        if ( stop && strchr(stop, *s) )
> >> > +            goto out;
> >> > +
> >> > +        if ( *s < '0' || (*s > '7' && base == 8) )
> >> > +        {
> >> > +            res = UINT_MAX;
> >> > +            goto out;
> >> > +        }
> >> > +
> >> > +        l = tolower(*s);
> >> > +
> >> > +        if ( *s > '9' && (base != 16 || l < 'a' || l > 'f') )
> >> > +        {
> >> > +            res = UINT_MAX;
> >> > +            goto out;
> >> > +        }
> >> > +
> >> > +        res *= base;
> >> > +        res += (l >= 'a') ? (l - 'a' + 10) : (*s - '0');
> >> > +
> >> > +        if ( ores > res )
> >> > +        {
> >> > +            res = UINT_MAX;
> >> > +            goto out;
> >> > +        }
> >>
> >> Without having spent time to try and find an example, it feels like this
> >> check won't catch all possible overflow conditions. If you care about
> >> overflow, please make sure you catch all cases.
> >
> > Hmmm.... How come? Could you give an example?
>
> Excuse me, but shouldn't you instead demonstrate the logic is
> correct? Or - consider what I had said - try to find an example
> yourself? It's not that difficult: With 16-bit word size
> 0x3333 * 10 = 0x1fffe, which truncates to 0xfffe and is hence
> larger than both inputs but still produced an overflow. This
> easily extends to 32- and 64-bit word size.

Oh, boy. I forgot about multiplication. I think that we can define
res as unsigned long and then check that it is < UINT_MAX.
If not then return UINT_MAX.

Daniel

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel