Re: [Linux Kernel Bugs] KASAN: slab-use-after-free Read in cec_queue_msg_fh and 4 other crashes in the cec device (`cec_ioctl`)

From: Hans Verkuil
Date: Fri Feb 23 2024 - 09:45:49 EST


Hi Chenyuan,

Here is another patch for you to try. I think it is good for blocking CEC_ADAP_S_LOG_ADDRS
ioctl calls, but if the filehandle is in non-blocking mode, I'm still not certain it
is correct. But one issue at a time :-)

Regards,

Hans

diff --git a/drivers/media/cec/core/cec-adap.c b/drivers/media/cec/core/cec-adap.c
index 559a172ebc6c..a493cbce2456 100644
--- a/drivers/media/cec/core/cec-adap.c
+++ b/drivers/media/cec/core/cec-adap.c
@@ -936,8 +936,7 @@ int cec_transmit_msg_fh(struct cec_adapter *adap, struct cec_msg *msg,
*/
mutex_unlock(&adap->lock);
wait_for_completion_killable(&data->c);
- if (!data->completed)
- cancel_delayed_work_sync(&data->work);
+ cancel_delayed_work_sync(&data->work);
mutex_lock(&adap->lock);

/* Cancel the transmit if it was interrupted */
@@ -1575,9 +1574,12 @@ static int cec_config_thread_func(void *arg)
*/
static void cec_claim_log_addrs(struct cec_adapter *adap, bool block)
{
- if (WARN_ON(adap->is_configuring || adap->is_configured))
+ if (WARN_ON(adap->is_claiming_log_addrs ||
+ adap->is_configuring || adap->is_configured))
return;

+ adap->is_claiming_log_addrs = true;
+
init_completion(&adap->config_completion);

/* Ready to kick off the thread */
@@ -1592,6 +1594,7 @@ static void cec_claim_log_addrs(struct cec_adapter *adap, bool block)
wait_for_completion(&adap->config_completion);
mutex_lock(&adap->lock);
}
+ adap->is_claiming_log_addrs = false;
}

/*
diff --git a/drivers/media/cec/core/cec-api.c b/drivers/media/cec/core/cec-api.c
index 67dc79ef1705..3ef915344304 100644
--- a/drivers/media/cec/core/cec-api.c
+++ b/drivers/media/cec/core/cec-api.c
@@ -178,7 +178,7 @@ static long cec_adap_s_log_addrs(struct cec_adapter *adap, struct cec_fh *fh,
CEC_LOG_ADDRS_FL_ALLOW_RC_PASSTHRU |
CEC_LOG_ADDRS_FL_CDC_ONLY;
mutex_lock(&adap->lock);
- if (!adap->is_configuring &&
+ if (!adap->is_claiming_log_addrs && !adap->is_configuring &&
(!log_addrs.num_log_addrs || !adap->is_configured) &&
!cec_is_busy(adap, fh)) {
err = __cec_s_log_addrs(adap, &log_addrs, block);
@@ -664,6 +664,8 @@ static int cec_release(struct inode *inode, struct file *filp)
list_del_init(&data->xfer_list);
}
mutex_unlock(&adap->lock);
+
+ mutex_lock(&fh->lock);
while (!list_empty(&fh->msgs)) {
struct cec_msg_entry *entry =
list_first_entry(&fh->msgs, struct cec_msg_entry, list);
@@ -681,6 +683,7 @@ static int cec_release(struct inode *inode, struct file *filp)
kfree(entry);
}
}
+ mutex_unlock(&fh->lock);
kfree(fh);

cec_put_device(devnode);
diff --git a/include/media/cec.h b/include/media/cec.h
index 10c9cf6058b7..cc3fcd0496c3 100644
--- a/include/media/cec.h
+++ b/include/media/cec.h
@@ -258,6 +258,7 @@ struct cec_adapter {
u16 phys_addr;
bool needs_hpd;
bool is_enabled;
+ bool is_claiming_log_addrs;
bool is_configuring;
bool must_reconfigure;
bool is_configured;