From: Joas Antonio dos Santos The bwi_active flag, callback pointer (cb), and callback context (cb_context) in st21nfca_se_info are accessed concurrently from multiple contexts without synchronization: - st21nfca_hci_se_io() sets them from the netlink path - st21nfca_se_wt_work() reads/clears them from the timeout workqueue - st21nfca_apdu_reader_event_received() reads/clears them from the HCI rx path Add a spinlock (se_lock) and take it around the accesses in these three paths. The callback pointer and context are copied under the lock and invoked after releasing it, since both the callback and nfc_hci_send_event() may sleep. Also move the bwi_active = true assignment before mod_timer() so that the flag is always set before the timeout handler could observe it. st21nfca_se_deinit() is not changed here. It still accesses bwi_active without the lock; the teardown ordering has problems of its own which need a separate fix. Found by LLM-assisted manual code review. Fixes: 2130fb97fecf ("NFC: st21nfca: Adding support for secure element") Cc: Simon Horman Signed-off-by: Joas Antonio dos Santos --- drivers/nfc/st21nfca/se.c | 24 ++++++++++++++++++++---- drivers/nfc/st21nfca/st21nfca.h | 2 ++ 2 files changed, 22 insertions(+), 4 deletions(-) diff --git a/drivers/nfc/st21nfca/se.c b/drivers/nfc/st21nfca/se.c index 7154bc1d644a..03a30f797ff8 100644 --- a/drivers/nfc/st21nfca/se.c +++ b/drivers/nfc/st21nfca/se.c @@ -227,11 +227,13 @@ int st21nfca_hci_se_io(struct nfc_hci_dev *hdev, u32 se_idx, switch (se_idx) { case ST21NFCA_ESE_HOST_ID: + spin_lock(&info->se_info.se_lock); info->se_info.cb = cb; info->se_info.cb_context = cb_context; + info->se_info.bwi_active = true; + spin_unlock(&info->se_info.se_lock); mod_timer(&info->se_info.bwi_timer, jiffies + msecs_to_jiffies(info->se_info.wt_timeout)); - info->se_info.bwi_active = true; return nfc_hci_send_event(hdev, ST21NFCA_APDU_READER_GATE, ST21NFCA_EVT_TRANSMIT_DATA, apdu, apdu_length); @@ -260,11 +262,17 @@ static void st21nfca_se_wt_work(struct work_struct *work) */ /* hardware reset managed through VCC_UICC_OUT power supply */ u8 param = 0x01; + se_io_cb_t cb; + void *cb_context; struct st21nfca_hci_info *info = container_of(work, struct st21nfca_hci_info, se_info.timeout_work); + spin_lock(&info->se_info.se_lock); info->se_info.bwi_active = false; + cb = info->se_info.cb; + cb_context = info->se_info.cb_context; + spin_unlock(&info->se_info.se_lock); if (!info->se_info.xch_error) { info->se_info.xch_error = true; @@ -275,7 +283,7 @@ static void st21nfca_se_wt_work(struct work_struct *work) nfc_hci_send_event(info->hdev, ST21NFCA_DEVICE_MGNT_GATE, ST21NFCA_EVT_SE_HARD_RESET, ¶m, 1); } - info->se_info.cb(info->se_info.cb_context, NULL, 0, -ETIME); + cb(cb_context, NULL, 0, -ETIME); } static void st21nfca_se_wt_timeout(struct timer_list *t) @@ -375,6 +383,8 @@ int st21nfca_apdu_reader_event_received(struct nfc_hci_dev *hdev, u8 event, struct sk_buff *skb) { int r = 0; + se_io_cb_t cb; + void *cb_context; struct st21nfca_hci_info *info = nfc_hci_get_clientdata(hdev); pr_debug("apdu reader gate event: %x\n", event); @@ -383,14 +393,19 @@ int st21nfca_apdu_reader_event_received(struct nfc_hci_dev *hdev, case ST21NFCA_EVT_TRANSMIT_DATA: timer_delete_sync(&info->se_info.bwi_timer); cancel_work_sync(&info->se_info.timeout_work); + + spin_lock(&info->se_info.se_lock); info->se_info.bwi_active = false; + cb = info->se_info.cb; + cb_context = info->se_info.cb_context; + spin_unlock(&info->se_info.se_lock); + r = nfc_hci_send_event(hdev, ST21NFCA_DEVICE_MGNT_GATE, ST21NFCA_EVT_SE_END_OF_APDU_TRANSFER, NULL, 0); if (r < 0) goto exit; - info->se_info.cb(info->se_info.cb_context, - skb->data, skb->len, 0); + cb(cb_context, skb->data, skb->len, 0); break; case ST21NFCA_EVT_WTX_REQUEST: mod_timer(&info->se_info.bwi_timer, jiffies + @@ -416,6 +431,7 @@ void st21nfca_se_init(struct nfc_hci_dev *hdev) /* initialize timers */ timer_setup(&info->se_info.bwi_timer, st21nfca_se_wt_timeout, 0); info->se_info.bwi_active = false; + spin_lock_init(&info->se_info.se_lock); timer_setup(&info->se_info.se_active_timer, st21nfca_se_activation_timeout, 0); diff --git a/drivers/nfc/st21nfca/st21nfca.h b/drivers/nfc/st21nfca/st21nfca.h index ae6771cc9894..03e41fadb5ed 100644 --- a/drivers/nfc/st21nfca/st21nfca.h +++ b/drivers/nfc/st21nfca/st21nfca.h @@ -6,6 +6,7 @@ #ifndef __LOCAL_ST21NFCA_H_ #define __LOCAL_ST21NFCA_H_ +#include #include #include #include @@ -131,6 +132,7 @@ struct st21nfca_se_info { struct timer_list bwi_timer; int wt_timeout; /* in msecs */ bool bwi_active; + spinlock_t se_lock; /* protects bwi_active, cb and cb_context */ struct timer_list se_active_timer; bool se_active; -- 2.43.0