summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorXavier Hernandez <jahernan@redhat.com>2017-11-02 09:01:48 +0100
committerAmar Tumballi <amarts@redhat.com>2017-11-07 13:58:01 +0000
commite3d5587a267106fc7fa728f29a12dc199a539202 (patch)
tree03c8be821818c9c90eefca23b047c1dafa5a2bdb
parentc3d7974e2be68f0fac8f54c9557d0f868e6be6c8 (diff)
cli: Fix several coverity issues in cli-cmd-parser.c
This patch fixes coverity issues 115, 191, 274, 462, 463, 508, 588, 612, 614, 618 and 698 from [1]. Also removed the need for some relatively big arrays as local variables of some functions. [1] https://download.gluster.org/pub/gluster/glusterfs/static-analysis/master/glusterfs-coverity/2017-10-30-9aa574a5/html/ Change-Id: I4f435dbdbedc1b6de067084cce3050b620b28391 BUG: 789278 Signed-off-by: Xavier Hernandez <jahernan@redhat.com>
-rw-r--r--cli/src/cli-cmd-parser.c272
1 files changed, 173 insertions, 99 deletions
diff --git a/cli/src/cli-cmd-parser.c b/cli/src/cli-cmd-parser.c
index 9e112954811..950067550be 100644
--- a/cli/src/cli-cmd-parser.c
+++ b/cli/src/cli-cmd-parser.c
@@ -19,6 +19,7 @@
#include "cli-cmd.h"
#include "cli-mem-types.h"
#include "dict.h"
+#include "list.h"
#include "protocol-common.h"
#include "cli1-xdr.h"
@@ -60,6 +61,12 @@ enum cli_snap_config_set_types {
};
typedef enum cli_snap_config_set_types cli_snap_config_set_types;
+typedef struct _cli_brick {
+ struct list_head list;
+ const char *name;
+ int32_t len;
+} cli_brick_t;
+
int
cli_cmd_validate_volume (char *volname);
@@ -79,17 +86,15 @@ int32_t
cli_cmd_bricks_parse (const char **words, int wordcount, int brick_index,
char **bricks, int *brick_count)
{
- int ret = 0;
- char *tmp_list = NULL;
- char brick_list[120000] = {0,};
- char *space = " ";
- char *delimiter = NULL;
- char *host_name = NULL;
- char *free_list_ptr = NULL;
- char *tmpptr = NULL;
- int j = 0;
- int brick_list_len = 0;
- char *tmp_host = NULL;
+ int ret = 0;
+ char *delimiter = NULL;
+ char *host_name = NULL;
+ char *tmp_host = NULL;
+ char *bricks_str = NULL;
+ int len = 0;
+ int brick_list_len = 1; /* For initial space */
+ struct list_head brick_list = { 0, };
+ cli_brick_t *brick = NULL;
GF_ASSERT (words);
GF_ASSERT (wordcount);
@@ -97,8 +102,8 @@ cli_cmd_bricks_parse (const char **words, int wordcount, int brick_index,
GF_ASSERT (brick_index > 0);
GF_ASSERT (brick_index < wordcount);
- strncpy (brick_list, space, strlen (space));
- brick_list_len++;
+ INIT_LIST_HEAD(&brick_list);
+
while (brick_index < wordcount) {
if (validate_brick_name ((char *)words[brick_index])) {
cli_err ("Wrong brick type: %s, use <HOSTNAME>:"
@@ -112,13 +117,6 @@ cli_cmd_bricks_parse (const char **words, int wordcount, int brick_index,
goto out;
}
- if ((brick_list_len + strlen (words[brick_index]) + 1) > sizeof (brick_list)) {
- cli_err ("Total brick list is larger than a request. "
- "Can take (brick_count %d)", *brick_count);
- ret = -1;
- goto out;
- }
-
tmp_host = gf_strdup ((char *)words[brick_index]);
if (!tmp_host) {
gf_log ("cli", GF_LOG_ERROR, "Out of memory");
@@ -130,6 +128,7 @@ cli_cmd_bricks_parse (const char **words, int wordcount, int brick_index,
ret = -1;
gf_log("cli",GF_LOG_ERROR, "Unable to allocate "
"memory");
+ GF_FREE (tmp_host);
goto out;
}
@@ -148,36 +147,59 @@ cli_cmd_bricks_parse (const char **words, int wordcount, int brick_index,
"standards", host_name);
}
GF_FREE (tmp_host);
- tmp_list = gf_strdup (brick_list + 1);
- if (free_list_ptr) {
- GF_FREE (free_list_ptr);
- free_list_ptr = NULL;
- }
- free_list_ptr = tmp_list;
- j = 0;
- while(j < *brick_count) {
- strtok_r (tmp_list, " ", &tmpptr);
- if (!(strcmp (tmp_list, words[brick_index]))) {
+ list_for_each_entry(brick, &brick_list, list) {
+ if (strcmp(brick->name, words[brick_index]) == 0) {
ret = -1;
- cli_err ("Found duplicate"
- " exports %s",words[brick_index]);
+ cli_err("Found duplicate exports %s",
+ words[brick_index]);
goto out;
- }
- tmp_list = tmpptr;
- j++;
+ }
}
- strcat (brick_list, words[brick_index]);
- strcat (brick_list, " ");
- brick_list_len += (strlen (words[brick_index]) + 1);
+
+ brick = GF_MALLOC(sizeof(cli_brick_t), gf_common_list_node);
+ if (brick == NULL) {
+ ret = -1;
+ gf_log("cli", GF_LOG_ERROR, "Out of memory");
+ goto out;
+ }
+ len = strlen(words[brick_index]);
+ brick->name = words[brick_index];
+ brick->len = len;
+ list_add_tail(&brick->list, &brick_list);
+
+ brick_list_len += len + 1; /* Brick name + space */
++(*brick_count);
++brick_index;
}
- *bricks = gf_strdup (brick_list);
- if (!*bricks)
+ brick_list_len++; /* For terminating null char */
+
+ bricks_str = GF_MALLOC(brick_list_len, gf_common_mt_char);
+ if (bricks_str == NULL) {
ret = -1;
+ gf_log("cli", GF_LOG_ERROR, "Out of memory");
+ goto out;
+ }
+ *bricks = bricks_str;
+ *bricks_str = ' ';
+ bricks_str++;
+ while (!list_empty(&brick_list)) {
+ brick = list_first_entry(&brick_list, cli_brick_t, list);
+ list_del_init(&brick->list);
+ memcpy(bricks_str, brick->name, brick->len);
+ bricks_str[brick->len] = ' ';
+ bricks_str += brick->len + 1;
+ GF_FREE(brick);
+ }
+ *bricks_str = 0;
+
out:
- GF_FREE (free_list_ptr);
+ while (!list_empty(&brick_list)) {
+ brick = list_first_entry(&brick_list, cli_brick_t, list);
+ list_del_init(&brick->list);
+ GF_FREE(brick);
+ }
+
return ret;
}
@@ -933,7 +955,7 @@ cli_cmd_get_state_parse (struct cli_state *state,
}
}
- ret = dict_set_str (dict, "daemon", daemon_name);
+ ret = dict_set_dynstr (dict, "daemon", daemon_name);
if (ret) {
*op_errstr = gf_strdup ("Command failed. Please check "
" log file for more details.");
@@ -941,6 +963,7 @@ cli_cmd_get_state_parse (struct cli_state *state,
"Setting daemon name to dictionary failed");
goto out;
}
+ daemon_name = NULL;
if (odir) {
ret = dict_set_str (dict, "odir", odir);
@@ -988,6 +1011,8 @@ cli_cmd_get_state_parse (struct cli_state *state,
if (ret && dict)
dict_unref (dict);
+ GF_FREE(daemon_name);
+
return ret;
}
@@ -1271,11 +1296,6 @@ cli_cmd_quota_parse (const char **words, int wordcount, dict_t **options)
}
if (strcmp (w, "list-objects") == 0) {
- if (wordcount < 4) {
- ret = -1;
- goto out;
- }
-
type = GF_QUOTA_OPTION_TYPE_LIST_OBJECTS;
i = 4;
@@ -1395,22 +1415,49 @@ cli_is_key_spl (char *key)
return (strcmp (key, "group") == 0);
}
+static int32_t
+cli_add_key_group_value(dict_t *dict, const char *name, const char *value,
+ int32_t id, char **op_errstr)
+{
+ char *key = NULL;
+ char *data = NULL;
+ int32_t ret = -1;
+
+ ret = gf_asprintf(&key, "%s%d", name, id);
+ if (ret < 0) {
+ goto out;
+ }
+ data = gf_strdup(value);
+ if (data == NULL) {
+ goto out;
+ }
+
+ ret = dict_set_dynstr(dict, key, data);
+ if (ret == 0) {
+ data = NULL;
+ }
+
+out:
+ GF_FREE(key);
+ GF_FREE(data);
+
+ if ((ret != 0) && (op_errstr != NULL)) {
+ *op_errstr = gf_strdup("Failed to allocate memory");
+ }
+
+ return ret;
+}
+
static int
cli_add_key_group (dict_t *dict, char *key, char *value, char **op_errstr)
{
int ret = -1;
int opt_count = 0;
- char iter_key[1024] = {0,};
- char iter_val[1024] = {0,};
char *saveptr = NULL;
char *tok_key = NULL;
char *tok_val = NULL;
- char *dkey = NULL;
- char *dval = NULL;
char *tagpath = NULL;
- char *buf = NULL;
char line[PATH_MAX + 256] = {0,};
- char errstr[2048] = "";
FILE *fp = NULL;
ret = gf_asprintf (&tagpath, "%s/groups/%s",
@@ -1423,51 +1470,54 @@ cli_add_key_group (dict_t *dict, char *key, char *value, char **op_errstr)
fp = fopen (tagpath, "r");
if (!fp) {
ret = -1;
- snprintf(errstr, sizeof(errstr), "Unable to open file '%s'."
- " Error: %s", tagpath, strerror (errno));
- if (op_errstr)
- *op_errstr = gf_strdup(errstr);
+ if (op_errstr) {
+ gf_asprintf(op_errstr, "Unable to open file '%s'. "
+ "Error: %s",
+ tagpath, strerror(errno));
+ }
goto out;
}
opt_count = 0;
- buf = line;
- while (fscanf (fp, "%s", buf) != EOF) {
-
+ while (fgets(line, sizeof(line), fp) != NULL) {
+ if (strlen(line) >= sizeof(line) - 1) {
+ ret = -1;
+ if (op_errstr != NULL) {
+ *op_errstr = gf_strdup("Line too long");
+ }
+ goto out;
+ }
opt_count++;
tok_key = strtok_r (line, "=", &saveptr);
- tok_val = strtok_r (NULL, "=", &saveptr);
+ tok_val = strtok_r (NULL, "\r\n", &saveptr);
if (!tok_key || !tok_val) {
ret = -1;
- snprintf(errstr, sizeof(errstr), "'%s' file format "
- "not valid.", tagpath);
- if (op_errstr)
- *op_errstr = gf_strdup(errstr);
+ if (op_errstr) {
+ gf_asprintf(op_errstr, "'%s' file format "
+ "not valid.",
+ tagpath);
+ }
goto out;
}
- snprintf (iter_key, sizeof (iter_key), "key%d", opt_count);
- dkey = gf_strdup (tok_key);
- ret = dict_set_dynstr (dict, iter_key, dkey);
- if (ret)
+ ret = cli_add_key_group_value(dict, "key", tok_key, opt_count,
+ op_errstr);
+ if (ret != 0) {
goto out;
- dkey = NULL;
-
- snprintf (iter_val, sizeof (iter_val), "value%d", opt_count);
- dval = gf_strdup (tok_val);
- ret = dict_set_dynstr (dict, iter_val, dval);
- if (ret)
+ }
+ ret = cli_add_key_group_value(dict, "value", tok_val,
+ opt_count, op_errstr);
+ if (ret != 0) {
goto out;
- dval = NULL;
-
+ }
}
if (!opt_count) {
ret = -1;
- snprintf(errstr, sizeof(errstr), "'%s' file format "
- "not valid.", tagpath);
- if (op_errstr)
- *op_errstr = gf_strdup(errstr);
+ if (op_errstr) {
+ gf_asprintf(op_errstr, "'%s' file format not valid.",
+ tagpath);
+ }
goto out;
}
ret = dict_set_int32 (dict, "count", opt_count);
@@ -1475,11 +1525,6 @@ out:
GF_FREE (tagpath);
- if (ret) {
- GF_FREE (dkey);
- GF_FREE (dval);
- }
-
if (fp)
fclose (fp);
@@ -2637,6 +2682,10 @@ config_parse (const char **words, int wordcount, dict_t *dict,
}
ret = dict_set_dynstr (dict, "op_value", append_str);
+ if (ret != 0) {
+ goto out;
+ }
+ append_str = NULL;
}
ret = -1;
@@ -2647,9 +2696,7 @@ config_parse (const char **words, int wordcount, dict_t *dict,
}
out:
- if (ret && append_str)
- GF_FREE (append_str);
-
+ GF_FREE (append_str);
GF_FREE (subop);
gf_log ("cli", GF_LOG_DEBUG, "Returning %d", ret);
@@ -2942,6 +2989,10 @@ cli_cmd_gsync_set_parse (const char **words, int wordcount, dict_t **options)
* pass down only <slavehost> else pass as it is.
*/
slave_temp = gf_strdup (words[slavei]);
+ if (slave_temp == NULL) {
+ ret = -1;
+ goto out;
+ }
token = strtok_r (slave_temp, "@", &save_ptr);
if (token && !strcmp (token, "root")) {
ret = dict_set_str (dict, "slave",
@@ -3507,7 +3558,8 @@ cli_cmd_volume_statedump_options_parse (const char **words, int wordcount,
dict_t *dict = NULL;
int option_cnt = 0;
char *option = NULL;
- char option_str[_POSIX_HOST_NAME_MAX + 100] = {0,};
+ char *option_str = NULL;
+ char *tmp_str = NULL;
char *tmp = NULL;
char *ip_addr = NULL;
char *pid = NULL;
@@ -3522,11 +3574,11 @@ cli_cmd_volume_statedump_options_parse (const char **words, int wordcount,
pid = strtok(NULL, ":");
if (valid_internet_address (ip_addr, _gf_true)
&& pid && gf_valid_pid (pid, strlen(pid))) {
- strncat (option_str, words[3], strlen (words[3]));
- strncat (option_str, " ", 1);
- strncat (option_str, ip_addr, strlen (ip_addr));
- strncat (option_str, " ", 1);
- strncat (option_str, pid, strlen (pid));
+ ret = gf_asprintf(&option_str, "%s %s %s", words[3],
+ ip_addr, pid);
+ if (ret < 0) {
+ goto out;
+ }
option_cnt = 3;
} else {
ret = -1;
@@ -3538,8 +3590,14 @@ cli_cmd_volume_statedump_options_parse (const char **words, int wordcount,
ret = -1;
goto out;
}
- strncat (option_str, option, strlen (option));
- strncat (option_str, " ", 1);
+ tmp_str = option_str;
+ option_str = NULL;
+ ret = gf_asprintf(&option_str, "%s%s ",
+ tmp_str ? tmp_str : "", option);
+ GF_FREE(tmp_str);
+ if (ret < 0) {
+ goto out;
+ }
}
if ((strstr (option_str, "nfs")) && strstr (option_str, "quotad")) {
ret = -1;
@@ -3548,12 +3606,15 @@ cli_cmd_volume_statedump_options_parse (const char **words, int wordcount,
}
dict = dict_new ();
- if (!dict)
+ if (!dict) {
+ ret = -1;
goto out;
+ }
- ret = dict_set_dynstr (dict, "options", gf_strdup (option_str));
+ ret = dict_set_dynstr (dict, "options", option_str);
if (ret)
goto out;
+ option_str = NULL;
ret = dict_set_int32 (dict, "option_cnt", option_cnt);
if (ret)
@@ -3562,6 +3623,7 @@ cli_cmd_volume_statedump_options_parse (const char **words, int wordcount,
*options = dict;
out:
GF_FREE (tmp);
+ GF_FREE (option_str);
if (ret && dict)
dict_unref (dict);
if (ret)
@@ -3738,16 +3800,26 @@ set_hostname_path_in_dict (const char *token, dict_t *dict, int heal_op)
hostname);
if (ret)
goto out;
+ hostname = NULL;
ret = dict_set_dynstr (dict, "heal-source-brickpath",
path);
+ if (ret) {
+ goto out;
+ }
+ path = NULL;
break;
case GF_SHD_OP_STATISTICS_HEAL_COUNT_PER_REPLICA:
ret = dict_set_dynstr (dict, "per-replica-cmd-hostname",
hostname);
if (ret)
goto out;
+ hostname = NULL;
ret = dict_set_dynstr (dict, "per-replica-cmd-path",
path);
+ if (ret) {
+ goto out;
+ }
+ path = NULL;
break;
default:
ret = -1;
@@ -3755,6 +3827,8 @@ set_hostname_path_in_dict (const char *token, dict_t *dict, int heal_op)
}
out:
+ GF_FREE(hostname);
+ GF_FREE(path);
return ret;
}