glusterfs.git/xlators/cluster/dht/src, branch v3.6.6

core: fix Ubuntu code audit (cppcheck) results

2015-06-10T12:03:01+00:00

See also http://review.gluster.org/#/c/8064/, BZ 1109180, and
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: Ia5a256281156dd1df2fa900caea7694d9d0a7077
BUG: 1122290
Signed-off-by: Kaleb S. KEITHLEY 
Reviewed-on: http://review.gluster.org/8351
Tested-by: Gluster Build System 
Reviewed-by: Vijay Bellur 
Reviewed-by: Raghavendra Bhat

cluster/dht: Unwind with proper op_ret

2015-05-04T20:25:36+00:00

1. Expected behavior of get_real_filename feature.

A getxattr on a existing dir with glusterfs.get_real_filename:
as key should result in one of the following things.

a. A value returned for that key having the real filename (a file whose
match is a case insensitive match to the filename passed in key).
b. op_ret = -1 and errno set to ENOENT meaning that no such file exists
under the specified dir in any case.
c. op_ret = -1 and errno set to ENODATA. This is a case assuming no
xlator interprets the glusterfs.get_real_filename key and it get
passed down to the posix xlator. Naturally, posix xlator would not
find any xattr with this key and would return ENODATA. This will be
interpreted specially by the caller as the feature not being supported
by underlying glusterfs.

2. What assumptions are wrong?
Initially the key used to be user.glusterfs.get_real_filename.
In that case, when posix xlator did a getxattr call it would have
received ENODATA as error. However, the key has now changed to
glusterfs.get_real_filename. This leads to a EOPNOTSUPP error instead.

Considering the above information, this is a rewrite of
get_real_filename logic in dht.

Change-Id: I012e9150047fc8563be91b0d112a368ac1cbf598
BUG: 1204140
Signed-off-by: Raghavendra Talur 
Reviewed-on: http://review.gluster.org/9956
Tested-by: Gluster Build System 
Reviewed-by: N Balachandran 
Reviewed-by: Shyamsundar Ranganathan 
(cherry picked from commit 331e705b6a458600c0b5cbcf2b0f7b9e1167bdc2)
Reviewed-on: http://review.gluster.org/10403

cluster/dht: Fix incorrect updates to parent times

2015-02-03T13:16:00+00:00

        Backport of: http://review.gluster.org/9457

In directory write FOPs, as far as updates to timestamps associated
with parent by DHT is concerned, there are three possibilities:
a) time (in sec) gotten from child of DHT < time (in sec) in inode ctx
b) time (in sec) gotten from child of DHT = time (in sec) in inode ctx
c) time (in sec) gotten from child of DHT > time (in sec) in inode ctx

In case (c), for time in nsecs, it is the value returned by DHT's child
that must be selected. But what DHT_UPDATE_TIME ends up doing is to choose
the maximum of (time in nsec gotten from DHT's child, time in nsec in inode ctx).

Change-Id: I1046bb7147ab48eca5c477be03ef2d4630e7752d
BUG: 1186119
Signed-off-by: Krutika Dhananjay 
Reviewed-on: http://review.gluster.org/9490
Tested-by: Gluster Build System 
Reviewed-by: Pranith Kumar Karampuri 
Reviewed-by: Raghavendra Bhat

Cluster/DHT : Fixed crash due to null deref

2015-01-21T12:00:09+00:00

A lookup on a linkto file whose trusted.glusterfs.dht.linkto
xattr points to a subvol that is not part of the volume
can cause the brick process to segfault due to a null dereference.
Modified to check for a non-null value before attempting to access
the variable.

> Change-Id: Ie8f9df058f842cfc0c2b52a8f147e557677386fa
> BUG: 1159571
> Signed-off-by: Nithya Balachandran 
> Reviewed-on: http://review.gluster.org/9034
> Tested-by: Gluster Build System 
> Reviewed-by: venkatesh somyajulu 
> Reviewed-by: Vijay Bellur 
> Signed-off-by: Raghavendra Bhat 

Change-Id: I53b086289d2386d269648653629a0750baae07a4
BUG: 1184191
Reviewed-on: http://review.gluster.org/9467
Reviewed-by: Vijay Bellur 
Reviewed-by: Shyamsundar Ranganathan 
Tested-by: Gluster Build System 
Reviewed-by: Raghavendra Bhat

cluster/dht: Change log level to avoid annoying logs

2015-01-05T12:59:53+00:00

[2015-01-04 08:03:23.820376] I [dht-common.c:1822:dht_lookup_cbk] 0-patchy-dht:
Entry /tls missing on subvol patchy-replicate-0

Change-Id: Id9eae47213ed39e8bf969e82cc7b935dfada4598
BUG: 1138385
Reviewed-on: http://review.gluster.org/9382
Tested-by: Gluster Build System 
Reviewed-by: Raghavendra Bhat

cluster/dht: Fix dict_t leaks in rebalance process' execution path

2014-09-20T08:56:06+00:00

        Backport of: http://review.gluster.org/8763

Two dict_t objects are leaked for every file migrated in success codepath.
It is the caller's responsibility to unref dict that it gets from calls to
syncop_getxattr(); and rebalance performs two syncop_getxattr()s per file
without freeing them.

Also, syncop_getxattr() on GF_XATTR_LINKINFO_KEY doesn't seem to be using
the response dict. Hence, NULL is now passed as opposed to @dict to
syncop_getxattr().

Change-Id: I48926389db965e006da151bf0ccb6bcaf3585199
BUG: 1144640
Signed-off-by: Krutika Dhananjay 
Reviewed-on: http://review.gluster.org/8785
Tested-by: Gluster Build System 
Reviewed-by: Vijay Bellur

Cluster/DHT: Changing rename log severity

2014-09-18T17:50:39+00:00

Changing log level for a rename message from debug
to info to improve debuggability

Change-Id: I53031fcf97fffd62095692477330ecde0cf47dcd
BUG: 1138395
Signed-off-by: Nithya Balachandran 
Reviewed-on-master: http://review.gluster.org/8582
Reviewed-by: Vijay Bellur 
Tested-by: Gluster Build System 
Signed-off-by: Shyam 
Reviewed-on: http://review.gluster.org/8626

cluster/dht: Changed log level to DEBUG

2014-09-18T17:50:12+00:00

Change-Id: I7a4ee0c5a6a94bd4f31aff510a2971750913ed45
BUG: 1142402
Signed-off-by: Venkatesh Somyajulu 
Reviewed-on-master: http://review.gluster.org/8621
Tested-by: Gluster Build System 
Reviewed-by: Xavier Hernandez 
Reviewed-by: susant palai 
Reviewed-by: Vijay Bellur 
Reviewed-on: http://review.gluster.org/8749

cluster/dht: Fixed double UNWIND in lookup everywhere code

2014-09-17T04:29:09+00:00

In dht_lookup_everywhere_done:
Line: 1194 we call DHT_STACK_UNWIND
and in the same if condition we go ahead and call, goto
unwind_hashed_and_cached; which at Line 1371 calls another UNWIND.

As is obvious, higher frames could cleanup their locals and on
receiving the next unwind could cause a coredump of the process.

Fixed the same by calling the required return post the first unwind

Change-Id: Ic5d57da98255b8616a65b4caaedabeba9144fd49
BUG: 1142409
Signed-off-by: Shyam 
Reviewed-on-master: http://review.gluster.org/8666
Reviewed-by: N Balachandran 
Reviewed-by: Vijay Bellur 
Reviewed-by: Jeff Darcy 
Tested-by: Gluster Build System 
Reviewed-by: susant palai 
Reviewed-on: http://review.gluster.org/8751

cluster/dht: fix memory corruption in locking api.

2014-09-17T04:25:56+00:00



     The  contents  of the array are sorted in ascending order
     according to a comparison function pointed to by compar, which is
     called with two arguments that "point to the objects being
     compared".



qsort passes "pointers to members of the array" to comparision
function. Since the members of the array happen to be (dht_lock_t *),
the arguments passed to dht_lock_request_cmp are of type (dht_lock_t
**). Previously we assumed them to be of type (dht_lock_t *), which
resulted in memory corruption.

Change-Id: Iee0758704434beaff3c3a1ad48d549cbdc9e1c96
BUG: 1142406
Signed-off-by: Raghavendra G 
Reviewed-on-master: http://review.gluster.org/8659
Tested-by: Gluster Build System 
Reviewed-by: Shyamsundar Ranganathan 
Reviewed-by: Vijay Bellur 
Reviewed-on: http://review.gluster.org/8750