xfs
[Top] [All Lists]

Re: [PATCH] Catch under/overflow cases in cvtnum() and cvttime().

To: Eric Sandeen <sandeen@xxxxxxxxxxx>
Subject: Re: [PATCH] Catch under/overflow cases in cvtnum() and cvttime().
From: Arkadiusz MiÅkiewicz <arekm@xxxxxxxx>
Date: Sat, 12 Jul 2014 08:13:27 +0200
Cc: xfs@xxxxxxxxxxx
Delivered-to: xfs@xxxxxxxxxxx
Dkim-signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=maven.pl; s=maven; h=from:to:subject:date:user-agent:cc:references:in-reply-to :mime-version:content-type:content-transfer-encoding:message-id; bh=mmc3yFMnosJinp/sLkAL1PEHp+gppQ6mI9eHfmUNvxI=; b=UL1donOj8598LgR6M7fgNZnP3DMuyRRrnQPVGrYTLAQtbycUYUxuzDa51dI5j2B36/ dBkJsGsli4nJWH1uOZHqD/2t61qoSy+8eYN/uzTD0nzW3FTpWBK4T0HPZINYorO9X/N9 Jez036C5OhkWP6UKvMCgwzqny1jhFUa9SWXus=
In-reply-to: <53C06FF3.2090600@xxxxxxxxxxx>
References: <1405107244-14234-1-git-send-email-arekm@xxxxxxxx> <53C06FF3.2090600@xxxxxxxxxxx>
User-agent: KMail/1.13.7 (Linux/3.16.0-rc4-00120-g85d90fa-dirty; KDE/4.13.2; x86_64; ; )
On Saturday 12 of July 2014, Eric Sandeen wrote:
> On 7/11/14, 2:34 PM, Arkadiusz MiÅkiewicz wrote:
> > cvtnum() and cvttime() silently ignore overflows. This leads to error
> > conditions not being catched. Example:
> > 
> > $ xfs_quota -x -c 'limit -u bsoft=987654321098765432199 \
> > 
> >         bhard=987654321098765432199 999' /
> > 
> > $
> > 
> > Fixed version:
> > $ xfs_quota -x -c 'limit -u bsoft=987654321098765432199 \
> > 
> >         bhard=987654321098765432199 999' /
> > 
> > xfs_quota: Error: could not parse size 987654321098765432199.
> > xfs_quota: unrecognised argument bsoft=987654321098765432199
> 
> So, strtol(3) suggests setting errno to 0 before the call:
> 
> NOTES
>        Since  strtol()  can  legitimately  return  0,  LONG_MAX,  or 
> LONG_MIN (LLONG_MAX or LLONG_MIN for strtoll()) on both success and
> failure, the calling  program should set errno to 0 before the call, and
> then deter- mine if an error occurred by checking  whether  errno  has  a 
> non-zero value after the call.
> 
> Ditto for strtoul().

Hm, my man pages 3.70 don't have such notes, strtol(3):

NOTES
       In locales other than the "C" locale, also other strings may be 
accepted.  (For example, the thousands separator of the current locale may be 
supported.)

       BSD also has

           quad_t
           strtoq(const char *nptr, char **endptr, int base);

       with completely analogous definition.  Depending on the wordsize of the 
current architecture, this may be equivalent to strtoll() or to strtol().

> 
> I guess that is just to ensure that there's not a leftover errno
> when we make the call?  Worth doing, maybe?

ERANGE is checked in few other places already in input.c and none initialize 
errno before strtoul() call.

> 
> Thanks,
> -Eric
> 
> > Signed-off-by: Arkadiusz MiÅkiewicz <arekm@xxxxxxxx>
> > ---
> > 
> >  libxcmd/input.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> > 
> > diff --git a/libxcmd/input.c b/libxcmd/input.c
> > index c06b5b8..397a124 100644
> > --- a/libxcmd/input.c
> > +++ b/libxcmd/input.c
> > @@ -154,6 +154,8 @@ cvtnum(
> > 
> >     int             c;
> >     
> >     i = strtoll(s, &sp, 0);
> > 
> > +   if ((i == LLONG_MIN || i == LLONG_MAX) && errno == ERANGE)
> > +           return -1LL;
> > 
> >     if (i == 0 && sp == s)
> >     
> >             return -1LL;
> >     
> >     if (*sp == '\0')
> > 
> > @@ -238,6 +240,8 @@ cvttime(
> > 
> >     char            *sp;
> >     
> >     i = strtoul(s, &sp, 0);
> > 
> > +   if (i == ULONG_MAX && errno == ERANGE)
> > +           return 0;
> > 
> >     if (i == 0 && sp == s)
> >     
> >             return 0;
> >     
> >     if (*sp == '\0')


-- 
Arkadiusz MiÅkiewicz, arekm / maven.pl

<Prev in Thread] Current Thread [Next in Thread>