public inbox for linux-xfs@vger.kernel.org
 help / color / mirror / Atom feed
From: "Arkadiusz Miśkiewicz" <arekm@maven.pl>
To: Eric Sandeen <sandeen@sandeen.net>
Cc: xfs@oss.sgi.com
Subject: Re: [PATCH] Catch under/overflow cases in cvtnum() and cvttime().
Date: Sat, 12 Jul 2014 08:13:27 +0200	[thread overview]
Message-ID: <201407120813.27317.arekm@maven.pl> (raw)
In-Reply-To: <53C06FF3.2090600@sandeen.net>

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@maven.pl>
> > ---
> > 
> >  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

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

  reply	other threads:[~2014-07-12  6:13 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-07-11 19:34 [PATCH] Catch under/overflow cases in cvtnum() and cvttime() Arkadiusz Miśkiewicz
2014-07-11 23:14 ` Eric Sandeen
2014-07-12  6:13   ` Arkadiusz Miśkiewicz [this message]
2014-07-12 13:37     ` Eric Sandeen
2014-07-12 13:43       ` Eric Sandeen
2014-07-13 17:38 ` [PATCH] Init errno before strto* calls Arkadiusz Miśkiewicz
2014-07-13 23:04   ` Dave Chinner
2014-07-14  7:56     ` [PATCH] Detect strto* failures based on errno Arkadiusz Miśkiewicz
2014-07-16  0:00       ` Dave Chinner
2014-07-16  3:01       ` Dave Chinner
2014-07-16  7:44         ` Arkadiusz Miśkiewicz
2014-07-16 23:32           ` Dave Chinner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=201407120813.27317.arekm@maven.pl \
    --to=arekm@maven.pl \
    --cc=sandeen@sandeen.net \
    --cc=xfs@oss.sgi.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox