From: Hans Verkuil <hverkuil-cisco@xxxxxxxxx>
commit da53c36ddd3f118a525a04faa8c47ca471e6c467 upstream.
A potential deadlock was found by Zheng Zhang with a local syzkaller
instance.
The problem is that when a non-blocking CEC transmit is canceled by calling
cec_data_cancel, that in turn can call the high-level received() driver
callback, which can call cec_transmit_msg() to transmit a new message.
The cec_data_cancel() function is called with the adap->lock mutex held,
and cec_transmit_msg() tries to take that same lock.
The root cause is that the received() callback can either be used to pass
on a received message (and then adap->lock is not held), or to report a
canceled transmit (and then adap->lock is held).
This is confusing, so create a new low-level adap_nb_transmit_canceled
callback that reports back that a non-blocking transmit was canceled.
And the received() callback is only called when a message is received,
as was the case before commit f9d0ecbf56f4 ("media: cec: correctly pass
on reply results") complicated matters.
Reported-by: Zheng Zhang <zheng.zhang@xxxxxxxxxxxxx>
Signed-off-by: Hans Verkuil <hverkuil-cisco@xxxxxxxxx>
Fixes: f9d0ecbf56f4 ("media: cec: correctly pass on reply results")
Signed-off-by: Mauro Carvalho Chehab <mchehab@xxxxxxxxxx>
Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
---
drivers/media/cec/core/cec-adap.c | 4 ++--
include/media/cec.h | 6 ++++--
2 files changed, 6 insertions(+), 4 deletions(-)
--- a/drivers/media/cec/core/cec-adap.c
+++ b/drivers/media/cec/core/cec-adap.c
@@ -397,8 +397,8 @@ static void cec_data_cancel(struct cec_d
cec_queue_msg_monitor(adap, &data->msg, 1);
if (!data->blocking && data->msg.sequence)
- /* Allow drivers to process the message first */
- call_op(adap, received, &data->msg);
+ /* Allow drivers to react to a canceled transmit */
+ call_void_op(adap, adap_nb_transmit_canceled, &data->msg);
cec_data_completed(data);
}
--- a/include/media/cec.h
+++ b/include/media/cec.h
@@ -120,14 +120,16 @@ struct cec_adap_ops {
int (*adap_log_addr)(struct cec_adapter *adap, u8 logical_addr);
int (*adap_transmit)(struct cec_adapter *adap, u8 attempts,
u32 signal_free_time, struct cec_msg *msg);
+ void (*adap_nb_transmit_canceled)(struct cec_adapter *adap,
+ const struct cec_msg *msg);
void (*adap_status)(struct cec_adapter *adap, struct seq_file *file);
void (*adap_free)(struct cec_adapter *adap);
- /* Error injection callbacks */
+ /* Error injection callbacks, called without adap->lock held */
int (*error_inj_show)(struct cec_adapter *adap, struct seq_file *sf);
bool (*error_inj_parse_line)(struct cec_adapter *adap, char *line);
- /* High-level CEC message callback */
+ /* High-level CEC message callback, called without adap->lock held */
int (*received)(struct cec_adapter *adap, struct cec_msg *msg);
};
Patches currently in stable-queue which might be from hverkuil-cisco@xxxxxxxxx are
queue-5.10/media-stk1160-fix-bounds-checking-in-stk1160_copy_vi.patch
queue-5.10/media-cec-use-call_op-and-check-for-unregistered.patch
queue-5.10/media-cec-correctly-pass-on-reply-results.patch
queue-5.10/media-cec-call-enable_adap-on-s_log_addrs.patch
queue-5.10/media-cec-core-avoid-confusing-transmit-timed-out-me.patch
queue-5.10/media-cec-fix-a-deadlock-situation.patch
queue-5.10/media-cec-core-avoid-recursive-cec_claim_log_addrs.patch
queue-5.10/media-core-headers-fix-kernel-doc-warnings.patch
queue-5.10/media-ngene-add-dvb_ca_en50221_init-return-value-che.patch
queue-5.10/media-cec-cec-adap-always-cancel-work-in-cec_transmi.patch
queue-5.10/media-cec-core-add-adap_nb_transmit_canceled-callback.patch
queue-5.10/media-cec-cec-api-add-locking-in-cec_release.patch
queue-5.10/media-cec-abort-if-the-current-transmit-was-canceled.patch
queue-5.10/media-cec-adap.c-drop-activate_cnt-use-state-info-in.patch
queue-5.10/media-radio-shark2-avoid-led_names-truncations.patch
