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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> 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)?

Jan


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