core: fix strncpy warnings

Since gcc-8.2.x (fedora-28 or so) gcc has been emitting warnings
about buggy use of strncpy.

Most uses that gcc warns about in our sources are exactly backwards;
the 'limit' or len is the strlen/size of the _source param_, giving
exactly zero protection against overruns. (Which was, after all, one
of the points of using strncpy in the first place.)

IOW, many warnings are about uses that look approximately like this:
    ...
    char dest[8];
    char src[] = "this is a string longer than eight chars";
    ...
    strncpy (dest, src, sizeof(src)); /* boom */
    ...

The len/limit should be sizeof(dest).

Note: the above example has a definite over-run. In our source the
overrun is typically only theoretical (but possibly exploitable.)

Also strncpy doesn't null-terminate on truncation; snprintf does; prefer
snprintf over strncpy.

Mildly surprising that coverity doesn't warn/isn't warning about this.

Change-Id: I022d5c6346a751e181ad44d9a099531c1172626e
updates: bz#1193929
Signed-off-by: Kaleb S. KEITHLE <kkeithle@redhat.com>

6 files changed, 65 insertions, 42 deletions

diff --git a/xlators/mgmt/glusterd/src/glusterd-locks.c b/xlators/mgmt/glusterd/src/glusterd-locks.c
index b21da601d8c..ad87c1df64f 100644
--- a/xlators/mgmt/glusterd/src/glusterd-locks.c
+++ b/xlators/mgmt/glusterd/src/glusterd-locks.c
@@ -791,7 +791,7 @@ glusterd_mgmt_v3_unlock(const char *name, uuid_t uuid, char *type)
         ret = -1;
         goto out;
     }
-    strncpy(key_dup, key, strlen(key));
+    (void)snprintf(key_dup, sizeof(key_dup), "%s", key);
 
     gf_msg_debug(this->name, 0, "Trying to release lock of %s %s for %s as %s",
                  type, name, uuid_utoa(uuid), key);
diff --git a/xlators/mgmt/glusterd/src/glusterd-replace-brick.c b/xlators/mgmt/glusterd/src/glusterd-replace-brick.c
index dcac3a8e52d..f14e79ecf5f 100644
--- a/xlators/mgmt/glusterd/src/glusterd-replace-brick.c
+++ b/xlators/mgmt/glusterd/src/glusterd-replace-brick.c
@@ -389,8 +389,8 @@ glusterd_op_perform_replace_brick(glusterd_volinfo_t *volinfo, char *old_brick,
     if (ret)
         goto out;
 
-    strncpy(new_brickinfo->brick_id, old_brickinfo->brick_id,
-            sizeof(new_brickinfo->brick_id));
+    (void)snprintf(new_brickinfo->brick_id, sizeof(new_brickinfo->brick_id),
+                   "%s", old_brickinfo->brick_id);
     new_brickinfo->port = old_brickinfo->port;
 
     /* A bricks mount dir is required only by snapshots which were
@@ -405,8 +405,8 @@ glusterd_op_perform_replace_brick(glusterd_volinfo_t *volinfo, char *old_brick,
                    "brick1.mount_dir not present");
             goto out;
         }
-        strncpy(new_brickinfo->mount_dir, brick_mount_dir,
-                sizeof(new_brickinfo->mount_dir));
+        (void)snprintf(new_brickinfo->mount_dir,
+                       sizeof(new_brickinfo->mount_dir), "%s", brick_mount_dir);
     }
 
     cds_list_add(&new_brickinfo->brick_list, &old_brickinfo->brick_list);
diff --git a/xlators/mgmt/glusterd/src/glusterd-store.c b/xlators/mgmt/glusterd/src/glusterd-store.c
index f0ee571c993..ff02d68d722 100644
--- a/xlators/mgmt/glusterd/src/glusterd-store.c
+++ b/xlators/mgmt/glusterd/src/glusterd-store.c
@@ -2728,7 +2728,12 @@ glusterd_store_retrieve_bricks(glusterd_volinfo_t *volinfo)
                     ret = -1;
                     goto out;
                 }
-                strncpy(brickinfo->real_path, abspath, strlen(abspath));
+                if (strlen(abspath) >= sizeof(brickinfo->real_path)) {
+                    ret = -1;
+                    goto out;
+                }
+                (void)strncpy(brickinfo->real_path, abspath,
+                              sizeof(brickinfo->real_path));
             }
         }
 
@@ -3667,7 +3672,7 @@ glusterd_recreate_vol_brick_mounts(xlator_t *this, glusterd_volinfo_t *volinfo)
     struct stat st_buf = {
         0,
     };
-    char abspath[VALID_GLUSTERD_PATHMAX] = {0};
+    char abspath[PATH_MAX] = {0};
 
     GF_ASSERT(this);
     GF_ASSERT(volinfo);
@@ -3730,7 +3735,12 @@ glusterd_recreate_vol_brick_mounts(xlator_t *this, glusterd_volinfo_t *volinfo)
                     ret = -1;
                     goto out;
                 }
-                strncpy(brickinfo->real_path, abspath, strlen(abspath));
+                if (strlen(abspath) >= sizeof(brickinfo->real_path)) {
+                    ret = -1;
+                    goto out;
+                }
+                (void)strncpy(brickinfo->real_path, abspath,
+                              sizeof(brickinfo->real_path));
             }
         }
 
diff --git a/xlators/mgmt/glusterd/src/glusterd-utils.c b/xlators/mgmt/glusterd/src/glusterd-utils.c
index e633d6ebc4d..d9e0d6110a0 100644
--- a/xlators/mgmt/glusterd/src/glusterd-utils.c
+++ b/xlators/mgmt/glusterd/src/glusterd-utils.c
@@ -1294,7 +1294,12 @@ glusterd_brickinfo_new_from_brick(char *brick, glusterd_brickinfo_t **brickinfo,
                 goto out;
             }
         }
-        strncpy(new_brickinfo->real_path, abspath, strlen(abspath));
+        if (strlen(abspath) >= sizeof(new_brickinfo->real_path)) {
+            ret = -1;
+            goto out;
+        }
+        (void)strncpy(new_brickinfo->real_path, abspath,
+                      sizeof(new_brickinfo->real_path));
     }
 
     *brickinfo = new_brickinfo;
@@ -1366,7 +1371,7 @@ glusterd_is_brickpath_available(uuid_t uuid, char *path)
     glusterd_volinfo_t *volinfo = NULL;
     glusterd_conf_t *priv = NULL;
     gf_boolean_t available = _gf_false;
-    char tmp_path[PATH_MAX + 1] = "";
+    char tmp_path[PATH_MAX] = "";
 
     priv = THIS->private;
 
@@ -1385,7 +1390,7 @@ glusterd_is_brickpath_available(uuid_t uuid, char *path)
             goto out;
         }
         /* When realpath(3) fails, tmp_path is undefined. */
-        strncpy(tmp_path, path, PATH_MAX);
+        (void)snprintf(tmp_path, sizeof(tmp_path), "%s", path);
     }
 
     cds_list_for_each_entry(volinfo, &priv->volumes, vol_list)
@@ -3762,8 +3767,8 @@ glusterd_import_new_brick(dict_t *peer_data, int32_t vol_count,
     }
     new_brickinfo->decommissioned = decommissioned;
     if (brick_id)
-        gf_strncpy(new_brickinfo->brick_id, brick_id,
-                   sizeof(new_brickinfo->brick_id));
+        (void)snprintf(new_brickinfo->brick_id, sizeof(new_brickinfo->brick_id),
+                       "%s", brick_id);
 
     snprintf(key, sizeof(key), "%s%d.brick%d", prefix, vol_count, brick_count);
     ret = gd_import_new_brick_snap_details(peer_data, key, new_brickinfo);
@@ -4441,10 +4446,16 @@ glusterd_volinfo_copy_brickinfo(glusterd_volinfo_t *old_volinfo,
                     ret = -1;
                     goto out;
                 }
-                strncpy(new_brickinfo->real_path, abspath, strlen(abspath));
+                if (strlen(abspath) >= sizeof(new_brickinfo->real_path)) {
+                    ret = -1;
+                    goto out;
+                }
+                (void)strncpy(new_brickinfo->real_path, abspath,
+                              sizeof(new_brickinfo->real_path));
             } else {
-                strncpy(new_brickinfo->real_path, old_brickinfo->real_path,
-                        strlen(old_brickinfo->real_path));
+                (void)strncpy(new_brickinfo->real_path,
+                              old_brickinfo->real_path,
+                              sizeof(new_brickinfo->real_path));
             }
         }
     }
@@ -5283,7 +5294,7 @@ glusterd_remote_hostname_get(rpcsvc_request_t *req, char *remote_host, int len)
         tmp_host = hostname = canon;
     }
 
-    strncpy(remote_host, hostname, strlen(hostname));
+    (void)snprintf(remote_host, len, "%s", hostname);
 
 out:
     GF_FREE(tmp_host);
@@ -6472,7 +6483,6 @@ _local_gsyncd_start(dict_t *this, char *key, data_t *value, void *data)
     char *slave_host = NULL;
     char *statefile = NULL;
     char buf[1024] = "faulty";
-    int uuid_len = 0;
     int ret = 0;
     int op_ret = 0;
     int ret_status = 0;
@@ -6498,9 +6508,8 @@ _local_gsyncd_start(dict_t *this, char *key, data_t *value, void *data)
         slave++;
     else
         return 0;
-    uuid_len = (slave - value->data - 1);
 
-    strncpy(uuid_str, (char *)value->data, uuid_len);
+    (void)snprintf(uuid_str, sizeof(uuid_str), "%s", (char *)value->data);
 
     /* Getting Local Brickpaths */
     ret = glusterd_get_local_brickpaths(volinfo, &path_list);
@@ -12883,6 +12892,9 @@ glusterd_update_mntopts(char *brick_path, glusterd_brickinfo_t *brickinfo)
         ret = -1;
         goto out;
     }
+    (void)snprintf(brickinfo->mnt_opts, sizeof(brickinfo->mnt_opts), "%s",
+                   entry->mnt_opts);
+
     gf_strncpy(brickinfo->mnt_opts, entry->mnt_opts,
                sizeof(brickinfo->mnt_opts));
 
diff --git a/xlators/mgmt/glusterd/src/glusterd-volgen.c b/xlators/mgmt/glusterd/src/glusterd-volgen.c
index 9a6623c2bd0..49542328916 100644
--- a/xlators/mgmt/glusterd/src/glusterd-volgen.c
+++ b/xlators/mgmt/glusterd/src/glusterd-volgen.c
@@ -5407,7 +5407,7 @@ glusterd_is_valid_volfpath(char *volname, char *brick)
         ret = 0;
         goto out;
     }
-    strncpy(volinfo->volname, volname, strlen(volname));
+    (void)snprintf(volinfo->volname, sizeof(volinfo->volname), "%s", volname);
     get_brick_filepath(volfpath, volinfo, brickinfo, NULL);
 
     ret = ((strlen(volfpath) < PATH_MAX) &&
@@ -6149,7 +6149,7 @@ build_bitd_volume_graph(volgen_graph_t *graph, glusterd_volinfo_t *volinfo,
 
     get_transport_type(volinfo, set_dict, transt, _gf_false);
     if (!strncmp(transt, "tcp,rdma", SLEN("tcp,rdma")))
-        strncpy(transt, "tcp", sizeof(transt));
+        (void)snprintf(transt, sizeof(transt), "%s", "tcp");
 
     cds_list_for_each_entry(brickinfo, &volinfo->bricks, brick_list)
     {
@@ -6308,7 +6308,7 @@ build_scrub_volume_graph(volgen_graph_t *graph, glusterd_volinfo_t *volinfo,
 
     get_transport_type(volinfo, set_dict, transt, _gf_false);
     if (!strncmp(transt, "tcp,rdma", SLEN("tcp,rdma")))
-        strncpy(transt, "tcp", sizeof(transt));
+        (void)snprintf(transt, sizeof(transt), "%s", "tcp");
 
     cds_list_for_each_entry(brickinfo, &volinfo->bricks, brick_list)
     {
diff --git a/xlators/mgmt/glusterd/src/glusterd.c b/xlators/mgmt/glusterd/src/glusterd.c
index 3da579464bb..27529f9fa59 100644
--- a/xlators/mgmt/glusterd/src/glusterd.c
+++ b/xlators/mgmt/glusterd/src/glusterd.c
@@ -1106,19 +1106,14 @@ glusterd_init_uds_listener(xlator_t *this)
     dict_t *options = NULL;
     rpcsvc_t *rpc = NULL;
     data_t *sock_data = NULL;
-    char sockfile[UNIX_PATH_MAX + 1] = {
-        0,
-    };
+    char sockfile[UNIX_PATH_MAX] = {0};
     int i = 0;
 
     GF_ASSERT(this);
 
     sock_data = dict_get(this->options, "glusterd-sockfile");
-    if (!sock_data) {
-        strncpy(sockfile, DEFAULT_GLUSTERD_SOCKFILE, UNIX_PATH_MAX);
-    } else {
-        strncpy(sockfile, sock_data->data, UNIX_PATH_MAX);
-    }
+    (void)snprintf(sockfile, sizeof(sockfile), "%s",
+                   sock_data ? sock_data->data : DEFAULT_GLUSTERD_SOCKFILE);
 
     options = dict_new();
     if (!options)
@@ -1182,9 +1177,7 @@ glusterd_stop_uds_listener(xlator_t *this)
     rpcsvc_listener_t *listener = NULL;
     rpcsvc_listener_t *next = NULL;
     data_t *sock_data = NULL;
-    char sockfile[UNIX_PATH_MAX + 1] = {
-        0,
-    };
+    char sockfile[UNIX_PATH_MAX] = {0};
 
     GF_ASSERT(this);
     conf = this->private;
@@ -1200,11 +1193,8 @@ glusterd_stop_uds_listener(xlator_t *this)
     (void)rpcsvc_unregister_notify(conf->uds_rpc, glusterd_rpcsvc_notify, this);
 
     sock_data = dict_get(this->options, "glusterd-sockfile");
-    if (!sock_data) {
-        strncpy(sockfile, DEFAULT_GLUSTERD_SOCKFILE, UNIX_PATH_MAX);
-    } else {
-        strncpy(sockfile, sock_data->data, UNIX_PATH_MAX);
-    }
+    (void)snprintf(sockfile, sizeof(sockfile), "%s",
+                   sock_data ? sock_data->data : DEFAULT_GLUSTERD_SOCKFILE);
     sys_unlink(sockfile);
 
     return;
@@ -1833,8 +1823,20 @@ init(xlator_t *this)
     conf->rpc = rpc;
     conf->uds_rpc = uds_rpc;
     conf->gfs_mgmt = &gd_brick_prog;
-    (void)strncpy(conf->workdir, workdir, strlen(workdir) + 1);
-    (void)strncpy(conf->rundir, rundir, strlen(rundir) + 1);
+    this->private = conf;
+    /* conf->workdir and conf->rundir are smaller than PATH_MAX; gcc's
+     * snprintf checking will throw an error here if sprintf is used. */
+    if (strlen(workdir) >= sizeof(conf->workdir)) {
+        ret = -1;
+        goto out;
+    }
+    (void)strncpy(conf->workdir, workdir, sizeof(conf->workdir));
+    /* separate tests because combined tests confuses gcc */
+    if (strlen(rundir) >= sizeof(conf->rundir)) {
+        ret = -1;
+        goto out;
+    }
+    (void)strncpy(conf->rundir, rundir, sizeof(conf->rundir));
 
     synclock_init(&conf->big_lock, SYNC_LOCK_RECURSIVE);
     pthread_mutex_init(&conf->xprt_lock, NULL);
@@ -1887,7 +1889,6 @@ init(xlator_t *this)
     ret = dict_get_int32(this->options, "ping-timeout", &conf->ping_timeout);
     /* Not failing here since ping-timeout can be optional as well */
 
-    this->private = conf;
     glusterd_mgmt_v3_lock_init();
     glusterd_mgmt_v3_lock_timer_init();
     glusterd_txn_opinfo_dict_init();