Skip to contentSkip to conversation

Lustre Klocwork fixes

PATCH New Oleg Drokin 1 participant 5 messages Apr 28, 2014 Staging

lore
Share & tools
lore

Oleg Drokin

author
#0 lore raw

v3 cover Lustre Klocwork fixes

This is just splitting big lnet fixes patch from Klocwork static analysis tool into smaller bits.
Reworked according to previous comments.
Also, this time with correct CC list.

Dmitry Eremin (2):
staging/lustre/lnet: remove unused variable in
lnet_destroy_remote_nets_table
staging/lustre/lnet: fix potential null pointer dereference in
kiblnd_rejected

Oleg Drokin (2):
staging/lustre/lnet: Drop useless LASSERT in ksocknal_select_ips staging/lustre/lnet: fix potential null pointer dereference

 drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd_cb.c | 8 ++++++--
 drivers/staging/lustre/lnet/klnds/socklnd/socklnd.c    | 4 ----
 drivers/staging/lustre/lnet/lnet/api-ni.c              | 6 +++---
 drivers/staging/lustre/lnet/lnet/router.c              | 2 +-
 4 files changed, 10 insertions(+), 10 deletions(-)
Signature · 1 line
1.9.0
author
#1 lore raw

PATCH v3 1/4 staging/lustre/lnet: Drop useless LASSERT in ksocknal_select_ips

It should never be NULL because our interface list is up to date, and even if it does, we'll just crash anyway so we are no better off.

Signed-off-by Oleg Drokin <[email protected]>
---
 drivers/staging/lustre/lnet/klnds/socklnd/socklnd.c | 4 ----
 1 file changed, 4 deletions(-)
drivers/staging/lustre/lnet/klnds/socklnd/socklnd.c +0 -4
index 21d36ee..a391d13 100644
@@ -793,8 +793,6 @@ ksocknal_select_ips(ksock_peer_t *peer, __u32 *peerips, int n_peerips)
 			ip = peer->ksnp_passive_ips[i];
 			best_iface = ksocknal_ip2iface(peer->ksnp_ni, ip);
 
-			/* peer passive ips are kept up to date */
-			LASSERT(best_iface != NULL);
 		} else {
 			/* choose a new interface */
 			LASSERT (i == peer->ksnp_n_passive_ips);

@@ -835,8 +833,6 @@ ksocknal_select_ips(ksock_peer_t *peer, __u32 *peerips, int n_peerips)
 			peer->ksnp_n_passive_ips = i+1;
 		}
 
-		LASSERT (best_iface != NULL);
-
 		/* mark the best matching peer IP used */
 		j = ksocknal_match_peerip(best_iface, peerips, n_peerips);
 		peerips[j] = 0;
Signature · 1 line
1.9.0
author
#2 lore raw

PATCH v3 2/4 staging/lustre/lnet: remove unused variable in lnet_destroy_remote_nets_table

In reply to Oleg Drokin

From: Dmitry Eremin <[email protected]>

Local variable 'hash' is never used
found by Klocwork Insight tool

Signed-off-by Dmitry Eremin <[email protected]>

Reviewed-on: http://review.whamcloud.com/9386
Intel-bug-id: https://jira.hpdd.intel.com/browse/LU-4629

Reviewed-by John L. Hammond <[email protected]>
Reviewed-by Isaac Huang <[email protected]>
Signed-off-by Oleg Drokin <[email protected]>
---
 drivers/staging/lustre/lnet/lnet/api-ni.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
drivers/staging/lustre/lnet/lnet/api-ni.c +3 -3
index 85b8d81..3f1fdaa 100644
@@ -127,8 +127,7 @@ lnet_create_remote_nets_table(void)
 static void
 lnet_destroy_remote_nets_table(void)
 {
-	int		i;
-	struct list_head	*hash;
+	int i;
 
 	if (the_lnet.ln_remote_nets_hash == NULL)
 		return;

@@ -137,7 +136,8 @@ lnet_destroy_remote_nets_table(void)
 		LASSERT(list_empty(&the_lnet.ln_remote_nets_hash[i]));
 
 	LIBCFS_FREE(the_lnet.ln_remote_nets_hash,
-		    LNET_REMOTE_NETS_HASH_SIZE * sizeof(*hash));
+		    LNET_REMOTE_NETS_HASH_SIZE *
+		    sizeof(the_lnet.ln_remote_nets_hash[0]));
 	the_lnet.ln_remote_nets_hash = NULL;
 }
Signature · 1 line
1.9.0
author
#3 lore raw

PATCH v3 3/4 staging/lustre/lnet: fix potential null pointer dereference in kiblnd_rejected

In reply to Oleg Drokin

From: Dmitry Eremin <[email protected]>

Null pointer 'cp' that comes from line 2544 may be dereferenced at line 2618.
found by Klocwork Insight tool

Signed-off-by Dmitry Eremin <[email protected]>

Reviewed-on: http://review.whamcloud.com/9386
Intel-bug-id: https://jira.hpdd.intel.com/browse/LU-4629

Reviewed-by John L. Hammond <[email protected]>
Reviewed-by Isaac Huang <[email protected]>
Signed-off-by Oleg Drokin <[email protected]>
---
 drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd_cb.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)
drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd_cb.c +6 -2
index 6173e74..9bf6c94 100644
@@ -2609,13 +2609,17 @@ kiblnd_rejected (kib_conn_t *conn, int reason, void *priv, int priv_nob)
 
 			case IBLND_REJECT_MSG_QUEUE_SIZE:
 				CERROR("%s rejected: incompatible message queue depth %d, %d\n",
-				       libcfs_nid2str(peer->ibp_nid), cp->ibcp_queue_depth,
+				       libcfs_nid2str(peer->ibp_nid),
+				       cp != NULL ? cp->ibcp_queue_depth :
+				       IBLND_MSG_QUEUE_SIZE(rej->ibr_version),
 				       IBLND_MSG_QUEUE_SIZE(conn->ibc_version));
 				break;
 
 			case IBLND_REJECT_RDMA_FRAGS:
 				CERROR("%s rejected: incompatible # of RDMA fragments %d, %d\n",
-				       libcfs_nid2str(peer->ibp_nid), cp->ibcp_max_frags,
+				       libcfs_nid2str(peer->ibp_nid),
+				       cp != NULL ? cp->ibcp_max_frags :
+				       IBLND_RDMA_FRAGS(rej->ibr_version),
 				       IBLND_RDMA_FRAGS(conn->ibc_version));
 				break;
Signature · 1 line
1.9.0
author
#4 lore raw

PATCH v3 4/4 staging/lustre/lnet: fix potential null pointer dereference

In reply to Oleg Drokin

Pointer 'ni' checked for NULL at line 1569 may be passed to function and may be dereferenced there by passing argument 1 to function 'lnet_ni_notify_locked' at line 1621.
found by Klocwork Insight tool

Signed-off-by Oleg Drokin <[email protected]>
CC Dmitry Eremin <[email protected]>
CC Liang Zhen <[email protected]>
---
 drivers/staging/lustre/lnet/lnet/router.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/staging/lustre/lnet/lnet/router.c +1 -1
index 995f509..926923a 100644
@@ -145,7 +145,7 @@ lnet_ni_notify_locked(lnet_ni_t *ni, lnet_peer_t *lp)
 	 * NB individual events can be missed; the only guarantee is that you
 	 * always get the most recent news */
 
-	if (lp->lp_notifying)
+	if (lp->lp_notifying || ni == NULL)
 		return;
 
 	lp->lp_notifying = 1;
Signature · 1 line
1.9.0