Thread (3 messages) read the whole thread 3 messages, 1 author, 1d ago
WARM1d

[PATCH net v2 2/2] nfc: st-nci: drain SE timers before NCI teardown

From: Fan Wu <hidden>
Date: 2026-07-31 02:44:55
Also in: lkml, oe-linux-nfc, stable
Subsystem: nfc subsystem, the rest · Maintainers: David Heidelberg, Linus Torvalds

st_nci_se_deinit() drains the two se_info timers (bwi_timer and
se_active_timer), but it has had no caller since the secure-element
support was added.  st_nci_remove() tears the device down with
ndlc_close() / nci_unregister_device() / nci_free_device() and never
reaches the helper.  A timer armed during operation therefore survives
st_nci_remove() and the devm release of struct st_nci_info: the bwi
callback st_nci_se_wt_timeout() reaches into the NCI core, and both
callbacks dereference info->se_info.

Register the helper as the NCI pre_unregister callback added by the
previous patch, so the NCI core invokes it in nci_unregister_device()
after the NFC admission barrier (new SE operations are already rejected)
and before nci_close_device() / destroy_workqueue() (the NCI core and its
workqueues still exist).

Shut down both timers with timer_shutdown_sync().  Complete req_completion
after the se_active_timer is shut down so an in-flight SE enable/disable is
woken rather than left waiting: nfc_enable_se()/nfc_disable_se() hold the
NFC device lock across st_nci_control_se().  For an outstanding APDU,
invoke its completion callback with -ENODEV; the existing netlink
completion path then frees se_io_ctx.
bwi_active is an xchg() ownership token shared by the response, the
timeout and teardown paths, so the callback is delivered exactly once.

This issue was found by an in-house static analysis tool.

Fixes: 8ae01f796771 ("NFC: st21nfcb: Add support for secure element")
Cc: stable@vger.kernel.org # 6.6+
Assisted-by: Codex:gpt-5.6
Signed-off-by: Fan Wu <redacted>
---

 drivers/nfc/st-nci/core.c |  1 +
 drivers/nfc/st-nci/se.c   | 24 ++++++++++++------------
 2 files changed, 13 insertions(+), 12 deletions(-)
diff --git a/drivers/nfc/st-nci/core.c b/drivers/nfc/st-nci/core.c
index a367136d4..6e8f799f5 100644
--- a/drivers/nfc/st-nci/core.c
+++ b/drivers/nfc/st-nci/core.c
@@ -105,6 +105,7 @@ static const struct nci_ops st_nci_ops = {
 	.hci_load_session = st_nci_hci_load_session,
 	.hci_event_received = st_nci_hci_event_received,
 	.hci_cmd_received = st_nci_hci_cmd_received,
+	.pre_unregister = st_nci_se_deinit,
 	.prop_ops = st_nci_prop_ops,
 	.n_prop_ops = ARRAY_SIZE(st_nci_prop_ops),
 };
diff --git a/drivers/nfc/st-nci/se.c b/drivers/nfc/st-nci/se.c
index 607ec768e..232249d60 100644
--- a/drivers/nfc/st-nci/se.c
+++ b/drivers/nfc/st-nci/se.c
@@ -283,9 +283,9 @@ static int st_nci_hci_apdu_reader_event_received(struct nci_dev *ndev,
 	switch (event) {
 	case ST_NCI_EVT_TRANSMIT_DATA:
 		timer_delete_sync(&info->se_info.bwi_timer);
-		info->se_info.bwi_active = false;
-		info->se_info.cb(info->se_info.cb_context,
-				 skb->data, skb->len, 0);
+		if (xchg(&info->se_info.bwi_active, false))
+			info->se_info.cb(info->se_info.cb_context,
+					 skb->data, skb->len, 0);
 	break;
 	case ST_NCI_EVT_WTX_REQUEST:
 		mod_timer(&info->se_info.bwi_timer, jiffies +
@@ -665,9 +665,9 @@ int st_nci_se_io(struct nci_dev *ndev, u32 se_idx,
 	case ST_NCI_ESE_HOST_ID:
 		info->se_info.cb = cb;
 		info->se_info.cb_context = cb_context;
+		xchg(&info->se_info.bwi_active, true);
 		mod_timer(&info->se_info.bwi_timer, jiffies +
 			  msecs_to_jiffies(info->se_info.wt_timeout));
-		info->se_info.bwi_active = true;
 		return nci_hci_send_event(ndev, ST_NCI_APDU_READER_GATE,
 					ST_NCI_EVT_TRANSMIT_DATA, apdu,
 					apdu_length);
@@ -699,7 +699,8 @@ static void st_nci_se_wt_timeout(struct timer_list *t)
 	struct st_nci_info *info = timer_container_of(info, t,
 						      se_info.bwi_timer);
 
-	info->se_info.bwi_active = false;
+	if (!xchg(&info->se_info.bwi_active, false))
+		return;
 
 	if (!info->se_info.xch_error) {
 		info->se_info.xch_error = true;
@@ -751,13 +752,12 @@ void st_nci_se_deinit(struct nci_dev *ndev)
 {
 	struct st_nci_info *info = nci_get_drvdata(ndev);
 
-	if (info->se_info.bwi_active)
-		timer_delete_sync(&info->se_info.bwi_timer);
-	if (info->se_info.se_active)
-		timer_delete_sync(&info->se_info.se_active_timer);
+	timer_shutdown_sync(&info->se_info.bwi_timer);
+	if (xchg(&info->se_info.bwi_active, false))
+		info->se_info.cb(info->se_info.cb_context, NULL, 0, -ENODEV);
 
-	info->se_info.se_active = false;
-	info->se_info.bwi_active = false;
+	timer_shutdown_sync(&info->se_info.se_active_timer);
+	xchg(&info->se_info.se_active, false);
+	complete(&info->se_info.req_completion);
 }
 EXPORT_SYMBOL(st_nci_se_deinit);
-
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help