7.3-stable review patch. If anyone has any objections, please let me know. ------------------ From: Sean Young commit c6d68866ad8aa1e957a3e5a6ede971148f1e3d59 upstream. The ir_raw_event_unregister() function takes the ir_raw_handler_lock first and then the rc_dev->lock. Other functions like change_protocol do this in the reverse order. This means that an ir decoder module unload and writing to the protocol sysfs file can cause a deadlock. ir_raw_handler_lock protects the client list, amongst other things. Split this out into a separate lock so we can ensure the locking order is always the same. Fixes: 93cffffc18f6 ("[media] media: rc: fix decoder module unloading") Signed-off-by: Sean Young Cc: stable@vger.kernel.org Acked-by: Hans Verkuil Signed-off-by: Greg Kroah-Hartman --- drivers/media/rc/rc-ir-raw.c | 39 ++++++++++++++++++++++++--------------- 1 file changed, 24 insertions(+), 15 deletions(-) --- a/drivers/media/rc/rc-ir-raw.c +++ b/drivers/media/rc/rc-ir-raw.c @@ -10,7 +10,8 @@ #include #include "rc-core-priv.h" -/* Used to keep track of IR raw clients, protected by ir_raw_handler_lock */ +/* Used to keep track of IR raw clients, protected by ir_raw_client_lock */ +static DEFINE_MUTEX(ir_raw_client_lock); static LIST_HEAD(ir_raw_client_list); /* Used to handle IR raw handler extensions */ @@ -273,13 +274,6 @@ static int change_protocol(struct rc_dev return 0; } -static void ir_raw_disable_protocols(struct rc_dev *dev, u64 protocols) -{ - mutex_lock(&dev->lock); - dev->enabled_protocols &= ~protocols; - mutex_unlock(&dev->lock); -} - /** * ir_raw_gen_manchester() - Encode data with Manchester (bi-phase) modulation. * @ev: Pointer to pointer to next free event. *@ev is incremented for @@ -615,15 +609,18 @@ int ir_raw_event_register(struct rc_dev { struct task_struct *thread; + /* Holding dev->lock could result in a dead-lock */ + lockdep_assert_not_held(&dev->lock); + thread = kthread_run(ir_raw_event_thread, dev->raw, "rc%u", dev->minor); if (IS_ERR(thread)) return PTR_ERR(thread); dev->raw->thread = thread; - mutex_lock(&ir_raw_handler_lock); + mutex_lock(&ir_raw_client_lock); list_add_tail(&dev->raw->list, &ir_raw_client_list); - mutex_unlock(&ir_raw_handler_lock); + mutex_unlock(&ir_raw_client_lock); return 0; } @@ -656,16 +653,19 @@ void ir_raw_event_unregister(struct rc_d kthread_stop(dev->raw->thread); timer_delete_sync(&dev->raw->edge_handle); - mutex_lock(&ir_raw_handler_lock); + mutex_lock(&ir_raw_client_lock); list_del(&dev->raw->list); + + mutex_lock(&ir_raw_handler_lock); list_for_each_entry(handler, &ir_raw_handler_list, list) if (handler->raw_unregister && (handler->protocols & dev->enabled_protocols)) handler->raw_unregister(dev); lirc_bpf_free(dev); - mutex_unlock(&ir_raw_handler_lock); + + mutex_unlock(&ir_raw_client_lock); } /* @@ -688,15 +688,24 @@ void ir_raw_handler_unregister(struct ir struct ir_raw_event_ctrl *raw; u64 protocols = ir_raw_handler->protocols; + mutex_lock(&ir_raw_client_lock); + mutex_lock(&ir_raw_handler_lock); list_del(&ir_raw_handler->list); + atomic64_andnot(protocols, &available_protocols); + mutex_unlock(&ir_raw_handler_lock); + list_for_each_entry(raw, &ir_raw_client_list, list) { + mutex_lock(&raw->dev->lock); + mutex_lock(&ir_raw_handler_lock); if (ir_raw_handler->raw_unregister && (raw->dev->enabled_protocols & protocols)) ir_raw_handler->raw_unregister(raw->dev); - ir_raw_disable_protocols(raw->dev, protocols); + raw->dev->enabled_protocols &= ~protocols; + mutex_unlock(&ir_raw_handler_lock); + mutex_unlock(&raw->dev->lock); } - atomic64_andnot(protocols, &available_protocols); - mutex_unlock(&ir_raw_handler_lock); + + mutex_unlock(&ir_raw_client_lock); } EXPORT_SYMBOL(ir_raw_handler_unregister);