diff options
| author | Pranith Kumar K <pkarampu@redhat.com> | 2014-06-20 16:06:42 +0530 | 
|---|---|---|
| committer | Vijay Bellur <vbellur@redhat.com> | 2014-06-22 20:40:57 -0700 | 
| commit | f022af610d85e7b7cc4f63bf0491a738b5a6f082 (patch) | |
| tree | 35f18ffee8cb700f14d659e52f72a558674d0bb8 | |
| parent | 0c85a8a125eccbd9cf0028116700d7025429656f (diff) | |
libglusterfs: Don't allow '-0' as input value for numbers
Problem:
io-stats tries to init log-buf-size as uint32. All gf_string2u*** functions
which get the unsigned values from string don't want the string to contain '-'.
So the GF_OPTION_INIT with '-0' as value fails init in io-stats, but by that
time 'ret' is already reset to 0. Io-stats ends up returning 0 even when init
failed. Because of this caller of init thinks initialization is successful when
it is not. iostat_xlator->private is still NULL.  Because of this when a fop
tries to access members of io-stat-private structure, it crashes.
Fix:
I initially thought may be we should fix all gf_string2u*** functions to
accept '-0'. But all these functions are used only for setting volume options.
If we accept '-0', gluster volume info shows output as follows:
diagnostics.brick-log-buf-size: -0
This seemed ugly, so I felt it is better to disallow '-0' as valid input for
numbers.
Also fixed return value in cases of failures in io-stats.
Change-Id: I67ac92853b6d2be70516ad1d07505ffd9f058aa4
BUG: 1111557
Signed-off-by: Pranith Kumar K <pkarampu@redhat.com>
Reviewed-on: http://review.gluster.org/8129
Reviewed-by: Krutika Dhananjay <kdhananj@redhat.com>
Tested-by: Gluster Build System <jenkins@build.gluster.com>
Reviewed-by: Harshavardhana <harsha@harshavardhana.net>
Tested-by: Harshavardhana <harsha@harshavardhana.net>
Reviewed-by: Vijay Bellur <vbellur@redhat.com>
| -rw-r--r-- | libglusterfs/src/options.c | 10 | ||||
| -rw-r--r-- | tests/bugs/bug-1111557.t | 12 | ||||
| -rw-r--r-- | xlators/debug/io-stats/src/io-stats.c | 3 | 
3 files changed, 25 insertions, 0 deletions
diff --git a/libglusterfs/src/options.c b/libglusterfs/src/options.c index f63c60476da..9edb7c03df2 100644 --- a/libglusterfs/src/options.c +++ b/libglusterfs/src/options.c @@ -58,6 +58,7 @@ xlator_option_validate_int (xlator_t *xl, const char *key, const char *value,                              volume_option_t *opt, char **op_errstr)  {          long long inputll = 0; +        unsigned long long uinputll = 0;          int       ret = -1;          char      errstr[256]; @@ -70,6 +71,15 @@ xlator_option_validate_int (xlator_t *xl, const char *key, const char *value,                  goto out;          } +        /* Handle '-0' */ +        if ((inputll == 0) && (gf_string2ulonglong (value, &uinputll) != 0)) { +                snprintf (errstr, 256, +                          "invalid number format \"%s\" in option \"%s\"", +                          value, key); +                gf_log (xl->name, GF_LOG_ERROR, "%s", errstr); +                goto out; +        } +          if ((opt->min == 0) && (opt->max == 0) &&              (opt->validate == GF_OPT_VALIDATE_BOTH)) {                  gf_log (xl->name, GF_LOG_TRACE, diff --git a/tests/bugs/bug-1111557.t b/tests/bugs/bug-1111557.t new file mode 100644 index 00000000000..656b6e6519b --- /dev/null +++ b/tests/bugs/bug-1111557.t @@ -0,0 +1,12 @@ +#!/bin/bash + +. $(dirname $0)/../include.rc + +cleanup; + +TEST glusterd +TEST pidof glusterd +TEST $CLI volume create $V0 $H0:$B0/${V0}{0} +TEST $CLI volume set $V0 diagnostics.brick-log-buf-size 0 +TEST ! $CLI volume set $V0 diagnostics.brick-log-buf-size -0 +cleanup diff --git a/xlators/debug/io-stats/src/io-stats.c b/xlators/debug/io-stats/src/io-stats.c index 2a5eedefbd3..6a2aa586ab1 100644 --- a/xlators/debug/io-stats/src/io-stats.c +++ b/xlators/debug/io-stats/src/io-stats.c @@ -2838,6 +2838,9 @@ init (xlator_t *this)          this->private = conf;          ret = 0;  out: +        if (!this->private) +                ret = -1; +          return ret;  }  | 
