diff options
author | Kaleb S. KEITHLEY <kkeithle@redhat.com> | 2014-06-13 11:09:25 -0400 |
---|---|---|
committer | Vijay Bellur <vbellur@redhat.com> | 2014-11-25 09:27:36 -0800 |
commit | 4ea5b8d2046b9e0bc7f24cdf1b2e72ab8b462c9e (patch) | |
tree | 2bd4b9314e30d57b93f0d4302118df65fa277b63 /xlators | |
parent | c6f27ed5cc9a6fafc6e6f83aff00196bc7a49d38 (diff) |
core: fix Ubuntu code audit (cppcheck) results
See also http://review.gluster.org/#/c/7693/, BZ 1091677
AFAICT these are false positives:
[geo-replication/src/gsyncd.c:100]: (error) Memory leak: str
[geo-replication/src/gsyncd.c:403]: (error) Memory leak: argv
[xlators/nfs/server/src/nlm4.c:1201]: (error) Possible null pointer dereference: fde
[xlators/cluster/afr/src/afr-self-heal-common.c:138]: (error) Possible null pointer dereference: __ptr
[xlators/cluster/afr/src/afr-self-heal-common.c:140]: (error) Possible null pointer dereference: __ptr
[xlators/cluster/afr/src/afr-self-heal-common.c:331]: (error) Possible null pointer dereference: __ptr
Test program:
[extras/test/test-ffop.c:27]: (error) Buffer overrun possible for long command line arguments.
[tests/basic/fops-sanity.c:55]: (error) Buffer overrun possible for long command line arguments.
the remainder are fixed with this change-set:
[cli/src/cli-rpc-ops.c:8883]: (error) Possible null pointer dereference: local
[cli/src/cli-rpc-ops.c:8886]: (error) Possible null pointer dereference: local
[contrib/uuid/gen_uuid.c:369]: (warning) %ld in format string (no. 2) requires 'long *' but the argument type is 'unsigned long *'.
[contrib/uuid/gen_uuid.c:369]: (warning) %ld in format string (no. 3) requires 'long *' but the argument type is 'unsigned long *'.
[xlators/cluster/dht/src/dht-rebalance.c:1734]: (error) Possible null pointer dereference: ctx
[xlators/cluster/stripe/src/stripe.c:4940]: (error) Possible null pointer dereference: local
[xlators/mgmt/glusterd/src/glusterd-geo-rep.c:1718]: (error) Possible null pointer dereference: command
[xlators/mgmt/glusterd/src/glusterd-replace-brick.c:942]: (error) Resource leak: file
[xlators/mgmt/glusterd/src/glusterd-replace-brick.c:1026]: (error) Resource leak: file
[xlators/mgmt/glusterd/src/glusterd-sm.c:249]: (error) Possible null pointer dereference: new_ev_ctx
[xlators/mgmt/glusterd/src/glusterd-snapshot.c:6917]: (error) Possible null pointer dereference: volinfo
[xlators/mgmt/glusterd/src/glusterd-utils.c:4517]: (error) Possible null pointer dereference: this
[xlators/mgmt/glusterd/src/glusterd-utils.c:6662]: (error) Possible null pointer dereference: this
[xlators/mgmt/glusterd/src/glusterd-utils.c:7708]: (error) Possible null pointer dereference: this
[xlators/mount/fuse/src/fuse-bridge.c:4687]: (error) Uninitialized variable: finh
[xlators/mount/fuse/src/fuse-bridge.c:3080]: (error) Possible null pointer dereference: state
[xlators/nfs/server/src/nfs-common.c:89]: (error) Dangerous usage of 'volname' (strncpy doesn't always null-terminate it).
[xlators/performance/quick-read/src/quick-read.c:586]: (error) Possible null pointer dereference: iobuf
Rerunning cppcheck after fixing the above:
As before, test program:
[extras/test/test-ffop.c:27]: (error) Buffer overrun possible for long command line arguments.
[tests/basic/fops-sanity.c:55]: (error) Buffer overrun possible for long command line arguments.
As before, false positive:
[geo-replication/src/gsyncd.c:100]: (error) Memory leak: str
[geo-replication/src/gsyncd.c:403]: (error) Memory leak: argv
[xlators/nfs/server/src/nlm4.c:1201]: (error) Possible null pointer dereference: fde
[xlators/cluster/afr/src/afr-self-heal-common.c:138]: (error) Possible null pointer dereference: __ptr
[xlators/cluster/afr/src/afr-self-heal-common.c:140]: (error) Possible null pointer dereference: __ptr
[xlators/cluster/afr/src/afr-self-heal-common.c:331]: (error) Possible null pointer dereference: __ptr
False positive after fix:
[xlators/performance/quick-read/src/quick-read.c:584]: (error) Possible null pointer dereference: iobuf
Change-Id: I20e0e3ac1d600b2f2120b8d8536cd6d9e17023e8
BUG: 1109180
Signed-off-by: Kaleb S. KEITHLEY <kkeithle@redhat.com>
Reviewed-on: http://review.gluster.org/8064
Tested-by: Gluster Build System <jenkins@build.gluster.com>
Reviewed-by: Vijay Bellur <vbellur@redhat.com>
Diffstat (limited to 'xlators')
-rw-r--r-- | xlators/cluster/dht/src/dht-rebalance.c | 2 | ||||
-rw-r--r-- | xlators/cluster/stripe/src/stripe.c | 7 | ||||
-rw-r--r-- | xlators/mgmt/glusterd/src/glusterd-geo-rep.c | 12 | ||||
-rw-r--r-- | xlators/mgmt/glusterd/src/glusterd-replace-brick.c | 15 | ||||
-rw-r--r-- | xlators/mgmt/glusterd/src/glusterd-sm.c | 3 | ||||
-rw-r--r-- | xlators/mgmt/glusterd/src/glusterd-snapshot.c | 14 | ||||
-rw-r--r-- | xlators/mgmt/glusterd/src/glusterd-utils.c | 14 | ||||
-rw-r--r-- | xlators/mount/fuse/src/fuse-bridge.c | 5 | ||||
-rw-r--r-- | xlators/nfs/server/src/nfs-common.c | 6 | ||||
-rw-r--r-- | xlators/performance/quick-read/src/quick-read.c | 11 |
10 files changed, 57 insertions, 32 deletions
diff --git a/xlators/cluster/dht/src/dht-rebalance.c b/xlators/cluster/dht/src/dht-rebalance.c index 42c9cde491f..2ca636c4564 100644 --- a/xlators/cluster/dht/src/dht-rebalance.c +++ b/xlators/cluster/dht/src/dht-rebalance.c @@ -1974,7 +1974,7 @@ out: { status = dict_new (); gf_defrag_status_get (defrag, status); - if (ctx->notify) + if (ctx && ctx->notify) ctx->notify (GF_EN_DEFRAG_STATUS, status); if (status) dict_unref (status); diff --git a/xlators/cluster/stripe/src/stripe.c b/xlators/cluster/stripe/src/stripe.c index 20b889b35e1..7783603c0a5 100644 --- a/xlators/cluster/stripe/src/stripe.c +++ b/xlators/cluster/stripe/src/stripe.c @@ -4933,8 +4933,11 @@ out: if (!count) { /* all entries are directories */ frame->local = NULL; - STRIPE_STACK_UNWIND (readdir, frame, local->op_ret, - local->op_errno, &local->entries, NULL); + STRIPE_STACK_UNWIND (readdir, frame, + local ? local->op_ret : -1, + local ? local->op_errno : EINVAL, + local ? &local->entries : NULL, + NULL); gf_dirent_free (&local->entries); stripe_local_wipe (local); mem_put (local); diff --git a/xlators/mgmt/glusterd/src/glusterd-geo-rep.c b/xlators/mgmt/glusterd/src/glusterd-geo-rep.c index a88729c0e6a..b48ea6f40fd 100644 --- a/xlators/mgmt/glusterd/src/glusterd-geo-rep.c +++ b/xlators/mgmt/glusterd/src/glusterd-geo-rep.c @@ -1713,9 +1713,15 @@ glusterd_op_stage_sys_exec (dict_t *dict, char **op_errstr) out: if (ret) { - if (errmsg[0] == '\0') - snprintf (errmsg, sizeof (errmsg), "%s not found.", - command); + if (errmsg[0] == '\0') { + if (command) + snprintf (errmsg, sizeof (errmsg), + "gsync peer_%s command not found.", + command); + else + snprintf (errmsg, sizeof (errmsg), "%s", + "gsync peer command was not specified"); + } *op_errstr = gf_strdup (errmsg); gf_log ("", GF_LOG_ERROR, "%s", errmsg); } diff --git a/xlators/mgmt/glusterd/src/glusterd-replace-brick.c b/xlators/mgmt/glusterd/src/glusterd-replace-brick.c index 5a98d497137..11e15c55f46 100644 --- a/xlators/mgmt/glusterd/src/glusterd-replace-brick.c +++ b/xlators/mgmt/glusterd/src/glusterd-replace-brick.c @@ -896,7 +896,7 @@ rb_generate_client_volfile (glusterd_volinfo_t *volinfo, "%s", strerror (errno)); goto out; } - close (fd); + sys_close (fd); file = fopen (filename, "w+"); if (!file) { @@ -920,12 +920,14 @@ rb_generate_client_volfile (glusterd_volinfo_t *volinfo, glusterd_auth_get_username (volinfo), glusterd_auth_get_password (volinfo)); - fclose (file); GF_FREE (ttype); ret = 0; out: + if (file) + fclose (file); + return ret; } @@ -969,13 +971,13 @@ rb_generate_dst_brick_volfile (glusterd_volinfo_t *volinfo, priv->workdir, volinfo->volname, RB_DSTBRICKVOL_FILENAME); - fd = creat (filename, S_IRUSR | S_IWUSR); + fd = sys_creat (filename, S_IRUSR | S_IWUSR); if (fd < 0) { gf_log (this->name, GF_LOG_ERROR, "%s", strerror (errno)); goto out; } - close (fd); + sys_close (fd); file = fopen (filename, "w+"); if (!file) { @@ -1005,11 +1007,12 @@ rb_generate_dst_brick_volfile (glusterd_volinfo_t *volinfo, GF_FREE (trans_type); - fclose (file); - ret = 0; out: + if (file) + fclose (file); + return ret; } diff --git a/xlators/mgmt/glusterd/src/glusterd-sm.c b/xlators/mgmt/glusterd/src/glusterd-sm.c index ca047bd3322..6b30361e3d5 100644 --- a/xlators/mgmt/glusterd/src/glusterd-sm.c +++ b/xlators/mgmt/glusterd/src/glusterd-sm.c @@ -246,7 +246,8 @@ glusterd_ac_reverse_probe_begin (glusterd_friend_sm_event_t *event, void *ctx) out: if (ret) { GF_FREE (new_event); - GF_FREE (new_ev_ctx->hostname); + if (new_ev_ctx) + GF_FREE (new_ev_ctx->hostname); GF_FREE (new_ev_ctx); } gf_log ("", GF_LOG_DEBUG, "returning with %d", ret); diff --git a/xlators/mgmt/glusterd/src/glusterd-snapshot.c b/xlators/mgmt/glusterd/src/glusterd-snapshot.c index 097613184a0..dbe31610d19 100644 --- a/xlators/mgmt/glusterd/src/glusterd-snapshot.c +++ b/xlators/mgmt/glusterd/src/glusterd-snapshot.c @@ -7403,6 +7403,20 @@ glusterd_snapshot_restore_postop (dict_t *dict, int32_t op_ret, goto out; } + ret = dict_get_str (dict, "snapname", &name); + if (ret) { + gf_log (this->name, GF_LOG_ERROR, "getting the snap " + "name failed (volume: %s)", volinfo->volname); + goto out; + } + + snap = glusterd_find_snap_by_name (name); + if (!snap) { + gf_log (this->name, GF_LOG_ERROR, "snap %s is not found", name); + ret = -1; + goto out; + } + /* On success perform the cleanup operation */ if (0 == op_ret) { ret = glusterd_snapshot_restore_cleanup (rsp_dict, volinfo, diff --git a/xlators/mgmt/glusterd/src/glusterd-utils.c b/xlators/mgmt/glusterd/src/glusterd-utils.c index e285a499a5c..6eaa5f6d05a 100644 --- a/xlators/mgmt/glusterd/src/glusterd-utils.c +++ b/xlators/mgmt/glusterd/src/glusterd-utils.c @@ -4647,6 +4647,8 @@ glusterd_delete_stale_volume (glusterd_volinfo_t *stale_volinfo, GF_ASSERT (stale_volinfo); GF_ASSERT (valid_volinfo); + this = THIS; + GF_ASSERT (this); /* Copy snap_volumes list from stale_volinfo to valid_volinfo */ valid_volinfo->snap_count = 0; @@ -6822,12 +6824,12 @@ glusterd_brick_start (glusterd_volinfo_t *volinfo, int ret = -1; xlator_t *this = NULL; - if ((!brickinfo) || (!volinfo)) - goto out; - this = THIS; GF_ASSERT (this); + if ((!brickinfo) || (!volinfo)) + goto out; + if (uuid_is_null (brickinfo->uuid)) { ret = glusterd_resolve_brick (brickinfo); if (ret) { @@ -7821,14 +7823,14 @@ glusterd_brick_stop (glusterd_volinfo_t *volinfo, xlator_t *this = NULL; glusterd_conf_t *conf = NULL; - if ((!brickinfo) || (!volinfo)) - goto out; - this = THIS; GF_ASSERT (this); conf = this->private; GF_ASSERT (conf); + if ((!brickinfo) || (!volinfo)) + goto out; + if (uuid_is_null (brickinfo->uuid)) { ret = glusterd_resolve_brick (brickinfo); if (ret) { diff --git a/xlators/mount/fuse/src/fuse-bridge.c b/xlators/mount/fuse/src/fuse-bridge.c index 6290d0a783f..97031e82733 100644 --- a/xlators/mount/fuse/src/fuse-bridge.c +++ b/xlators/mount/fuse/src/fuse-bridge.c @@ -3107,6 +3107,8 @@ fuse_setxattr (xlator_t *this, fuse_in_header_t *finh, void *msg) priv = this->private; + GET_STATE (this, finh, state); + #ifdef GF_DARWIN_HOST_OS if (fsi->position) { gf_log ("glusterfs-fuse", GF_LOG_WARNING, @@ -3167,7 +3169,6 @@ fuse_setxattr (xlator_t *this, fuse_in_header_t *finh, void *msg) return; } - GET_STATE (this, finh, state); state->size = fsi->size; fuse_resolve_inode_init (state, &state->resolve, finh->nodeid); @@ -4715,7 +4716,7 @@ fuse_thread_proc (void *data) fuse_private_t *priv = NULL; ssize_t res = 0; struct iobuf *iobuf = NULL; - fuse_in_header_t *finh; + fuse_in_header_t *finh = NULL; struct iovec iov_in[2]; void *msg = NULL; const size_t msg0_size = sizeof (*finh) + 128; diff --git a/xlators/nfs/server/src/nfs-common.c b/xlators/nfs/server/src/nfs-common.c index f74396ee893..f942d37d6e1 100644 --- a/xlators/nfs/server/src/nfs-common.c +++ b/xlators/nfs/server/src/nfs-common.c @@ -77,15 +77,15 @@ nfs_xlator_to_xlid (xlator_list_t *cl, xlator_t *xl) xlator_t * nfs_mntpath_to_xlator (xlator_list_t *cl, char *path) { - char volname[MNTPATHLEN]; + char *volname = NULL; char *volptr = NULL; - size_t pathlen; + size_t pathlen; xlator_t *targetxl = NULL; if ((!cl) || (!path)) return NULL; - strncpy (volname, path, MNTPATHLEN); + volname = strdupa (path); pathlen = strlen (volname); gf_log (GF_NFS, GF_LOG_TRACE, "Subvolume search: %s", path); if (volname[0] == '/') diff --git a/xlators/performance/quick-read/src/quick-read.c b/xlators/performance/quick-read/src/quick-read.c index b8b4c532674..0e4ce71a571 100644 --- a/xlators/performance/quick-read/src/quick-read.c +++ b/xlators/performance/quick-read/src/quick-read.c @@ -545,8 +545,6 @@ qr_readv_cached (call_frame_t *frame, qr_inode_t *qr_inode, size_t size, LOCK (&table->lock); { - op_ret = -1; - if (!qr_inode->data) goto unlock; @@ -582,19 +580,16 @@ qr_readv_cached (call_frame_t *frame, qr_inode_t *qr_inode, size_t size, unlock: UNLOCK (&table->lock); - if (op_ret > 0) { + if (op_ret >= 0) { iov.iov_base = iobuf->ptr; iov.iov_len = op_ret; STACK_UNWIND_STRICT (readv, frame, op_ret, 0, &iov, 1, &buf, iobref, xdata); } + iobuf_unref (iobuf); - if (iobuf) - iobuf_unref (iobuf); - - if (iobref) - iobref_unref (iobref); + iobref_unref (iobref); return op_ret; } |