[PATCH] nfc: pn544: close firmware requests and IRQ before teardown

[PATCH] nfc: pn544: close firmware requests and IRQ before teardown

[Myeonghun Pak]

The I2C interrupt can queue fw_work after remove has cancelled it, or
pass a received frame to the HCI device while that device is being freed.
Releasing the managed IRQ only after remove returns leaves both paths
active during teardown.

Serialize firmware request admission with a driver-local mutex. Close the
request gate before joining the IRQ and draining firmware work, so a late
request cannot change the firmware state or report success after the
worker has been disabled. Do not hold the mutex across either join.

Keep the gate closed until the HCI context is initialized and the IRQ is
enabled. NFC registration publishes the device before pn544_hci_probe()
assigns the PHY's HCI pointer, so an earlier firmware request must be
rejected without modifying the firmware state.

Request the IRQ with IRQF_NO_AUTOEN and enable it after HCI setup. Release
and synchronize it before disabling firmware work. Complete an aborted
download while the HCI device is still alive, then remove that device.

This issue was identified as part of our ongoing research while
reviewing kernel code.

Fixes: 06c660340f1e ("NFC: pn544: i2c: Add firmware download implementation for pn544")
Assisted-by: LLM
Co-developed-by: Ijae Kim <ae878000@gmail.com>
Signed-off-by: Ijae Kim <ae878000@gmail.com>
Signed-off-by: Myeonghun Pak <mhun512@gmail.com>
---
Compile-tested with GCC 15.2.0, W=1 (x86_64). No runtime testing.

 drivers/nfc/pn544/i2c.c | 27 +++++++++++++++++++++++++--
 1 file changed, 25 insertions(+), 2 deletions(-)

diff --git a/drivers/nfc/pn544/i2c.c b/drivers/nfc/pn544/i2c.c
index 7fde3aefae70..057ef481579b 100644
--- a/drivers/nfc/pn544/i2c.c
+++ b/drivers/nfc/pn544/i2c.c
@@ -9,6 +9,7 @@
 
 #include <linux/crc-ccitt.h>
 #include <linux/module.h>
+#include <linux/mutex.h>
 #include <linux/i2c.h>
 #include <linux/acpi.h>
 #include <linux/interrupt.h>
@@ -153,6 +154,9 @@ struct pn544_i2c_phy {
 
 	u8 hw_variant;
 
+	/* Serializes firmware request admission with probe and removal. */
+	struct mutex fw_download_lock;
+	bool fw_download_enabled;
 	struct work_struct fw_work;
 	int fw_work_state;
 	char firmware_name[NFC_FIRMWARE_NAME_MAXSIZE + 1];
@@ -522,6 +526,12 @@ static int pn544_hci_i2c_fw_download(void *phy_id, const char *firmware_name,
 {
 	struct pn544_i2c_phy *phy = phy_id;
 
+	mutex_lock(&phy->fw_download_lock);
+	if (!phy->fw_download_enabled) {
+		mutex_unlock(&phy->fw_download_lock);
+		return -ENODEV;
+	}
+
 	pr_info("Starting Firmware Download (%s)\n", firmware_name);
 
 	strscpy(phy->firmware_name, firmware_name);
@@ -530,6 +540,7 @@ static int pn544_hci_i2c_fw_download(void *phy_id, const char *firmware_name,
 	phy->fw_work_state = FW_WORK_STATE_START;
 
 	schedule_work(&phy->fw_work);
+	mutex_unlock(&phy->fw_download_lock);
 
 	return 0;
 }
@@ -878,6 +889,7 @@ static int pn544_hci_i2c_probe(struct i2c_client *client)
 	if (!phy)
 		return -ENOMEM;
 
+	mutex_init(&phy->fw_download_lock);
 	INIT_WORK(&phy->fw_work, pn544_hci_i2c_fw_work);
 	phy->fw_work_state = FW_WORK_STATE_IDLE;
 
@@ -906,7 +918,8 @@ static int pn544_hci_i2c_probe(struct i2c_client *client)
 
 	r = devm_request_threaded_irq(&client->dev, client->irq, NULL,
 				      pn544_hci_i2c_irq_thread_fn,
-				      IRQF_TRIGGER_RISING | IRQF_ONESHOT,
+				      IRQF_TRIGGER_RISING | IRQF_ONESHOT |
+				      IRQF_NO_AUTOEN,
 				      PN544_HCI_I2C_DRIVER_NAME, phy);
 	if (r < 0) {
 		nfc_err(&client->dev, "Unable to register IRQ handler\n");
@@ -920,6 +933,11 @@ static int pn544_hci_i2c_probe(struct i2c_client *client)
 	if (r < 0)
 		return r;
 
+	enable_irq(client->irq);
+	mutex_lock(&phy->fw_download_lock);
+	phy->fw_download_enabled = true;
+	mutex_unlock(&phy->fw_download_lock);
+
 	return 0;
 }
 
@@ -927,7 +945,12 @@ static void pn544_hci_i2c_remove(struct i2c_client *client)
 {
 	struct pn544_i2c_phy *phy = i2c_get_clientdata(client);
 
-	cancel_work_sync(&phy->fw_work);
+	mutex_lock(&phy->fw_download_lock);
+	phy->fw_download_enabled = false;
+	mutex_unlock(&phy->fw_download_lock);
+
+	devm_free_irq(&client->dev, client->irq, phy);
+	disable_work_sync(&phy->fw_work);
 	if (phy->fw_work_state != FW_WORK_STATE_IDLE)
 		pn544_hci_i2c_fw_work_complete(phy, -ENODEV);