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

[Daniel Kiper <daniel.kiper@xxxxxxxxxx>]

On Thu, Sep 01, 2016 at 01:41:26AM -0600, Jan Beulich wrote:
> >>> On 31.08.16 at 21:31, <daniel.kiper@xxxxxxxxxx> wrote:
> > 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.
>
> Aren't we in 32-bit code here, i.e. sizeof(int) == sizeof(long)?

Yep, this should be unsigned long long here.

Daniel

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