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/ioctl 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) to serialize access to these fields. 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. 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..abcef1ee48f2 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; struct timer_list se_active_timer; bool se_active; -- 2.43.0