nci_tx_work() looks up conn_info from conn_info_list and keeps using
that pointer while sending queued data. nci_core_conn_close_rsp_packet()
runs on the separate rx_wq and can remove and free the same conn_info,
so the tx worker can dereference freed memory.

Race:

  CPU0 (tx_wq)                         CPU1 (rx_wq)
  ============                         ============
  nci_tx_work()
    conn_info = nci_get_conn_info_by_conn_id()
                                       nci_core_conn_close_rsp_packet()
                                         list_del(&conn_info->list)
                                         devm_kfree(conn_info)
    atomic_read(&conn_info->credits_cnt)
    // UAF: conn_info was freed by CPU1

Protect conn_info_list with a dedicated conn_lock while nci_tx_work()
uses conn_info and while nci_core_conn_close_rsp_packet() removes and
frees it. This makes close wait for any in-flight tx worker before
freeing the entry, and makes later tx workers observe that the entry is
already gone.

Signed-off-by: Sanghyun Park <sanghyun.park.cnu@gmail.com>
---
v2:
  - Replace flush-only fix with conn_lock protection
v1: https://patchwork.kernel.org/project/netdevbpf/patch/CAOrxSK5UmFFfzdRG+P89+E+Rvg_1DmOvTs+M7353Q8=hkPXmSg@mail.gmail.com/

 include/net/nfc/nci_core.h | 1 +
 net/nfc/nci/core.c         | 9 +++++++--
 net/nfc/nci/rsp.c          | 2 ++
 3 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/include/net/nfc/nci_core.h b/include/net/nfc/nci_core.h
index e066bdbc807..22b245bc17c 100644
--- a/include/net/nfc/nci_core.h
+++ b/include/net/nfc/nci_core.h
@@ -227,6 +227,7 @@ struct nci_dev {
 	struct sk_buff_head	tx_q;
 
 	struct mutex		req_lock;
+	struct mutex		conn_lock;
 	struct completion	req_completion;
 	__u32			req_status;
 	__u32			req_result;
diff --git a/net/nfc/nci/core.c b/net/nfc/nci/core.c
index 058d4eb530f..a24b9703115 100644
--- a/net/nfc/nci/core.c
+++ b/net/nfc/nci/core.c
@@ -1277,6 +1277,7 @@ int nci_register_device(struct nci_dev *ndev)
 	timer_setup(&ndev->data_timer, nci_data_timer, 0);
 
 	mutex_init(&ndev->req_lock);
+	mutex_init(&ndev->conn_lock);
 	INIT_LIST_HEAD(&ndev->conn_info_list);
 
 	rc = nfc_register_device(ndev->nfc_dev);
@@ -1512,9 +1513,10 @@ static void nci_tx_work(struct work_struct *work)
 	struct nci_conn_info *conn_info;
 	struct sk_buff *skb;
 
+	mutex_lock(&ndev->conn_lock);
 	conn_info = nci_get_conn_info_by_conn_id(ndev, ndev->cur_conn_id);
 	if (!conn_info)
-		return;
+		goto unlock;
 
 	pr_debug("credits_cnt %d\n", atomic_read(&conn_info->credits_cnt));
 
@@ -1522,7 +1524,7 @@ static void nci_tx_work(struct work_struct *work)
 	while (atomic_read(&conn_info->credits_cnt)) {
 		skb = skb_dequeue(&ndev->tx_q);
 		if (!skb)
-			return;
+			goto unlock;
 		kcov_remote_start_common(skb_get_kcov_handle(skb));
 
 		/* Check if data flow control is used */
@@ -1541,6 +1543,9 @@ static void nci_tx_work(struct work_struct *work)
 			  jiffies + msecs_to_jiffies(NCI_DATA_TIMEOUT));
 		kcov_remote_stop();
 	}
+
+unlock:
+	mutex_unlock(&ndev->conn_lock);
 }
 
 /* ----- NCI RX worker thread (data & control) ----- */
diff --git a/net/nfc/nci/rsp.c b/net/nfc/nci/rsp.c
index b911ab78bed..dfebf1ed945 100644
--- a/net/nfc/nci/rsp.c
+++ b/net/nfc/nci/rsp.c
@@ -330,6 +330,7 @@ static void nci_core_conn_close_rsp_packet(struct nci_dev *ndev,
 
 	pr_debug("status 0x%x\n", status);
 	if (status == NCI_STATUS_OK) {
+		mutex_lock(&ndev->conn_lock);
 		conn_info = nci_get_conn_info_by_conn_id(ndev,
 							 ndev->cur_conn_id);
 		if (conn_info) {
@@ -338,6 +339,7 @@ static void nci_core_conn_close_rsp_packet(struct nci_dev *ndev,
 				ndev->rf_conn_info = NULL;
 			devm_kfree(&ndev->nfc_dev->dev, conn_info);
 		}
+		mutex_unlock(&ndev->conn_lock);
 	}
 	nci_req_complete(ndev, status);
 }
-- 
2.48.1