Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   korantwork@gmail.com
To:     hverkuil-cisco@xs4all.nl, mchehab@kernel.org
Cc:     linux-kernel@vger.kernel.org, linux-media@vger.kernel.org,
        Xinghui Li <korantli@tencent.com>, loydlv <loydlv@tencent.com>
Subject: [PATCH] media:cec:fix double free and uaf issue when cancel data during noblocking
Date:   Wed, 11 Jan 2023 20:37:12 +0800
Message-Id: <20230111123712.160882-1-korantwork@gmail.com>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Precedence: bulk

From: Xinghui Li <korantli@tencent.com>

data could be free when it is not completed during transmit if
the opt is nonblocking.In this case,the regular free could lead
to double-free.So, add the return value '-EPERM' to mark the
above case.

Reported-by: loydlv <loydlv@tencent.com>
Signed-off-by: Xinghui Li <korantli@tencent.com>
---
 drivers/media/cec/core/cec-adap.c | 27 ++++++++++++++++-----------
 1 file changed, 16 insertions(+), 11 deletions(-)

diff --git a/drivers/media/cec/core/cec-adap.c b/drivers/media/cec/core/cec-adap.c
index 4f5ab3cae8a7..c2ba8d1173c1 100644
--- a/drivers/media/cec/core/cec-adap.c
+++ b/drivers/media/cec/core/cec-adap.c
@@ -311,7 +311,7 @@ static void cec_post_state_event(struct cec_adapter *adap)
  *
  * This function is called with adap->lock held.
  */
-static void cec_data_completed(struct cec_data *data)
+static int cec_data_completed(struct cec_data *data)
 {
 	/*
 	 * Delete this transmit from the filehandle's xfer_list since
@@ -339,7 +339,9 @@ static void cec_data_completed(struct cec_data *data)
 		if (data->fh)
 			cec_queue_msg_fh(data->fh, &data->msg);
 		kfree(data);
+		return -EPERM;
 	}
+	return 0;
 }
 
 /*
@@ -349,7 +351,7 @@ static void cec_data_completed(struct cec_data *data)
  *
  * This function is called with adap->lock held.
  */
-static void cec_data_cancel(struct cec_data *data, u8 tx_status, u8 rx_status)
+static int cec_data_cancel(struct cec_data *data, u8 tx_status, u8 rx_status)
 {
 	struct cec_adapter *adap = data->adap;
 
@@ -388,7 +390,7 @@ static void cec_data_cancel(struct cec_data *data, u8 tx_status, u8 rx_status)
 		/* Allow drivers to process the message first */
 		call_op(adap, received, &data->msg);
 
-	cec_data_completed(data);
+	return cec_data_completed(data);
 }
 
 /*
@@ -744,6 +746,7 @@ int cec_transmit_msg_fh(struct cec_adapter *adap, struct cec_msg *msg,
 {
 	struct cec_data *data;
 	bool is_raw = msg_is_raw(msg);
+	int ret = 0;
 
 	if (adap->devnode.unregistered)
 		return -ENODEV;
@@ -916,18 +919,20 @@ int cec_transmit_msg_fh(struct cec_adapter *adap, struct cec_msg *msg,
 	/* Cancel the transmit if it was interrupted */
 	if (!data->completed) {
 		if (data->msg.tx_status & CEC_TX_STATUS_OK)
-			cec_data_cancel(data, CEC_TX_STATUS_OK, CEC_RX_STATUS_ABORTED);
+			ret = cec_data_cancel(data, CEC_TX_STATUS_OK, CEC_RX_STATUS_ABORTED);
 		else
-			cec_data_cancel(data, CEC_TX_STATUS_ABORTED, 0);
+			ret = cec_data_cancel(data, CEC_TX_STATUS_ABORTED, 0);
 	}
 
 	/* The transmit completed (possibly with an error) */
-	*msg = data->msg;
-	if (WARN_ON(!list_empty(&data->list)))
-		list_del(&data->list);
-	if (WARN_ON(!list_empty(&data->xfer_list)))
-		list_del(&data->xfer_list);
-	kfree(data);
+	if (!ret) {
+		*msg = data->msg;
+		if (WARN_ON(!list_empty(&data->list)))
+			list_del(&data->list);
+		if (WARN_ON(!list_empty(&data->xfer_list)))
+			list_del(&data->xfer_list);
+		kfree(data);
+	}
 	return 0;
 }
 
-- 
2.34.1