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

From: Daniel Kiper <daniel.kiper@oracle.com>
To: Jan Beulich <JBeulich@suse.com>
Cc: Juergen Gross <JGross@suse.com>,
	sstabellini@kernel.org, andrew.cooper3@citrix.com,
	cardoe@cardoe.com, pgnet.dev@gmail.com, ning.sun@intel.com,
	david.vrabel@citrix.com, xen-devel@lists.xenproject.org,
	qiaowei.ren@intel.com, gang.wei@intel.com, fu.wei@linaro.org
Subject: Re: [PATCH v5 14/16] x86/boot: implement early command line parser in C
Date: Wed, 31 Aug 2016 21:31:36 +0200	[thread overview]
Message-ID: <20160831193136.GH5364@olila.local.net-space.pl> (raw)
In-Reply-To: <57C6F136020000780010A7C6@prv-mh.provo.novell.com>

On Wed, Aug 31, 2016 at 07:01:10AM -0600, Jan Beulich wrote:
> >>> On 30.08.16 at 21:58, <daniel.kiper@oracle.com> wrote:
> > On Thu, Aug 25, 2016 at 07:27:21AM -0600, Jan Beulich wrote:
> >> >>> On 20.08.16 at 00:43, <daniel.kiper@oracle.com> 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@lists.xen.org
https://lists.xen.org/xen-devel