andy/glusterfs
1
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

core: fix Ubuntu code audit (cppcheck) results

Browse Source 
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>
This commit is contained in:
Kaleb S. KEITHLEY 2014-06-13 11:09:25 -04:00 committed by Vijay Bellur
parent c6f27ed5cc
commit 4ea5b8d204
12 changed files with 64 additions and 51 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
cli/src/cli-rpc-ops.c
View File

@ -9179,11 +9179,10 @@ gf_cli_snapshot (call_frame_t *frame, xlator_t *this,
if (!frame || !this || !data)
goto out;
if (frame->local) {
local = frame->local;
} else {
if (!frame->local)
goto out;
}
local = frame->local;
options = data;
@ -9240,7 +9239,7 @@ gf_cli_snapshot (call_frame_t *frame, xlator_t *this,
ret = 0;
out:
if (ret && GF_SNAP_OPTION_TYPE_STATUS == type) {
if (ret && local && GF_SNAP_OPTION_TYPE_STATUS == type) {
tmp_ret = dict_get_str (local->dict, "op_err_str", &err_str);
if (err_str) {
cli_err ("Snapshot Status : failed: %s", err_str);

17
contrib/uuid/gen_uuid.c
View File

@ -44,18 +44,6 @@
#include <windows.h>
#define UUID MYUUID
#endif
#ifdef __APPLE__
#define PRI_TIME "ld"
#define PRI_TIME_USEC "d"
#define SCAN_TIME "lu"
#else
#define PRI_TIME "lu"
#define PRI_TIME_USEC "lu"
#define SCAN_TIME "ld"
#endif
#include <stdio.h>
#ifdef HAVE_UNISTD_H
#include <unistd.h>
@ -366,7 +354,8 @@ static int get_clock(uint32_t *clock_high, uint32_t *clock_low,
unsigned int cl;
unsigned long tv1, tv2;
int a;
if (fscanf(state_f, "clock: %04x tv: %" SCAN_TIME " %" SCAN_TIME " adj: %d\n",
if (fscanf(state_f, "clock: %04x tv: %lu %lu adj: %d\n",
&cl, &tv1, &tv2, &a) == 4) {
clock_seq = cl & 0x3FFF;
last.tv_sec = tv1;
@ -415,7 +404,7 @@ try_again:
if (state_fd > 0) {
rewind(state_f);
len = fprintf(state_f,
"clock: %04x tv: %016" PRI_TIME "%08" PRI_TIME_USEC "adj: %08d\n",
"clock: %04x tv: %016lu %08lu adj: %08d\n",
clock_seq, last.tv_sec, last.tv_usec, adjustment);
fflush(state_f);
if (ftruncate(state_fd, len) < 0) {

2
xlators/cluster/dht/src/dht-rebalance.c
View File

@ -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);

7
xlators/cluster/stripe/src/stripe.c
View File

@ -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);

12
xlators/mgmt/glusterd/src/glusterd-geo-rep.c
View File

@ -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);
}

15
xlators/mgmt/glusterd/src/glusterd-replace-brick.c
View File

@ -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;
}

3
xlators/mgmt/glusterd/src/glusterd-sm.c
View File

@ -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);

14
xlators/mgmt/glusterd/src/glusterd-snapshot.c
View File

@ -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,

14
xlators/mgmt/glusterd/src/glusterd-utils.c
View File

@ -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) {

5
xlators/mount/fuse/src/fuse-bridge.c
View File

@ -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;

6
xlators/nfs/server/src/nfs-common.c
View File

@ -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] == '/')

11
xlators/performance/quick-read/src/quick-read.c
View File

@ -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;
}