[PATCH net] net/smc: prevent device removal during connection setup

From: Chengfeng Ye <[email protected]>

smc_pnet_determine_gid() stores an smc_ib_device pointer in connection
initialization data while holding smc_ib_devices.mutex. Connection setup
keeps using the pointer after releasing the mutex, but device removal only
waits for links which have already incremented lnk_cnt.

This permits the following interleaving:

  connection setup                 device removal
  ----------------                 --------------
  select smcibdev
  store ini->smcrv2.ib_dev_v2
  mutex_unlock()
                                   remove smcibdev from the device list
                                   observe lnk_cnt == 0
                                   return from the remove callback
                                   kfree(smcibdev)
  copy smcibdev->pnetid

KASAN reported:

  BUG: KASAN: slab-use-after-free in smc_conn_create+0x1330/0x2680
  Read of size 16 at addr ffff888112dbd934 by task smc_bug86/97
  Call Trace:
   __asan_memcpy
   smc_conn_create
   __smc_connect
   smc_connect
   __sys_connect
  Allocated by task 93:
   smc_ib_add_dev
   add_client_context
   ib_register_device
   nldev_newlink
  Freed by task 98:
   kfree
   remove_client_context
   __ib_unregister_device
   nldev_dellink

Track selected-device users separately from established links. Acquire a
selection reference while smc_ib_devices.mutex protects the device list,
and release it when the connection initialization data is discarded. After
delisting a device, wait for these users before terminating its links. This
lets in-flight setup either fail or make any newly created link visible to
the termination pass, while delisting prevents new users.

Fixes: a046d57da19f ("smc: CLC handshake (incl. preparation steps)")
Cc: [email protected]
Signed-off-by: Chengfeng Ye <[email protected]>
Signed-off-by: Qi Zhang <[email protected]>
---
 net/smc/af_smc.c   | 11 +++++++----
 net/smc/smc_core.c | 11 +++++++++++
 net/smc/smc_core.h |  3 +++
 net/smc/smc_ib.c   |  5 +++++
 net/smc/smc_ib.h   | 13 +++++++++++++
 net/smc/smc_llc.c  |  8 ++++----
 net/smc/smc_pnet.c |  8 ++++++++
 7 files changed, 51 insertions(+), 8 deletions(-)

diff --git a/net/smc/af_smc.c b/net/smc/af_smc.c
index b5db69073e20..160271464305 100644
--- a/net/smc/af_smc.c
+++ b/net/smc/af_smc.c
@@ -1225,7 +1225,7 @@ void smc_fill_gid_list(struct smc_link_group *lgr,
 	       SMC_GID_SIZE);
 
 out:
-	kfree(alt_ini);
+	smc_init_info_free(alt_ini);
 }
 
 static int smc_connect_rdma_v2_prepare(struct smc_sock *smc,
@@ -1596,14 +1596,14 @@ static int __smc_connect(struct smc_sock *smc)
 	SMC_STAT_CLNT_SUCC_INC(sock_net(smc->clcsock->sk), aclc);
 	smc_connect_ism_vlan_cleanup(ini);
 	kfree(buf);
-	kfree(ini);
+	smc_init_info_free(ini);
 	return 0;
 
 vlan_cleanup:
 	smc_connect_ism_vlan_cleanup(ini);
 	kfree(buf);
 fallback:
-	kfree(ini);
+	smc_init_info_free(ini);
 	return smc_connect_decline_fallback(smc, rc, version);
 }
 
@@ -2343,7 +2343,10 @@ static void smc_find_rdma_v2_device_serv(struct smc_sock *new_smc,
 
 not_found:
 	ini->smcr_version &= ~SMC_V2;
+	if (ini->smcrv2.ib_dev_v2_ref)
+		smc_ibdev_init_put(ini->smcrv2.ib_dev_v2);
 	ini->smcrv2.ib_dev_v2 = NULL;
+	ini->smcrv2.ib_dev_v2_ref = false;
 	ini->check_smcrv2 = false;
 }
 
@@ -2587,7 +2590,7 @@ static void smc_listen_work(struct work_struct *work)
 	smc_listen_decline(new_smc, rc, ini ? ini->first_contact_local : 0,
 			   proposal_version);
 out_free:
-	kfree(ini);
+	smc_init_info_free(ini);
 	kfree(buf);
 }
 
diff --git a/net/smc/smc_core.c b/net/smc/smc_core.c
index cf6b620fef05..e59f327f294c 100644
--- a/net/smc/smc_core.c
+++ b/net/smc/smc_core.c
@@ -55,6 +55,17 @@ static void __smc_lgr_terminate(struct smc_link_group *lgr, bool soft);
 
 static void smc_link_down_work(struct work_struct *work);
 
+void smc_init_info_free(struct smc_init_info *ini)
+{
+	if (!ini)
+		return;
+	if (ini->ib_dev_ref)
+		smc_ibdev_init_put(ini->ib_dev);
+	if (ini->smcrv2.ib_dev_v2_ref)
+		smc_ibdev_init_put(ini->smcrv2.ib_dev_v2);
+	kfree(ini);
+}
+
 /* return head of link group list and its lock for a given link group */
 static inline struct list_head *smc_lgr_list_head(struct smc_link_group *lgr,
 						  spinlock_t **lgr_lock)
diff --git a/net/smc/smc_core.h b/net/smc/smc_core.h
index 5c18f08a4c8a..385f16cc5368 100644
--- a/net/smc/smc_core.h
+++ b/net/smc/smc_core.h
@@ -403,6 +403,7 @@ struct smc_init_info_smcrv2 {
 
 	/* Output fields when saddr is set */
 	struct smc_ib_device	*ib_dev_v2;
+	bool			ib_dev_v2_ref;
 	u8			ib_port_v2;
 	u8			ib_gid_v2[SMC_GID_SIZE];
 
@@ -438,6 +439,7 @@ struct smc_init_info {
 	u8			peer_mac[ETH_ALEN];
 	u8			peer_systemid[SMC_SYSTEMID_LEN];
 	struct smc_ib_device	*ib_dev;
+	bool			ib_dev_ref;
 	u8			ib_gid[SMC_GID_SIZE];
 	u8			ib_port;
 	u32			ib_clcqpn;
@@ -594,6 +596,7 @@ void smc_rtoken_set2(struct smc_link_group *lgr, int rtok_idx, int link_id,
 void smc_sndbuf_sync_sg_for_device(struct smc_connection *conn);
 void smc_rmb_sync_sg_for_cpu(struct smc_connection *conn);
 int smc_vlan_by_tcpsk(struct socket *clcsock, struct smc_init_info *ini);
+void smc_init_info_free(struct smc_init_info *ini);
 
 void smc_conn_free(struct smc_connection *conn);
 int smc_conn_create(struct smc_sock *smc, struct smc_init_info *ini);
diff --git a/net/smc/smc_ib.c b/net/smc/smc_ib.c
index 9bb495707445..01c9b5c62df7 100644
--- a/net/smc/smc_ib.c
+++ b/net/smc/smc_ib.c
@@ -950,6 +950,8 @@ static int smc_ib_add_dev(struct ib_device *ibdev)
 
 	smcibdev->ibdev = ibdev;
 	INIT_WORK(&smcibdev->port_event_work, smc_ib_port_event_work);
+	atomic_set(&smcibdev->init_cnt, 0);
+	init_waitqueue_head(&smcibdev->init_wait);
 	atomic_set(&smcibdev->lnk_cnt, 0);
 	init_waitqueue_head(&smcibdev->lnks_deleted);
 	mutex_init(&smcibdev->mutex);
@@ -1000,6 +1002,9 @@ static void smc_ib_remove_dev(struct ib_device *ibdev, void *client_data)
 	mutex_unlock(&smc_ib_devices.mutex);
 	pr_warn_ratelimited("smc: removing ib device %s\n",
 			    smcibdev->ibdev->name);
+	if (atomic_read(&smcibdev->init_cnt))
+		wait_event(smcibdev->init_wait,
+			   !atomic_read(&smcibdev->init_cnt));
 	smc_smcr_terminate_all(smcibdev);
 	smc_ib_cleanup_per_ibdev(smcibdev);
 	ib_unregister_event_handler(&smcibdev->event_handler);
diff --git a/net/smc/smc_ib.h b/net/smc/smc_ib.h
index ef8ac2b7546d..8e9fd5a200a2 100644
--- a/net/smc/smc_ib.h
+++ b/net/smc/smc_ib.h
@@ -51,6 +51,8 @@ struct smc_ib_device {				/* ib-device infos for smc */
 	struct work_struct	port_event_work;
 	unsigned long		port_event_mask;
 	DECLARE_BITMAP(ports_going_away, SMC_MAX_PORTS);
+	atomic_t		init_cnt;	/* number of device selections */
+	wait_queue_head_t	init_wait;	/* wait for selections to end */
 	atomic_t		lnk_cnt;	/* number of links on ibdev */
 	wait_queue_head_t	lnks_deleted;	/* wait 4 removal of all links*/
 	struct mutex		mutex;		/* protect dev setup+cleanup */
@@ -59,6 +61,17 @@ struct smc_ib_device {				/* ib-device infos for smc */
 	int			ndev_ifidx[SMC_MAX_PORTS]; /* ndev if indexes */
 };
 
+static inline void smc_ibdev_init_get(struct smc_ib_device *smcibdev)
+{
+	atomic_inc(&smcibdev->init_cnt);
+}
+
+static inline void smc_ibdev_init_put(struct smc_ib_device *smcibdev)
+{
+	if (atomic_dec_and_test(&smcibdev->init_cnt))
+		wake_up(&smcibdev->init_wait);
+}
+
 static inline __be32 smc_ib_gid_to_ipv4(u8 gid[SMC_GID_SIZE])
 {
 	struct in6_addr *addr6 = (struct in6_addr *)gid;
diff --git a/net/smc/smc_llc.c b/net/smc/smc_llc.c
index 954b2ff1815c..15ae19a9db07 100644
--- a/net/smc/smc_llc.c
+++ b/net/smc/smc_llc.c
@@ -1118,7 +1118,7 @@ int smc_llc_cli_add_link(struct smc_link *link, struct smc_llc_qentry *qentry)
 out_reject:
 	smc_llc_cli_add_link_reject(qentry);
 out:
-	kfree(ini);
+	smc_init_info_free(ini);
 	kfree(qentry);
 	return rc;
 }
@@ -1192,7 +1192,7 @@ static void smc_llc_cli_add_link_invite(struct smc_link *link,
 	smc_llc_send_add_link(link, ini->ib_dev->mac[ini->ib_port - 1],
 			      ini->ib_gid, NULL, SMC_LLC_REQ);
 out:
-	kfree(ini);
+	smc_init_info_free(ini);
 	kfree(qentry);
 }
 
@@ -1512,7 +1512,7 @@ int smc_llc_srv_add_link(struct smc_link *link,
 	rc = smc_llc_srv_conf_link(link, link_new, lgr_new_t);
 	if (rc)
 		goto out_err;
-	kfree(ini);
+	smc_init_info_free(ini);
 	return 0;
 out_err:
 	if (link_new) {
@@ -1520,7 +1520,7 @@ int smc_llc_srv_add_link(struct smc_link *link,
 		smcr_link_clear(link_new, false);
 	}
 out:
-	kfree(ini);
+	smc_init_info_free(ini);
 	if (send_req_add_link_resp)
 		smc_llc_send_req_add_link_response(req_qentry);
 	return rc;
diff --git a/net/smc/smc_pnet.c b/net/smc/smc_pnet.c
index 63e286e2dfaa..7e01e01a522b 100644
--- a/net/smc/smc_pnet.c
+++ b/net/smc/smc_pnet.c
@@ -980,14 +980,22 @@ static int smc_pnet_determine_gid(struct smc_ib_device *ibdev, int i,
 	if (!ini->check_smcrv2 &&
 	    !smc_ib_determine_gid(ibdev, i, ini->vlan_id, ini->ib_gid, NULL,
 				  NULL)) {
+		if (ini->ib_dev_ref)
+			smc_ibdev_init_put(ini->ib_dev);
+		smc_ibdev_init_get(ibdev);
 		ini->ib_dev = ibdev;
+		ini->ib_dev_ref = true;
 		ini->ib_port = i;
 		return 0;
 	}
 	if (ini->check_smcrv2 &&
 	    !smc_ib_determine_gid(ibdev, i, ini->vlan_id, ini->smcrv2.ib_gid_v2,
 				  NULL, &ini->smcrv2)) {
+		if (ini->smcrv2.ib_dev_v2_ref)
+			smc_ibdev_init_put(ini->smcrv2.ib_dev_v2);
+		smc_ibdev_init_get(ibdev);
 		ini->smcrv2.ib_dev_v2 = ibdev;
+		ini->smcrv2.ib_dev_v2_ref = true;
 		ini->smcrv2.ib_port_v2 = i;
 		return 0;
 	}
-- 
2.43.0