xfs
[Top] [All Lists]

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

To: Arkadiusz MiÅkiewicz <arekm@xxxxxxxx>, xfs@xxxxxxxxxxx
Subject: Re: [PATCH] Catch under/overflow cases in cvtnum() and cvttime().
From: Eric Sandeen <sandeen@xxxxxxxxxxx>
Date: Fri, 11 Jul 2014 18:14:59 -0500
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <1405107244-14234-1-git-send-email-arekm@xxxxxxxx>
References: <1405107244-14234-1-git-send-email-arekm@xxxxxxxx>
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().

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

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

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