Messages in this thread Patch in this message | | | Date | Wed, 18 May 2022 19:05:59 +0800 (GMT+08:00) | From | duoming@zju ... | Subject | Re: [PATCH net v2] NFC: hci: fix sleep in atomic context bugs in nfc_hci_hcp_message_tx |
| |
Hello,
Date: Wed, 18 May 2022 11:39:27 +0200 Krzysztof wrote:
> >> There is. > >> > >> nfc_hci_failure -> spin lock -> nfc_driver_failure -> nfc_targets_found > >> -> device_lock > >> > >> I found it just by a very quick look, so I suspect there are several > >> other places, not really checked. > > > > I agree with you, the spin_lock is not a good solution to this problem. There is another solution: > > > > We could put the nfc_hci_send_event() of st21nfca_se_wt_timeout() in a work item, then, using > > schedule_work() in st21nfca_se_wt_timeout() to execute the work item. The schedule_work() will > > wake up another kernel thread which is in process context to execute the bottom half of the interrupt, > > so it allows sleep. > > > > The following is the details. > > Yes, this seems good solution. You might also need to add > cancel_work_sync to all places removing the timer.
Thanks for your approval.
There are two del_timer_sync() functions related with bwi_timer timer. I neglect one site in st21nfca_apdu_reader_event_received(), I add cancel_work_sync() in it this time.
diff --git a/drivers/nfc/st21nfca/se.c b/drivers/nfc/st21nfca/se.c index c922f10d0d7..7e213f8ddc9 100644 --- a/drivers/nfc/st21nfca/se.c +++ b/drivers/nfc/st21nfca/se.c @@ -241,7 +241,7 @@ int st21nfca_hci_se_io(struct nfc_hci_dev *hdev, u32 se_idx, } EXPORT_SYMBOL(st21nfca_hci_se_io);
-static void st21nfca_se_wt_timeout(struct timer_list *t) +static void st21nfca_se_wt_work(struct work_struct *work) { /* * No answer from the secure element @@ -254,8 +254,9 @@ static void st21nfca_se_wt_timeout(struct timer_list *t) */ /* hardware reset managed through VCC_UICC_OUT power supply */ u8 param = 0x01; - struct st21nfca_hci_info *info = from_timer(info, t, - se_info.bwi_timer); + struct st21nfca_hci_info *info = container_of(work, + struct st21nfca_hci_info, + se_info.timeout_work);
info->se_info.bwi_active = false; @@ -271,6 +272,13 @@ static void st21nfca_se_wt_timeout(struct timer_list *t) info->se_info.cb(info->se_info.cb_context, NULL, 0, -ETIME); }
+static void st21nfca_se_wt_timeout(struct timer_list *t) +{ + struct st21nfca_hci_info *info = from_timer(info, t, se_info.bwi_timer); + + schedule_work(&info->se_info.timeout_work); +} + static void st21nfca_se_activation_timeout(struct timer_list *t) { struct st21nfca_hci_info *info = from_timer(info, t, @@ -360,6 +368,7 @@ int st21nfca_apdu_reader_event_received(struct nfc_hci_dev *hdev, switch (event) { case ST21NFCA_EVT_TRANSMIT_DATA: del_timer_sync(&info->se_info.bwi_timer); + cancel_work_sync(&info->se_info.timeout_work); info->se_info.bwi_active = false; r = nfc_hci_send_event(hdev, ST21NFCA_DEVICE_MGNT_GATE, ST21NFCA_EVT_SE_END_OF_APDU_TRANSFER, NULL, 0); @@ -389,6 +398,7 @@ void st21nfca_se_init(struct nfc_hci_dev *hdev) struct st21nfca_hci_info *info = nfc_hci_get_clientdata(hdev);
init_completion(&info->se_info.req_completion); + INIT_WORK(&info->se_info.timeout_work, st21nfca_se_wt_work); /* initialize timers */ timer_setup(&info->se_info.bwi_timer, st21nfca_se_wt_timeout, 0); info->se_info.bwi_active = false; @@ -416,6 +426,7 @@ void st21nfca_se_deinit(struct nfc_hci_dev *hdev) if (info->se_info.se_active) del_timer_sync(&info->se_info.se_active_timer);
+ cancel_work_sync(&info->se_info.timeout_work); info->se_info.bwi_active = false; info->se_info.se_active = false; } diff --git a/drivers/nfc/st21nfca/st21nfca.h b/drivers/nfc/st21nfca/st21nfca.h index cb6ad916be9..ae6771cc989 100644 --- a/drivers/nfc/st21nfca/st21nfca.h +++ b/drivers/nfc/st21nfca/st21nfca.h @@ -141,6 +141,7 @@ struct st21nfca_se_info {
se_io_cb_t cb; void *cb_context; + struct work_struct timeout_work; };
struct st21nfca_hci_info { If you think this solution is ok, I will send "PATCH v3".
Best regards, Duoming Zhou
| |