media: cec: check 'transmit_in_progress', not 'transmitting'
authorHans Verkuil <hverkuil-cisco@xs4all.nl>
Wed, 11 Dec 2019 11:47:57 +0000 (12:47 +0100)
committerGreg Kroah-Hartman <gregkh@linuxfoundation.org>
Thu, 9 Jan 2020 09:19:56 +0000 (10:19 +0100)
commit ac479b51f3f4aaa852b5d3f00ecfb9290230cf64 upstream.

Currently wait_event_interruptible_timeout is called in cec_thread_func()
when adap->transmitting is set. But if the adapter is unconfigured
while transmitting, then adap->transmitting is set to NULL. But the
hardware is still actually transmitting the message, and that's
indicated by adap->transmit_in_progress and we should wait until that
is finished or times out before transmitting new messages.

As the original commit says: adap->transmitting is the userspace view,
adap->transmit_in_progress reflects the hardware state.

However, if adap->transmitting is NULL and adap->transmit_in_progress
is true, then wait_event_interruptible is called (no timeout), which
can get stuck indefinitely if the CEC driver is flaky and never marks
the transmit-in-progress as 'done'.

So test against transmit_in_progress when deciding whether to use
the timeout variant or not, instead of testing against adap->transmitting.

Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Fixes: 32804fcb612b ("media: cec: keep track of outstanding transmits")
Cc: <stable@vger.kernel.org> # for v4.19 and up
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
drivers/media/cec/cec-adap.c

index fb88972..b14c09c 100644 (file)
@@ -463,7 +463,7 @@ int cec_thread_func(void *_adap)
                bool timeout = false;
                u8 attempts;
 
-               if (adap->transmitting) {
+               if (adap->transmit_in_progress) {
                        int err;
 
                        /*
@@ -498,7 +498,7 @@ int cec_thread_func(void *_adap)
                        goto unlock;
                }
 
-               if (adap->transmitting && timeout) {
+               if (adap->transmit_in_progress && timeout) {
                        /*
                         * If we timeout, then log that. Normally this does
                         * not happen and it is an indication of a faulty CEC
@@ -507,14 +507,18 @@ int cec_thread_func(void *_adap)
                         * so much traffic on the bus that the adapter was
                         * unable to transmit for CEC_XFER_TIMEOUT_MS (2.1s).
                         */
-                       pr_warn("cec-%s: message %*ph timed out\n", adap->name,
-                               adap->transmitting->msg.len,
-                               adap->transmitting->msg.msg);
+                       if (adap->transmitting) {
+                               pr_warn("cec-%s: message %*ph timed out\n", adap->name,
+                                       adap->transmitting->msg.len,
+                                       adap->transmitting->msg.msg);
+                               /* Just give up on this. */
+                               cec_data_cancel(adap->transmitting,
+                                               CEC_TX_STATUS_TIMEOUT);
+                       } else {
+                               pr_warn("cec-%s: transmit timed out\n", adap->name);
+                       }
                        adap->transmit_in_progress = false;
                        adap->tx_timeouts++;
-                       /* Just give up on this. */
-                       cec_data_cancel(adap->transmitting,
-                                       CEC_TX_STATUS_TIMEOUT);
                        goto unlock;
                }