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`)

["Yang, Chenyuan" <cy54@xxxxxxxxxxxx>]

Hi Hans,

Thank you for your continued efforts in investigating this bug and implementing the new patch!

Regarding the two warnings, they have been addressed by this new patch and are no longer reproducible. Additionally, I conducted a 48-hour fuzzing test on the CEC driver, which has successfully eliminated the previous hanging issue.

One thing to note that the system will now log timeout events:
```
[ 2281.265385][ T2034] cec-vivid-001-vid-out0: transmit timed out
[ 2282.994510][ T2017] cec-vivid-000-vid-cap0: transmit timed out
[ 2283.063484][ T2050] cec-vivid-002-vid-out0: transmit timed out
[ 2283.073468][ T2065] cec-vivid-003-vid-cap0: transmit timed out
[ 2283.373518][ T2033] cec-vivid-001-vid-cap0: transmit timed out
[ 2285.113544][ T2018] cec-vivid-000-vid-out0: transmit timed out
[ 2285.193502][ T2050] cec-vivid-002-vid-out0: transmit timed out
[ 2285.193570][ T2065] cec-vivid-003-vid-cap0: transmit timed out
[ 2285.513570][ T2033] cec-vivid-001-vid-cap0: transmit timed out
```

Best,
Chenyuan

From: Hans Verkuil <hverkuil-cisco@xxxxxxxxx>
Date: Friday, February 23, 2024 at 8:44 AM
To: Yang, Chenyuan <cy54@xxxxxxxxxxxx>, linux-media@xxxxxxxxxxxxxxx <linux-media@xxxxxxxxxxxxxxx>, linux-kernel@xxxxxxxxxxxxxxx <linux-kernel@xxxxxxxxxxxxxxx>
Cc: jani.nikula@xxxxxxxxx <jani.nikula@xxxxxxxxx>, syzkaller@xxxxxxxxxxxxxxxx <syzkaller@xxxxxxxxxxxxxxxx>, mchehab@xxxxxxxxxx <mchehab@xxxxxxxxxx>, Zhao, Zijie <zijie4@xxxxxxxxxxxx>, Zhang, Lingming <lingming@xxxxxxxxxxxx>
Subject: 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`)
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;