[PATCH net] nfc: st21nfca: complete a pending SE APDU request only once

[PATCH net] nfc: st21nfca: complete a pending SE APDU request only once

[Joas Antonio dos Santos]

st21nfca_hci_se_io() stores the netlink callback and its context in
info->se_info and arms the BWI timer.  The request is then completed
either by st21nfca_se_wt_work(), when the timer expires, or by an
EVT_TRANSMIT_DATA event on the APDU reader gate.  Neither checks that a
request is still pending, and the callback, se_io_cb(), frees its
context every time it is called.

So when the secure element answers after the timeout, the response
calls se_io_cb() again on the context the timeout already freed, and
frees it a second time.  The same happens if the controller sends two
EVT_TRANSMIT_DATA events for one request.  An EVT_TRANSMIT_DATA, or a
timer armed by EVT_WTX_REQUEST, with no request pending calls the
callback on the context of an earlier, already completed request, or
calls a NULL callback if there never was one.

With st21nfca on a virtual phy and an emulated controller answering an
NFC_CMD_SE_IO request 4 s after it was sent (the default timeout is
3.2 s):

  BUG: KASAN: slab-use-after-free in se_io_cb+0x2ec/0x334
  Read of size 4 at addr fff00000c5277da0 by task kworker/3:1/59
  Workqueue: events nfc_hci_msg_rx_work
  Call trace:
   se_io_cb+0x2ec/0x334
   st21nfca_apdu_reader_event_received+0x1ec/0x248
   st21nfca_hci_event_received+0xb0/0x300
   nfc_hci_event_received+0xb8/0x30c
   ...
  Freed by task 59:
   kfree+0x214/0x498
   se_io_cb+0x2bc/0x334
   st21nfca_se_wt_work+0x14c/0x1d4

and for an EVT_TRANSMIT_DATA without a request:

  Unable to handle kernel NULL pointer dereference at virtual address 0000000000000000
  pc : 0x0
  lr : st21nfca_apdu_reader_event_received+0x1ec/0x248

Use bwi_active, which st21nfca_hci_se_io() sets, to tell whether a
request is pending, and clear it in whichever of the two paths
completes the request first.  The event handler already stops the timer
and waits for the timeout work before looking at it.  Also complete the
request with the error if sending EVT_SE_END_OF_APDU_TRANSFER fails
rather than never calling the callback, which leaked the context.

Fixes: 2130fb97fecf ("NFC: st21nfca: Adding support for secure element")
Cc: stable@vger.kernel.org
Signed-off-by: Joas Antonio dos Santos <joasantonio108@gmail.com>
Assisted-by: Yaga Agent HackerSec Claude:claude-opus-5-5
---
Tested on arm64 QEMU with KASAN, with a test-only module that registers
st21nfca through st21nfca_hci_probe() on a virtual phy (LLC nop) and a
userspace controller emulator (DEV_UP, ENABLE_SE, SE_IO over netlink).
Without the patch: slab-use-after-free in se_io_cb() for a late and for
a duplicated EVT_TRANSMIT_DATA, NULL pointer call for an unsolicited one.
With it all three are quiet and a normal SE_IO exchange still works.
Not tested on hardware.

st-nci has the same pattern; Fan Wu's pending "nfc: st-nci: drain SE
timers before NCI teardown" v2 already makes bwi_active an ownership
token there, so this only touches st21nfca.

 drivers/nfc/st21nfca/se.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/drivers/nfc/st21nfca/se.c b/drivers/nfc/st21nfca/se.c
index 7154bc1d6..4261ed1d9 100644
--- a/drivers/nfc/st21nfca/se.c
+++ b/drivers/nfc/st21nfca/se.c
@@ -264,6 +264,12 @@ static void st21nfca_se_wt_work(struct work_struct *work)
 						struct st21nfca_hci_info,
 						se_info.timeout_work);
 
+	/* The timer can also be armed by EVT_WTX_REQUEST while no
+	 * st21nfca_hci_se_io() is pending.
+	 */
+	if (!info->se_info.bwi_active)
+		return;
+
 	info->se_info.bwi_active = false;
 
 	if (!info->se_info.xch_error) {
@@ -383,11 +389,20 @@ 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);
+		/* Only complete a pending st21nfca_hci_se_io(), and only once:
+		 * the timeout work may already have completed it, and the
+		 * callback frees its context.
+		 */
+		if (!info->se_info.bwi_active)
+			break;
+
 		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);
-		if (r < 0)
+		if (r < 0) {
+			info->se_info.cb(info->se_info.cb_context, NULL, 0, r);
 			goto exit;
+		}
 
 		info->se_info.cb(info->se_info.cb_context,
 			skb->data, skb->len, 0);
-- 
2.43.0