[Gluster-devel] Fwd: New Defects reported by Coverity Scan for GlusterFS

Pranith Kumar Karampuri pkarampu at redhat.com
Wed Mar 26 07:17:40 UTC 2014


Thanks Lala, Ravi (CCed) and I will fix them as soon as possible.

Pranith
----- Original Message -----
> From: "Lalatendu Mohanty" <lmohanty at redhat.com>
> To: gluster-devel at nongnu.org
> Sent: Wednesday, March 26, 2014 12:32:29 PM
> Subject: [Gluster-devel] Fwd: New Defects reported by Coverity Scan for	GlusterFS
> 
> 
> FYI,
> 
> All new reported issues are from afr recent merge. I haven't gone through
> them, but if anyone some time, please go through them.
> 
> 
> -------- Original Message --------
> Subject: 	New Defects reported by Coverity Scan for GlusterFS
> Date: 	Tue, 25 Mar 2014 23:50:48 -0700
> From: 	scan-admin at coverity.com
> 
> Hi,
> 
> 
> Please find the latest report on new defect(s) introduced to GlusterFS found
> with Coverity Scan.
> 
> Defect(s) Reported-by: Coverity Scan
> Showing 12 of 12 defect(s)
> 
> 
> ** CID 1194648:  Dereference after null check  (FORWARD_NULL)
> /xlators/cluster/afr/src/afr-inode-write.c: 54 in
> __afr_inode_write_finalize()
> 
> ** CID 1194647:  Dereference after null check  (FORWARD_NULL)
> /xlators/cluster/afr/src/afr-dir-write.c: 1125 in afr_rename()
> 
> ** CID 1194651:  Data race condition  (MISSING_LOCK)
> /xlators/cluster/afr/src/afr-dir-write.c: 141 in __afr_dir_write_finalize()
> 
> ** CID 1194650:  Data race condition  (MISSING_LOCK)
> /xlators/cluster/afr/src/afr-inode-write.c: 96 in
> __afr_inode_write_finalize()
> 
> ** CID 1194649:  Data race condition  (MISSING_LOCK)
> /xlators/cluster/afr/src/afr-inode-write.c: 180 in afr_writev_copy_outvars()
> 
> ** CID 1194652:  Dereference null return value  (NULL_RETURNS)
> /xlators/cluster/afr/src/afr-inode-write.c: 670 in afr_ftruncate()
> 
> ** CID 1194653:  Resource leak  (RESOURCE_LEAK)
> /xlators/cluster/afr/src/afr-self-heal-entry.c: 191 in
> afr_selfheal_newentry_mark()
> /xlators/cluster/afr/src/afr-self-heal-entry.c: 174 in
> afr_selfheal_newentry_mark()
> 
> ** CID 1194643:  Unchecked return value  (CHECKED_RETURN)
> /xlators/cluster/afr/src/afr-common.c: 491 in afr_selfheal_enabled()
> 
> ** CID 1194642:  Unchecked return value  (CHECKED_RETURN)
> /xlators/cluster/afr/src/afr-common.c: 467 in afr_refresh_selfheal_wrap()
> 
> ** CID 1194644:  Operands don't affect result  (CONSTANT_EXPRESSION_RESULT)
> /xlators/cluster/afr/src/afr-self-heald.c: 1138 in afr_xl_op()
> 
> ** CID 1194645:  Copy-paste error  (COPY_PASTE_ERROR)
> /xlators/cluster/afr/src/afr-common.c: 1629 in afr_discover_do()
> 
> ** CID 1194646:  Logically dead code  (DEADCODE)
> /xlators/cluster/afr/src/pump.c: 1448 in pump_getxattr()
> 
> 
> ________________________________________________________________________________________________________
> *** CID 1194648:  Dereference after null check  (FORWARD_NULL)
> /xlators/cluster/afr/src/afr-inode-write.c: 54 in
> __afr_inode_write_finalize()
> 48     	int read_subvol = 0;
> 49     	int i = 0;
> 50
> 51     	local = frame->local;
> 52     	priv = this->private;
> 53
> >>>     CID 1194648:  Dereference after null check  (FORWARD_NULL)
> >>>     Comparing "local->inode" to null implies that "local->inode" might be
> >>>     null.
> 54     	if (local->inode) {
> 55     		if (local->transaction.type == AFR_METADATA_TRANSACTION)
> 56     			read_subvol = afr_metadata_subvol_get (local->inode, this,
> 57     							       NULL, NULL);
> 58     		else
> 59     			read_subvol = afr_data_subvol_get (local->inode, this,
> 
> ________________________________________________________________________________________________________
> *** CID 1194647:  Dereference after null check  (FORWARD_NULL)
> /xlators/cluster/afr/src/afr-dir-write.c: 1125 in afr_rename()
> 1119             QUORUM_CHECK(rename,out);
> 1120
> 1121             transaction_frame = copy_frame (frame);
> 1122             if (!transaction_frame)
> 1123                     op_errno = ENOMEM;
> 1124
> >>>     CID 1194647:  Dereference after null check  (FORWARD_NULL)
> >>>     Dereferencing null pointer "transaction_frame".
> 1125     	local = AFR_FRAME_INIT (transaction_frame, op_errno);
> 1126     	if (!local)
> 1127     		goto out;
> 1128
> 1129             loc_copy (&local->loc,    oldloc);
> 1130             loc_copy (&local->newloc, newloc);
> 
> ________________________________________________________________________________________________________
> *** CID 1194651:  Data race condition  (MISSING_LOCK)
> /xlators/cluster/afr/src/afr-dir-write.c: 141 in __afr_dir_write_finalize()
> 135     				local->replies[i].postparent;
> 136     			local->cont.dir_fop.prenewparent =
> 137     				local->replies[i].preparent2;
> 138     			local->cont.dir_fop.postnewparent =
> 139     				local->replies[i].postparent2;
> 140     			if (local->replies[i].xdata)
> >>>     CID 1194651:  Data race condition  (MISSING_LOCK)
> >>>     Accessing "local->xdata_rsp" without holding lock
> >>>     "_call_frame_t.lock". Elsewhere, "local->xdata_rsp" is accessed with
> >>>     "_call_frame_t.lock" held 14 out of 18 times.
> 141     				local->xdata_rsp =
> 142     					dict_ref (local->replies[i].xdata);
> 143     			continue;
> 144     		}
> 145
> 146     		if (i == inode_read_subvol) {
> 
> ________________________________________________________________________________________________________
> *** CID 1194650:  Data race condition  (MISSING_LOCK)
> /xlators/cluster/afr/src/afr-inode-write.c: 96 in
> __afr_inode_write_finalize()
> 90     			local->cont.inode_wfop.postbuf =
> 91     				local->replies[i].poststat;
> 92
> 93     			if (local->replies[i].xdata) {
> 94     				if (local->xdata_rsp)
> 95     					dict_unref (local->xdata_rsp);
> >>>     CID 1194650:  Data race condition  (MISSING_LOCK)
> >>>     Accessing "local->xdata_rsp" without holding lock
> >>>     "_call_frame_t.lock". Elsewhere, "local->xdata_rsp" is accessed with
> >>>     "_call_frame_t.lock" held 14 out of 18 times.
> 96     				local->xdata_rsp =
> 97     					dict_ref (local->replies[i].xdata);
> 98     			}
> 99     		}
> 100     	}
> 101     }
> 
> ________________________________________________________________________________________________________
> *** CID 1194649:  Data race condition  (MISSING_LOCK)
> /xlators/cluster/afr/src/afr-inode-write.c: 180 in afr_writev_copy_outvars()
> 174
> 175             dst_local->op_ret = src_local->op_ret;
> 176             dst_local->op_errno = src_local->op_errno;
> 177             dst_local->cont.inode_wfop.prebuf =
> src_local->cont.inode_wfop.prebuf;
> 178             dst_local->cont.inode_wfop.postbuf =
> src_local->cont.inode_wfop.postbuf;
> 179     	if (src_local->xdata_rsp)
> >>>     CID 1194649:  Data race condition  (MISSING_LOCK)
> >>>     Accessing "dst_local->xdata_rsp" without holding lock
> >>>     "_call_frame_t.lock". Elsewhere, "dst_local->xdata_rsp" is accessed
> >>>     with "_call_frame_t.lock" held 14 out of 18 times.
> 180     		dst_local->xdata_rsp = dict_ref (src_local->xdata_rsp);
> 181     }
> 182
> 183     void
> 184     afr_writev_unwind (call_frame_t *frame, xlator_t *this)
> 185     {
> 
> ________________________________________________________________________________________________________
> *** CID 1194652:  Dereference null return value  (NULL_RETURNS)
> /xlators/cluster/afr/src/afr-inode-write.c: 670 in afr_ftruncate()
> 664             QUORUM_CHECK(ftruncate,out);
> 665
> 666     	transaction_frame = copy_frame (frame);
> 667     	if (!frame)
> 668     		goto out;
> 669
> >>>     CID 1194652:  Dereference null return value  (NULL_RETURNS)
> >>>     Dereferencing a null pointer "transaction_frame".
> 670             local = AFR_FRAME_INIT (transaction_frame, op_errno);
> 671             if (!local)
> 672     		goto out;
> 673
> 674             local->cont.ftruncate.offset  = offset;
> 675     	if (xdata)
> 
> ________________________________________________________________________________________________________
> *** CID 1194653:  Resource leak  (RESOURCE_LEAK)
> /xlators/cluster/afr/src/afr-self-heal-entry.c: 191 in
> afr_selfheal_newentry_mark()
> 185     		if (!sources[i])
> 186     			continue;
> 187     		afr_selfheal_post_op (frame, this, inode, i, xattr);
> 188     	}
> 189
> 190     	dict_unref (xattr);
> >>>     CID 1194653:  Resource leak  (RESOURCE_LEAK)
> >>>     Variable "changelog" going out of scope leaks the storage it points
> >>>     to.
> 191     	return ret;
> 192     }
> 193
> 194
> 195     static int
> 196     __afr_selfheal_heal_dirent (call_frame_t *frame, xlator_t *this, fd_t
> *fd,
> /xlators/cluster/afr/src/afr-self-heal-entry.c: 174 in
> afr_selfheal_newentry_mark()
> 168     	uuid_copy (inode->gfid, replies[source].poststat.ia_gfid);
> 169
> 170     	changelog = afr_matrix_create (priv->child_count,
> AFR_NUM_CHANGE_LOGS);
> 171
> 172     	xattr = dict_new();
> 173     	if (!xattr)
> >>>     CID 1194653:  Resource leak  (RESOURCE_LEAK)
> >>>     Variable "changelog" going out of scope leaks the storage it points
> >>>     to.
> 174     		return -ENOMEM;
> 175
> 176     	for (i = 0; i < priv->child_count; i++) {
> 177     		if (!newentry[i])
> 178     			continue;
> 179     		changelog[i][idx] = hton32(1);
> 
> ________________________________________________________________________________________________________
> *** CID 1194643:  Unchecked return value  (CHECKED_RETURN)
> /xlators/cluster/afr/src/afr-common.c: 491 in afr_selfheal_enabled()
> 485     {
> 486     	afr_private_t *priv = NULL;
> 487     	gf_boolean_t data = _gf_false;
> 488
> 489     	priv = this->private;
> 490
> >>>     CID 1194643:  Unchecked return value  (CHECKED_RETURN)
> >>>     No check of the return value of
> >>>     "gf_string2boolean(priv->data_self_heal, &data)".
> 491     	gf_string2boolean (priv->data_self_heal, &data);
> 492
> 493     	return data || priv->metadata_self_heal || priv->entry_self_heal;
> 494     }
> 495
> 496
> 
> ________________________________________________________________________________________________________
> *** CID 1194642:  Unchecked return value  (CHECKED_RETURN)
> /xlators/cluster/afr/src/afr-common.c: 467 in afr_refresh_selfheal_wrap()
> 461
> 462     	local = frame->local;
> 463     	this = frame->this;
> 464
> 465     	afr_selfheal (frame->this, local->refreshinode->gfid);
> 466
> >>>     CID 1194642:  Unchecked return value  (CHECKED_RETURN)
> >>>     No check of the return value of
> >>>     "afr_selfheal_unlocked_discover(frame, local->refreshinode,
> >>>     local->refreshinode->gfid, local->replies)".
> 467     	afr_selfheal_unlocked_discover (frame, local->refreshinode,
> 468     					local->refreshinode->gfid,
> 469     					local->replies);
> 470
> 471     	afr_replies_interpret (frame, this, local->refreshinode);
> 472
> 
> ________________________________________________________________________________________________________
> *** CID 1194644:  Operands don't affect result  (CONSTANT_EXPRESSION_RESULT)
> /xlators/cluster/afr/src/afr-self-heald.c: 1138 in afr_xl_op()
> 1132     	int64_t cnt = 0;
> 1133
> 1134     	priv = this->private;
> 1135     	shd = &priv->shd;
> 1136
> 1137     	for (i = 0; i < priv->child_count; i++)
> >>>     CID 1194644:  Operands don't affect result
> >>>     (CONSTANT_EXPRESSION_RESULT)
> >>>     "priv->child_up[i] == -1" is always false regardless of the values of
> >>>     its operands. This occurs as the logical operand of if.
> 1138     		if (priv->child_up[i] == -1)
> 1139     			goto out;
> 1140
> 1141             ret = dict_get_int32 (input, "xl-op", (int32_t*)&op);
> 1142             if (ret)
> 1143                     goto out;
> 
> ________________________________________________________________________________________________________
> *** CID 1194645:  Copy-paste error  (COPY_PASTE_ERROR)
> /xlators/cluster/afr/src/afr-common.c: 1629 in afr_discover_do()
> 1623
> 1624     	local = frame->local;
> 1625     	priv = this->private;
> 1626
> 1627     	if (err) {
> 1628     		local->op_errno = -err;
> >>>     CID 1194645:  Copy-paste error  (COPY_PASTE_ERROR)
> >>>     "ret" in "ret = -1" looks like a copy-paste error.  Should it say
> >>>     "err" instead?
> 1629     		ret = -1;
> 1630     		goto out;
> 1631     	}
> 1632
> 1633     	call_count = local->call_count = AFR_COUNT (local->child_up,
> 1634     						    priv->child_count);
> 
> ________________________________________________________________________________________________________
> *** CID 1194646:  Logically dead code  (DEADCODE)
> /xlators/cluster/afr/src/pump.c: 1448 in pump_getxattr()
> 1442
> 1443     	afr_getxattr (frame, this, loc, name, xdata);
> 1444
> 1445     	ret = 0;
> 1446     out:
> 1447     	if (ret < 0)
> >>>     CID 1194646:  Logically dead code  (DEADCODE)
> >>>     Execution cannot reach this statement "do  {
>   afr_local_t *__loca...".
> 1448     		AFR_STACK_UNWIND (getxattr, frame, -1, op_errno, NULL, NULL);
> 1449     	return 0;
> 1450     }
> 1451
> 1452     int
> 1453     pump_command_reply (call_frame_t *frame, xlator_t *this)
> 
> 
> ________________________________________________________________________________________________________
> To view the defects in Coverity Scan visit,
> http://scan.coverity.com/projects/987?tab=Overview To unsubscribe from the
> email notification for new defects,
> http://scan5.coverity.com/cgi-bin/unsubscribe.py
> 
> 
> 
> _______________________________________________
> Gluster-devel mailing list
> Gluster-devel at nongnu.org
> https://lists.nongnu.org/mailman/listinfo/gluster-devel
> 




More information about the Gluster-devel mailing list