* [PATCH net v6 1/6] nfc: nci: skip controller reset while unregistering
2026-10-21 3:24 [PATCH net v6 0/6] nfc: fix NCI UART and firmware teardown races Shubham Antil
@ 2026-10-21 3:24 ` Shubham Antil
2026-10-21 3:24 ` [PATCH net v6 2/6] nfc: nci: uart: serialize driver selection Shubham Antil
` (4 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: Shubham Antil @ 2026-10-21 3:24 UTC (permalink / raw)
To: netdev; +Cc: David Heidelberg, oe-linux-nfc, gio, stable
nci_unregister_device() marks a device as unregistering before closing it,
but nci_close_device() still sends CORE_RESET and waits for a response. A
transport may already be detaching at this point. In particular, line
discipline close holds the TTY line discipline lock for writing, so receive
callbacks cannot deliver the response and close waits for the full timeout.
Skip the reset request when NCI_UNREG is set. Keep the reset for a normal
device-down operation, where the transport remains attached and can deliver
the response.
Found during a source audit assisted by OpenAI Codex, gpt-5.6-sol, and
reproduced by closing a pty-backed NCI UART while its NCI device was up.
Fixes: 6a2968aaf50c ("NFC: basic NCI protocol implementation")
Cc: stable@vger.kernel.org
Assisted-by: LLM KASAN
Signed-off-by: Shubham Antil <shubham@octane.security>
---
net/nfc/nci/core.c | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/net/nfc/nci/core.c b/net/nfc/nci/core.c
index 73e3a96470ac..8ee33505e93a 100644
--- a/net/nfc/nci/core.c
+++ b/net/nfc/nci/core.c
@@ -582,13 +582,15 @@ static int nci_close_device(struct nci_dev *ndev)
/* Flush TX wq, RX wq flush can't be under the lock */
flush_workqueue(ndev->tx_wq);
- /* Reset device */
+ /* Reset device unless it is being unregistered */
skb_queue_purge(&ndev->cmd_q);
- atomic_set(&ndev->cmd_cnt, 1);
- set_bit(NCI_INIT, &ndev->flags);
- __nci_request(ndev, nci_reset_req, (void *)0,
- msecs_to_jiffies(NCI_RESET_TIMEOUT));
+ if (!test_bit(NCI_UNREG, &ndev->flags)) {
+ atomic_set(&ndev->cmd_cnt, 1);
+ set_bit(NCI_INIT, &ndev->flags);
+ __nci_request(ndev, nci_reset_req, (void *)0,
+ msecs_to_jiffies(NCI_RESET_TIMEOUT));
+ }
/* After this point our queues are empty
* rx work may be running but will see that NCI_UP was cleared
--
2.43.0
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH net v6 2/6] nfc: nci: uart: serialize driver selection
2026-10-21 3:24 [PATCH net v6 0/6] nfc: fix NCI UART and firmware teardown races Shubham Antil
2026-10-21 3:24 ` [PATCH net v6 1/6] nfc: nci: skip controller reset while unregistering Shubham Antil
@ 2026-10-21 3:24 ` Shubham Antil
2026-10-21 3:24 ` [PATCH net v6 3/6] nfc: nci: uart: quiesce callbacks during teardown Shubham Antil
` (3 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: Shubham Antil @ 2026-10-21 3:24 UTC (permalink / raw)
To: netdev; +Cc: David Heidelberg, oe-linux-nfc, gio, stable
NCIUARTSETDRIVER assumes that tty->disc_data remains NULL until a
transport has been selected. Two concurrent callers can therefore both
allocate and register an NCI device for the same line discipline.
The transport registry can also be changed while a caller is copying a
template from it, before the transport module has been pinned.
Allocate one UART context when the line discipline is opened and claim it
atomically during driver selection. Protect the transport registry with a
mutex, take the module reference while holding that mutex, and copy only
the immutable template fields. Keep TTY callbacks disabled until the
transport has finished opening, and unwind the context so a failed
selection can be retried. Use acquire/release bit operations for the
selection claim so a retry observes the completed unwind. Treat the driver
number as unsigned before using it to index the registry.
Found during a source audit assisted by OpenAI Codex, gpt-5.6-sol, and
reproduced with two concurrent NCIUARTSETDRIVER ioctls on a pty-backed NCI
UART.
Fixes: 9961127d4bce ("NFC: nci: add generic uart support")
Cc: stable@vger.kernel.org
Assisted-by: LLM KASAN
Signed-off-by: Shubham Antil <shubham@octane.security>
---
include/net/nfc/nci_core.h | 1 +
net/nfc/nci/uart.c | 133 +++++++++++++++++++++++++------------
2 files changed, 93 insertions(+), 41 deletions(-)
diff --git a/include/net/nfc/nci_core.h b/include/net/nfc/nci_core.h
index 664d5058e66e..2f542f5c9496 100644
--- a/include/net/nfc/nci_core.h
+++ b/include/net/nfc/nci_core.h
@@ -454,6 +454,7 @@ struct nci_uart {
spinlock_t rx_lock;
struct work_struct write_work;
struct tty_struct *tty;
+ unsigned long flags;
unsigned long tx_state;
struct sk_buff_head tx_q;
struct sk_buff *tx_skb;
diff --git a/net/nfc/nci/uart.c b/net/nfc/nci/uart.c
index aa20e8603f32..fb9ed281f62c 100644
--- a/net/nfc/nci/uart.c
+++ b/net/nfc/nci/uart.c
@@ -13,6 +13,7 @@
#include <linux/kernel.h>
#include <linux/init.h>
+#include <linux/mutex.h>
#include <linux/types.h>
#include <linux/fcntl.h>
#include <linux/interrupt.h>
@@ -34,7 +35,12 @@
#define NCI_UART_SENDING 1
#define NCI_UART_TX_WAKEUP 2
+/* UART flags */
+#define NCI_UART_DRIVER_SET 0
+#define NCI_UART_READY 1
+
static struct nci_uart *nci_uart_drivers[NCI_UART_DRIVER_MAX];
+static DEFINE_MUTEX(nci_uart_drivers_lock);
static inline struct sk_buff *nci_uart_dequeue(struct nci_uart *nu)
{
@@ -102,39 +108,57 @@ restart:
clear_bit(NCI_UART_SENDING, &nu->tx_state);
}
-static int nci_uart_set_driver(struct tty_struct *tty, unsigned int driver)
+static int nci_uart_set_driver(struct nci_uart *nu, unsigned int driver)
{
- struct nci_uart *nu = NULL;
+ struct nci_uart *template;
int ret;
if (driver >= NCI_UART_DRIVER_MAX)
return -EINVAL;
- if (!nci_uart_drivers[driver])
- return -ENOENT;
+ if (test_and_set_bit_lock(NCI_UART_DRIVER_SET, &nu->flags))
+ return -EBUSY;
- nu = kzalloc_obj(*nu);
- if (!nu)
- return -ENOMEM;
+ mutex_lock(&nci_uart_drivers_lock);
+ template = nci_uart_drivers[driver];
+ if (!template || !try_module_get(template->owner)) {
+ ret = -ENOENT;
+ goto err_unlock;
+ }
- memcpy(nu, nci_uart_drivers[driver], sizeof(struct nci_uart));
- nu->tty = tty;
- skb_queue_head_init(&nu->tx_q);
- INIT_WORK(&nu->write_work, nci_uart_write_work);
- spin_lock_init(&nu->rx_lock);
+ nu->owner = template->owner;
+ nu->ops = template->ops;
+ nu->name = template->name;
+ nu->driver = template->driver;
+ mutex_unlock(&nci_uart_drivers_lock);
ret = nu->ops.open(nu);
- if (ret) {
- kfree(nu);
- return ret;
- } else if (!try_module_get(nu->owner)) {
- nu->ops.close(nu);
- kfree(nu);
- return -ENOENT;
- }
- tty->disc_data = nu;
+ if (ret)
+ goto err_module;
+
+ /* Publish transport fields before callbacks observe READY. */
+ smp_mb__before_atomic();
+ set_bit(NCI_UART_READY, &nu->flags);
return 0;
+
+err_module:
+ cancel_work_sync(&nu->write_work);
+ kfree_skb(nu->tx_skb);
+ nu->tx_skb = NULL;
+ kfree_skb(nu->rx_skb);
+ nu->rx_skb = NULL;
+ skb_queue_purge(&nu->tx_q);
+ clear_bit(NCI_UART_SENDING, &nu->tx_state);
+ clear_bit(NCI_UART_TX_WAKEUP, &nu->tx_state);
+ module_put(nu->owner);
+ nu->owner = NULL;
+ goto err_clear;
+err_unlock:
+ mutex_unlock(&nci_uart_drivers_lock);
+err_clear:
+ clear_bit_unlock(NCI_UART_DRIVER_SET, &nu->flags);
+ return ret;
}
/* ------ LDISC part ------ */
@@ -156,7 +180,17 @@ static int nci_uart_tty_open(struct tty_struct *tty)
if (!tty->ops->write)
return -EOPNOTSUPP;
- tty->disc_data = NULL;
+ struct nci_uart *nu = kzalloc_obj(*nu);
+
+ if (!nu)
+ return -ENOMEM;
+
+ nu->tty = tty;
+ skb_queue_head_init(&nu->tx_q);
+ INIT_WORK(&nu->write_work, nci_uart_write_work);
+ spin_lock_init(&nu->rx_lock);
+
+ tty->disc_data = nu;
tty->receive_room = 65536;
/* Flush any pending characters in the driver */
@@ -180,14 +214,17 @@ static void nci_uart_tty_close(struct tty_struct *tty)
if (!nu)
return;
- kfree_skb(nu->tx_skb);
- kfree_skb(nu->rx_skb);
+ if (test_bit(NCI_UART_DRIVER_SET, &nu->flags)) {
+ kfree_skb(nu->tx_skb);
+ kfree_skb(nu->rx_skb);
- skb_queue_purge(&nu->tx_q);
+ skb_queue_purge(&nu->tx_q);
+
+ nu->ops.close(nu);
+ module_put(nu->owner);
+ }
- nu->ops.close(nu);
nu->tty = NULL;
- module_put(nu->owner);
cancel_work_sync(&nu->write_work);
@@ -211,7 +248,8 @@ static void nci_uart_tty_wakeup(struct tty_struct *tty)
clear_bit(TTY_DO_WRITE_WAKEUP, &tty->flags);
- if (tty != nu->tty)
+ if (tty != nu->tty ||
+ !test_bit_acquire(NCI_UART_READY, &nu->flags))
return;
nci_uart_tx_wakeup(nu);
@@ -300,7 +338,8 @@ static void nci_uart_tty_receive(struct tty_struct *tty, const u8 *data,
{
struct nci_uart *nu = tty->disc_data;
- if (!nu || tty != nu->tty)
+ if (!nu || tty != nu->tty ||
+ !test_bit_acquire(NCI_UART_READY, &nu->flags))
return;
spin_lock(&nu->rx_lock);
@@ -328,13 +367,12 @@ static int nci_uart_tty_ioctl(struct tty_struct *tty, unsigned int cmd,
struct nci_uart *nu = tty->disc_data;
int err = 0;
+ if (!nu)
+ return -EBADF;
+
switch (cmd) {
case NCIUARTSETDRIVER:
- if (!nu)
- return nci_uart_set_driver(tty, (unsigned int)arg);
- else
- return -EBUSY;
- break;
+ return nci_uart_set_driver(nu, (unsigned int)arg);
default:
err = n_tty_ioctl_helper(tty, cmd, arg);
break;
@@ -370,33 +408,46 @@ static int nci_uart_send(struct nci_uart *nu, struct sk_buff *skb)
int nci_uart_register(struct nci_uart *nu)
{
+ int ret = 0;
+
if (!nu || !nu->ops.open ||
- !nu->ops.recv || !nu->ops.close)
+ !nu->ops.recv || !nu->ops.close ||
+ (unsigned int)nu->driver >= NCI_UART_DRIVER_MAX)
return -EINVAL;
- /* Set the send callback */
- nu->ops.send = nci_uart_send;
-
/* Add this driver in the driver list */
+ mutex_lock(&nci_uart_drivers_lock);
if (nci_uart_drivers[nu->driver]) {
pr_err("driver %d is already registered\n", nu->driver);
- return -EBUSY;
+ ret = -EBUSY;
+ goto unlock;
}
+
+ /* Set the send callback */
+ nu->ops.send = nci_uart_send;
nci_uart_drivers[nu->driver] = nu;
pr_info("NCI uart driver '%s [%d]' registered\n", nu->name, nu->driver);
- return 0;
+unlock:
+ mutex_unlock(&nci_uart_drivers_lock);
+ return ret;
}
EXPORT_SYMBOL_GPL(nci_uart_register);
void nci_uart_unregister(struct nci_uart *nu)
{
+ mutex_lock(&nci_uart_drivers_lock);
+
pr_info("NCI uart driver '%s [%d]' unregistered\n", nu->name,
nu->driver);
/* Remove this driver from the driver list */
- nci_uart_drivers[nu->driver] = NULL;
+ if ((unsigned int)nu->driver < NCI_UART_DRIVER_MAX &&
+ nci_uart_drivers[nu->driver] == nu)
+ nci_uart_drivers[nu->driver] = NULL;
+
+ mutex_unlock(&nci_uart_drivers_lock);
}
EXPORT_SYMBOL_GPL(nci_uart_unregister);
--
2.43.0
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH net v6 3/6] nfc: nci: uart: quiesce callbacks during teardown
2026-10-21 3:24 [PATCH net v6 0/6] nfc: fix NCI UART and firmware teardown races Shubham Antil
2026-10-21 3:24 ` [PATCH net v6 1/6] nfc: nci: skip controller reset while unregistering Shubham Antil
2026-10-21 3:24 ` [PATCH net v6 2/6] nfc: nci: uart: serialize driver selection Shubham Antil
@ 2026-10-21 3:24 ` Shubham Antil
2026-10-21 3:24 ` [PATCH net v6 4/6] nfc: nfcmrvl: publish UART state before registration Shubham Antil
` (2 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: Shubham Antil @ 2026-10-21 3:24 UTC (permalink / raw)
To: netdev; +Cc: David Heidelberg, oe-linux-nfc, gio, stable
nci_uart_tty_close() releases transport state before ensuring that the
UART write worker and receive callback have stopped using it. A write
wakeup can also race the worker's final TX_WAKEUP test and SENDING handoff.
Serialize work scheduling and the worker's final state transition with a
transmit lock. Once quiescing starts, block new scheduling and cancel the
worker synchronously. Clear the TTY write-wakeup request after the
cancellation. Keep a locked TX_WAKEUP check before tx_done() and another
after it so the original Marvell BREAK timing is preserved while the
lost-wakeup window remains closed.
Stop generic receive and transmit callbacks before invoking the transport
close operation. The preceding change skips the unregister-time controller
reset because the TTY core cannot deliver its response after line
discipline close starts. Detach the UART context from the TTY before
quiescing, and let the Marvell transport teardown clear its back-pointers
before releasing private state. Apply the same barriers to the failed-open
path.
Found during a source audit assisted by OpenAI Codex, gpt-5.6-sol, and
reproduced with repeated pty-backed NCI UART registration and hangup cycles
under KASAN.
Fixes: 9961127d4bce ("NFC: nci: add generic uart support")
Cc: stable@vger.kernel.org
Assisted-by: LLM KASAN
Signed-off-by: Shubham Antil <shubham@octane.security>
---
drivers/nfc/nfcmrvl/main.c | 5 ++
drivers/nfc/nfcmrvl/nfcmrvl.h | 1 +
drivers/nfc/nfcmrvl/uart.c | 11 ++++
include/net/nfc/nci_core.h | 5 ++
net/nfc/nci/uart.c | 109 ++++++++++++++++++++++++++++------
5 files changed, 113 insertions(+), 18 deletions(-)
diff --git a/drivers/nfc/nfcmrvl/main.c b/drivers/nfc/nfcmrvl/main.c
index 6efa83219117..baf4c781ee6b 100644
--- a/drivers/nfc/nfcmrvl/main.c
+++ b/drivers/nfc/nfcmrvl/main.c
@@ -167,6 +167,8 @@ struct nfcmrvl_private *nfcmrvl_nci_register_dev(enum nfcmrvl_phy phy,
return priv;
error_fw_dnld_deinit:
+ if (priv->if_ops->nci_quiesce)
+ priv->if_ops->nci_quiesce(priv);
nfcmrvl_fw_dnld_deinit(priv);
error_free_dev:
nci_free_device(priv->ndev);
@@ -181,6 +183,9 @@ void nfcmrvl_nci_unregister_dev(struct nfcmrvl_private *priv)
struct nci_dev *ndev = priv->ndev;
nci_unregister_device(ndev);
+ if (priv->if_ops->nci_quiesce)
+ priv->if_ops->nci_quiesce(priv);
+
if (priv->ndev->nfc_dev->fw_download_in_progress)
nfcmrvl_fw_dnld_abort(priv);
diff --git a/drivers/nfc/nfcmrvl/nfcmrvl.h b/drivers/nfc/nfcmrvl/nfcmrvl.h
index 62fa77d587b2..61b3f4a4f096 100644
--- a/drivers/nfc/nfcmrvl/nfcmrvl.h
+++ b/drivers/nfc/nfcmrvl/nfcmrvl.h
@@ -112,6 +112,7 @@ struct nfcmrvl_if_ops {
int (*nci_open) (struct nfcmrvl_private *priv);
int (*nci_close) (struct nfcmrvl_private *priv);
int (*nci_send) (struct nfcmrvl_private *priv, struct sk_buff *skb);
+ void (*nci_quiesce)(struct nfcmrvl_private *priv);
void (*nci_update_config)(struct nfcmrvl_private *priv,
const void *param);
};
diff --git a/drivers/nfc/nfcmrvl/uart.c b/drivers/nfc/nfcmrvl/uart.c
index 9aedd168759c..66803f1c6fd3 100644
--- a/drivers/nfc/nfcmrvl/uart.c
+++ b/drivers/nfc/nfcmrvl/uart.c
@@ -44,6 +44,16 @@ static int nfcmrvl_uart_nci_send(struct nfcmrvl_private *priv,
return nu->ops.send(nu, skb);
}
+static void nfcmrvl_uart_nci_quiesce(struct nfcmrvl_private *priv)
+{
+ struct nci_uart *nu = priv->drv_data;
+
+ nci_uart_rx_stop(nu);
+ nci_uart_tx_stop(nu);
+ nu->drv_data = NULL;
+ nu->ndev = NULL;
+}
+
static void nfcmrvl_uart_nci_update_config(struct nfcmrvl_private *priv,
const void *param)
{
@@ -58,6 +68,7 @@ static const struct nfcmrvl_if_ops uart_ops = {
.nci_open = nfcmrvl_uart_nci_open,
.nci_close = nfcmrvl_uart_nci_close,
.nci_send = nfcmrvl_uart_nci_send,
+ .nci_quiesce = nfcmrvl_uart_nci_quiesce,
.nci_update_config = nfcmrvl_uart_nci_update_config
};
diff --git a/include/net/nfc/nci_core.h b/include/net/nfc/nci_core.h
index 2f542f5c9496..99797872c2e0 100644
--- a/include/net/nfc/nci_core.h
+++ b/include/net/nfc/nci_core.h
@@ -436,6 +436,7 @@ struct nci_uart;
struct nci_uart_ops {
int (*open)(struct nci_uart *nci_uart);
+ /* Release transport-private state after generic callbacks stop. */
void (*close)(struct nci_uart *nci_uart);
int (*recv)(struct nci_uart *nci_uart, struct sk_buff *skb);
int (*send)(struct nci_uart *nci_uart, struct sk_buff *skb);
@@ -452,6 +453,7 @@ struct nci_uart {
/* Dynamic data */
struct nci_dev *ndev;
spinlock_t rx_lock;
+ spinlock_t tx_lock; /* protects tx_state */
struct work_struct write_work;
struct tty_struct *tty;
unsigned long flags;
@@ -466,5 +468,8 @@ struct nci_uart {
int nci_uart_register(struct nci_uart *nu);
void nci_uart_unregister(struct nci_uart *nu);
void nci_uart_set_config(struct nci_uart *nu, int baudrate, int flow_ctrl);
+void nci_uart_set_ready(struct nci_uart *nu);
+void nci_uart_rx_stop(struct nci_uart *nu);
+void nci_uart_tx_stop(struct nci_uart *nu);
#endif /* __NCI_CORE_H */
diff --git a/net/nfc/nci/uart.c b/net/nfc/nci/uart.c
index fb9ed281f62c..aa24e1f572d9 100644
--- a/net/nfc/nci/uart.c
+++ b/net/nfc/nci/uart.c
@@ -34,6 +34,7 @@
/* TX states */
#define NCI_UART_SENDING 1
#define NCI_UART_TX_WAKEUP 2
+#define NCI_UART_CLOSING 3
/* UART flags */
#define NCI_UART_DRIVER_SET 0
@@ -64,24 +65,61 @@ static inline int nci_uart_queue_empty(struct nci_uart *nu)
static int nci_uart_tx_wakeup(struct nci_uart *nu)
{
+ unsigned long flags;
+
+ spin_lock_irqsave(&nu->tx_lock, flags);
+ if (test_bit(NCI_UART_CLOSING, &nu->tx_state) ||
+ !test_bit_acquire(NCI_UART_READY, &nu->flags))
+ goto unlock;
+
if (test_and_set_bit(NCI_UART_SENDING, &nu->tx_state)) {
set_bit(NCI_UART_TX_WAKEUP, &nu->tx_state);
- return 0;
+ goto unlock;
}
schedule_work(&nu->write_work);
+unlock:
+ spin_unlock_irqrestore(&nu->tx_lock, flags);
+
return 0;
}
+void nci_uart_tx_stop(struct nci_uart *nu)
+{
+ unsigned long flags;
+
+ spin_lock_irqsave(&nu->tx_lock, flags);
+ set_bit(NCI_UART_CLOSING, &nu->tx_state);
+ spin_unlock_irqrestore(&nu->tx_lock, flags);
+
+ cancel_work_sync(&nu->write_work);
+ clear_bit(TTY_DO_WRITE_WAKEUP, &nu->tty->flags);
+
+ spin_lock_irqsave(&nu->tx_lock, flags);
+ clear_bit(NCI_UART_SENDING, &nu->tx_state);
+ clear_bit(NCI_UART_TX_WAKEUP, &nu->tx_state);
+ spin_unlock_irqrestore(&nu->tx_lock, flags);
+}
+EXPORT_SYMBOL_GPL(nci_uart_tx_stop);
+
static void nci_uart_write_work(struct work_struct *work)
{
struct nci_uart *nu = container_of(work, struct nci_uart, write_work);
struct tty_struct *tty = nu->tty;
struct sk_buff *skb;
+ unsigned long flags;
restart:
+ spin_lock_irqsave(&nu->tx_lock, flags);
+ if (test_bit(NCI_UART_CLOSING, &nu->tx_state)) {
+ clear_bit(NCI_UART_SENDING, &nu->tx_state);
+ clear_bit(NCI_UART_TX_WAKEUP, &nu->tx_state);
+ spin_unlock_irqrestore(&nu->tx_lock, flags);
+ return;
+ }
clear_bit(NCI_UART_TX_WAKEUP, &nu->tx_state);
+ spin_unlock_irqrestore(&nu->tx_lock, flags);
if (nu->ops.tx_start)
nu->ops.tx_start(nu);
@@ -99,14 +137,50 @@ restart:
kfree_skb(skb);
}
- if (test_bit(NCI_UART_TX_WAKEUP, &nu->tx_state))
+ spin_lock_irqsave(&nu->tx_lock, flags);
+ if (test_bit(NCI_UART_CLOSING, &nu->tx_state)) {
+ clear_bit(NCI_UART_SENDING, &nu->tx_state);
+ clear_bit(NCI_UART_TX_WAKEUP, &nu->tx_state);
+ spin_unlock_irqrestore(&nu->tx_lock, flags);
+ return;
+ }
+ if (test_and_clear_bit(NCI_UART_TX_WAKEUP, &nu->tx_state)) {
+ spin_unlock_irqrestore(&nu->tx_lock, flags);
goto restart;
+ }
+ spin_unlock_irqrestore(&nu->tx_lock, flags);
if (nu->ops.tx_done && nci_uart_queue_empty(nu))
nu->ops.tx_done(nu);
+ spin_lock_irqsave(&nu->tx_lock, flags);
+ if (!test_bit(NCI_UART_CLOSING, &nu->tx_state) &&
+ test_and_clear_bit(NCI_UART_TX_WAKEUP, &nu->tx_state)) {
+ spin_unlock_irqrestore(&nu->tx_lock, flags);
+ goto restart;
+ }
clear_bit(NCI_UART_SENDING, &nu->tx_state);
+ clear_bit(NCI_UART_TX_WAKEUP, &nu->tx_state);
+ spin_unlock_irqrestore(&nu->tx_lock, flags);
+}
+
+void nci_uart_set_ready(struct nci_uart *nu)
+{
+ /* Publish transport fields before callbacks observe READY. */
+ smp_mb__before_atomic();
+ set_bit(NCI_UART_READY, &nu->flags);
}
+EXPORT_SYMBOL_GPL(nci_uart_set_ready);
+
+void nci_uart_rx_stop(struct nci_uart *nu)
+{
+ spin_lock(&nu->rx_lock);
+ clear_bit(NCI_UART_READY, &nu->flags);
+ kfree_skb(nu->rx_skb);
+ nu->rx_skb = NULL;
+ spin_unlock(&nu->rx_lock);
+}
+EXPORT_SYMBOL_GPL(nci_uart_rx_stop);
static int nci_uart_set_driver(struct nci_uart *nu, unsigned int driver)
{
@@ -136,21 +210,17 @@ static int nci_uart_set_driver(struct nci_uart *nu, unsigned int driver)
if (ret)
goto err_module;
- /* Publish transport fields before callbacks observe READY. */
- smp_mb__before_atomic();
- set_bit(NCI_UART_READY, &nu->flags);
+ nci_uart_set_ready(nu);
return 0;
err_module:
- cancel_work_sync(&nu->write_work);
+ nci_uart_rx_stop(nu);
+ nci_uart_tx_stop(nu);
kfree_skb(nu->tx_skb);
nu->tx_skb = NULL;
- kfree_skb(nu->rx_skb);
- nu->rx_skb = NULL;
skb_queue_purge(&nu->tx_q);
- clear_bit(NCI_UART_SENDING, &nu->tx_state);
- clear_bit(NCI_UART_TX_WAKEUP, &nu->tx_state);
+ clear_bit(NCI_UART_CLOSING, &nu->tx_state);
module_put(nu->owner);
nu->owner = NULL;
goto err_clear;
@@ -188,6 +258,7 @@ static int nci_uart_tty_open(struct tty_struct *tty)
nu->tty = tty;
skb_queue_head_init(&nu->tx_q);
INIT_WORK(&nu->write_work, nci_uart_write_work);
+ spin_lock_init(&nu->tx_lock);
spin_lock_init(&nu->rx_lock);
tty->disc_data = nu;
@@ -208,26 +279,24 @@ static void nci_uart_tty_close(struct tty_struct *tty)
{
struct nci_uart *nu = tty->disc_data;
- /* Detach from the tty */
- tty->disc_data = NULL;
-
if (!nu)
return;
+ tty->disc_data = NULL;
+
if (test_bit(NCI_UART_DRIVER_SET, &nu->flags)) {
- kfree_skb(nu->tx_skb);
- kfree_skb(nu->rx_skb);
+ nci_uart_rx_stop(nu);
+ nci_uart_tx_stop(nu);
+ nu->ops.close(nu);
+ kfree_skb(nu->tx_skb);
skb_queue_purge(&nu->tx_q);
- nu->ops.close(nu);
module_put(nu->owner);
}
nu->tty = NULL;
- cancel_work_sync(&nu->write_work);
-
kfree(nu);
}
@@ -343,6 +412,10 @@ static void nci_uart_tty_receive(struct tty_struct *tty, const u8 *data,
return;
spin_lock(&nu->rx_lock);
+ if (!test_bit(NCI_UART_READY, &nu->flags)) {
+ spin_unlock(&nu->rx_lock);
+ return;
+ }
nci_uart_default_recv_buf(nu, data, count);
spin_unlock(&nu->rx_lock);
--
2.43.0
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH net v6 4/6] nfc: nfcmrvl: publish UART state before registration
2026-10-21 3:24 [PATCH net v6 0/6] nfc: fix NCI UART and firmware teardown races Shubham Antil
` (2 preceding siblings ...)
2026-10-21 3:24 ` [PATCH net v6 3/6] nfc: nci: uart: quiesce callbacks during teardown Shubham Antil
@ 2026-10-21 3:24 ` Shubham Antil
2026-10-21 3:24 ` [PATCH net v6 5/6] nfc: serialize firmware download state Shubham Antil
2026-10-21 3:24 ` [PATCH net v6 6/6] nfc: nfcmrvl: stop firmware work before unregistering Shubham Antil
5 siblings, 0 replies; 7+ messages in thread
From: Shubham Antil @ 2026-10-21 3:24 UTC (permalink / raw)
To: netdev; +Cc: David Heidelberg, oe-linux-nfc, gio, stable
nfcmrvl_nci_uart_open() does not publish the UART back-pointers until
nfcmrvl_nci_register_dev() returns. The registration call exposes the NCI
device first, so an early open can reach the transmit path while those
pointers are unset. Its response is also dropped because the TTY receive
path is not enabled yet.
Add a transport callback which publishes the Marvell UART state and
enables its TTY callbacks before nci_register_device(). Halt the
controller before that publication. The existing quiesce callback rolls
the state back if registration fails and withdraws it during normal
teardown.
The generic UART layer still marks the transport ready after ->open()
returns. For Marvell, repeating the publication is idempotent. It also
retains the fallback for transports without an early publication hook.
Found during a source audit assisted by OpenAI Codex, gpt-5.6-sol, and
reproduced with a pty-backed controller that returned CORE_RESET_RSP during
device registration.
Fixes: e097dc624f78 ("NFC: nfcmrvl: add UART driver")
Cc: stable@vger.kernel.org
Assisted-by: LLM KASAN
Signed-off-by: Shubham Antil <shubham@octane.security>
---
drivers/nfc/nfcmrvl/main.c | 9 ++++++---
drivers/nfc/nfcmrvl/nfcmrvl.h | 1 +
drivers/nfc/nfcmrvl/uart.c | 16 +++++++++++-----
3 files changed, 18 insertions(+), 8 deletions(-)
diff --git a/drivers/nfc/nfcmrvl/main.c b/drivers/nfc/nfcmrvl/main.c
index baf4c781ee6b..b6f47cce0f1f 100644
--- a/drivers/nfc/nfcmrvl/main.c
+++ b/drivers/nfc/nfcmrvl/main.c
@@ -154,15 +154,18 @@ struct nfcmrvl_private *nfcmrvl_nci_register_dev(enum nfcmrvl_phy phy,
nci_set_drvdata(priv->ndev, priv);
+ /* Ensure that the controller is powered off before exposing it. */
+ nfcmrvl_chip_halt(priv);
+
+ if (priv->if_ops->nci_ready)
+ priv->if_ops->nci_ready(priv);
+
rc = nci_register_device(priv->ndev);
if (rc) {
nfc_err(dev, "nci_register_device failed %d\n", rc);
goto error_fw_dnld_deinit;
}
- /* Ensure that controller is powered off */
- nfcmrvl_chip_halt(priv);
-
nfc_info(dev, "registered with nci successfully\n");
return priv;
diff --git a/drivers/nfc/nfcmrvl/nfcmrvl.h b/drivers/nfc/nfcmrvl/nfcmrvl.h
index 61b3f4a4f096..5c2a5382c280 100644
--- a/drivers/nfc/nfcmrvl/nfcmrvl.h
+++ b/drivers/nfc/nfcmrvl/nfcmrvl.h
@@ -112,6 +112,7 @@ struct nfcmrvl_if_ops {
int (*nci_open) (struct nfcmrvl_private *priv);
int (*nci_close) (struct nfcmrvl_private *priv);
int (*nci_send) (struct nfcmrvl_private *priv, struct sk_buff *skb);
+ void (*nci_ready)(struct nfcmrvl_private *priv);
void (*nci_quiesce)(struct nfcmrvl_private *priv);
void (*nci_update_config)(struct nfcmrvl_private *priv,
const void *param);
diff --git a/drivers/nfc/nfcmrvl/uart.c b/drivers/nfc/nfcmrvl/uart.c
index 66803f1c6fd3..95223c20d50f 100644
--- a/drivers/nfc/nfcmrvl/uart.c
+++ b/drivers/nfc/nfcmrvl/uart.c
@@ -44,6 +44,16 @@ static int nfcmrvl_uart_nci_send(struct nfcmrvl_private *priv,
return nu->ops.send(nu, skb);
}
+static void nfcmrvl_uart_nci_ready(struct nfcmrvl_private *priv)
+{
+ struct nci_uart *nu = priv->drv_data;
+
+ priv->support_fw_dnld = true;
+ nu->drv_data = priv;
+ nu->ndev = priv->ndev;
+ nci_uart_set_ready(nu);
+}
+
static void nfcmrvl_uart_nci_quiesce(struct nfcmrvl_private *priv)
{
struct nci_uart *nu = priv->drv_data;
@@ -68,6 +78,7 @@ static const struct nfcmrvl_if_ops uart_ops = {
.nci_open = nfcmrvl_uart_nci_open,
.nci_close = nfcmrvl_uart_nci_close,
.nci_send = nfcmrvl_uart_nci_send,
+ .nci_ready = nfcmrvl_uart_nci_ready,
.nci_quiesce = nfcmrvl_uart_nci_quiesce,
.nci_update_config = nfcmrvl_uart_nci_update_config
};
@@ -147,11 +158,6 @@ static int nfcmrvl_nci_uart_open(struct nci_uart *nu)
if (IS_ERR(priv))
return PTR_ERR(priv);
- priv->support_fw_dnld = true;
-
- nu->drv_data = priv;
- nu->ndev = priv->ndev;
-
return 0;
}
--
2.43.0
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH net v6 5/6] nfc: serialize firmware download state
2026-10-21 3:24 [PATCH net v6 0/6] nfc: fix NCI UART and firmware teardown races Shubham Antil
` (3 preceding siblings ...)
2026-10-21 3:24 ` [PATCH net v6 4/6] nfc: nfcmrvl: publish UART state before registration Shubham Antil
@ 2026-10-21 3:24 ` Shubham Antil
2026-10-21 3:24 ` [PATCH net v6 6/6] nfc: nfcmrvl: stop firmware work before unregistering Shubham Antil
5 siblings, 0 replies; 7+ messages in thread
From: Shubham Antil @ 2026-10-21 3:24 UTC (permalink / raw)
To: netdev; +Cc: David Heidelberg, oe-linux-nfc, gio, stable
nfc_fw_download() marks a firmware download in progress but does not reject
a second request. An overlapping request can replace driver-owned download
state, and an error from that request clears the shared in-progress flag
while the first download is still running.
Make the in-progress state atomic and claim it before invoking the driver.
An overlapping request then leaves the first download's state unchanged.
Copy the firmware name before releasing that claim, then send the
completion notification after the release so userspace can immediately
start its next operation. Use release/acquire ordering so the next
operation observes the completed download cleanup.
OpenAI Codex, gpt-5.6-sol assisted a source audit of NFC firmware teardown.
Fixes: 9674da8759df ("NFC: Add firmware upload netlink command")
Cc: stable@vger.kernel.org
Assisted-by: LLM KASAN
Signed-off-by: Shubham Antil <shubham@octane.security>
---
drivers/nfc/nfcmrvl/main.c | 4 ++--
drivers/nfc/nfcmrvl/uart.c | 4 ++--
drivers/nfc/st-nci/vendor_cmds.c | 8 ++++----
include/net/nfc/nfc.h | 3 ++-
net/nfc/core.c | 19 ++++++++++++++-----
5 files changed, 24 insertions(+), 14 deletions(-)
diff --git a/drivers/nfc/nfcmrvl/main.c b/drivers/nfc/nfcmrvl/main.c
index b6f47cce0f1f..27fadd64155e 100644
--- a/drivers/nfc/nfcmrvl/main.c
+++ b/drivers/nfc/nfcmrvl/main.c
@@ -189,7 +189,7 @@ void nfcmrvl_nci_unregister_dev(struct nfcmrvl_private *priv)
if (priv->if_ops->nci_quiesce)
priv->if_ops->nci_quiesce(priv);
- if (priv->ndev->nfc_dev->fw_download_in_progress)
+ if (atomic_read(&priv->ndev->nfc_dev->fw_download_in_progress))
nfcmrvl_fw_dnld_abort(priv);
nfcmrvl_fw_dnld_deinit(priv);
@@ -213,7 +213,7 @@ int nfcmrvl_nci_recv_frame(struct nfcmrvl_private *priv, struct sk_buff *skb)
}
}
- if (priv->ndev->nfc_dev->fw_download_in_progress) {
+ if (atomic_read(&priv->ndev->nfc_dev->fw_download_in_progress)) {
nfcmrvl_fw_dnld_recv_frame(priv, skb);
return 0;
}
diff --git a/drivers/nfc/nfcmrvl/uart.c b/drivers/nfc/nfcmrvl/uart.c
index 95223c20d50f..d87f6497baf2 100644
--- a/drivers/nfc/nfcmrvl/uart.c
+++ b/drivers/nfc/nfcmrvl/uart.c
@@ -176,7 +176,7 @@ static void nfcmrvl_nci_uart_tx_start(struct nci_uart *nu)
{
struct nfcmrvl_private *priv = (struct nfcmrvl_private *)nu->drv_data;
- if (priv->ndev->nfc_dev->fw_download_in_progress)
+ if (atomic_read(&priv->ndev->nfc_dev->fw_download_in_progress))
return;
/* Remove BREAK to wake up the NFCC */
@@ -190,7 +190,7 @@ static void nfcmrvl_nci_uart_tx_done(struct nci_uart *nu)
{
struct nfcmrvl_private *priv = (struct nfcmrvl_private *)nu->drv_data;
- if (priv->ndev->nfc_dev->fw_download_in_progress)
+ if (atomic_read(&priv->ndev->nfc_dev->fw_download_in_progress))
return;
/*
diff --git a/drivers/nfc/st-nci/vendor_cmds.c b/drivers/nfc/st-nci/vendor_cmds.c
index 6335d7afca24..ec577d497500 100644
--- a/drivers/nfc/st-nci/vendor_cmds.c
+++ b/drivers/nfc/st-nci/vendor_cmds.c
@@ -158,11 +158,12 @@ static int st_nci_hci_dm_fwupd_start(struct nfc_dev *dev, void *data,
int r;
struct nci_dev *ndev = nfc_get_drvdata(dev);
- dev->fw_download_in_progress = true;
+ if (atomic_cmpxchg_acquire(&dev->fw_download_in_progress, 0, 1))
+ return -EBUSY;
r = nci_hci_send_cmd(ndev, ST_NCI_DEVICE_MGNT_GATE,
ST_NCI_HCI_DM_FWUPD_START, data, data_len, NULL);
if (r)
- dev->fw_download_in_progress = false;
+ atomic_set_release(&dev->fw_download_in_progress, 0);
return r;
}
@@ -181,8 +182,7 @@ static int st_nci_hci_dm_direct_load(struct nfc_dev *dev, void *data,
{
struct nci_dev *ndev = nfc_get_drvdata(dev);
- if (dev->fw_download_in_progress) {
- dev->fw_download_in_progress = false;
+ if (atomic_xchg(&dev->fw_download_in_progress, 0)) {
return nci_hci_send_cmd(ndev, ST_NCI_DEVICE_MGNT_GATE,
ST_NCI_HCI_DM_LOAD, data, data_len, NULL);
}
diff --git a/include/net/nfc/nfc.h b/include/net/nfc/nfc.h
index bcafab5c53e5..f6cb17f46ca7 100644
--- a/include/net/nfc/nfc.h
+++ b/include/net/nfc/nfc.h
@@ -11,6 +11,7 @@
#ifndef __NET_NFC_H
#define __NET_NFC_H
+#include <linux/atomic.h>
#include <linux/nfc.h>
#include <linux/device.h>
#include <linux/skbuff.h>
@@ -172,7 +173,7 @@ struct nfc_dev {
int targets_generation;
struct device dev;
bool dev_up;
- bool fw_download_in_progress;
+ atomic_t fw_download_in_progress;
u8 rf_mode;
bool polling;
struct nfc_target *active_target;
diff --git a/net/nfc/core.c b/net/nfc/core.c
index f521669293f0..1f335fa7f6f0 100644
--- a/net/nfc/core.c
+++ b/net/nfc/core.c
@@ -15,6 +15,7 @@
#include <linux/slab.h>
#include <linux/rfkill.h>
#include <linux/nfc.h>
+#include <linux/string.h>
#include <net/genetlink.h>
@@ -53,10 +54,14 @@ int nfc_fw_download(struct nfc_dev *dev, const char *firmware_name)
goto error;
}
- dev->fw_download_in_progress = true;
+ if (atomic_cmpxchg_acquire(&dev->fw_download_in_progress, 0, 1)) {
+ rc = -EBUSY;
+ goto error;
+ }
+
rc = dev->ops->fw_download(dev, firmware_name);
if (rc)
- dev->fw_download_in_progress = false;
+ atomic_set_release(&dev->fw_download_in_progress, 0);
error:
device_unlock(&dev->dev);
@@ -73,9 +78,12 @@ error:
int nfc_fw_download_done(struct nfc_dev *dev, const char *firmware_name,
u32 result)
{
- dev->fw_download_in_progress = false;
+ char name[NFC_FIRMWARE_NAME_MAXSIZE + 1];
+
+ strscpy(name, firmware_name, sizeof(name));
+ atomic_set_release(&dev->fw_download_in_progress, 0);
- return nfc_genl_fw_download_done(dev, firmware_name, result);
+ return nfc_genl_fw_download_done(dev, name, result);
}
EXPORT_SYMBOL(nfc_fw_download_done);
@@ -104,7 +112,7 @@ int nfc_dev_up(struct nfc_dev *dev)
goto error;
}
- if (dev->fw_download_in_progress) {
+ if (atomic_read_acquire(&dev->fw_download_in_progress)) {
rc = -EBUSY;
goto error;
}
@@ -1073,6 +1081,7 @@ struct nfc_dev *nfc_allocate_device(const struct nfc_ops *ops,
dev->dev.class = &nfc_class;
dev_set_name(&dev->dev, "nfc%d", dev->idx);
device_initialize(&dev->dev);
+ atomic_set(&dev->fw_download_in_progress, 0);
dev->ops = ops;
dev->supported_protocols = supported_protocols;
--
2.43.0
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH net v6 6/6] nfc: nfcmrvl: stop firmware work before unregistering
2026-10-21 3:24 [PATCH net v6 0/6] nfc: fix NCI UART and firmware teardown races Shubham Antil
` (4 preceding siblings ...)
2026-10-21 3:24 ` [PATCH net v6 5/6] nfc: serialize firmware download state Shubham Antil
@ 2026-10-21 3:24 ` Shubham Antil
5 siblings, 0 replies; 7+ messages in thread
From: Shubham Antil @ 2026-10-21 3:24 UTC (permalink / raw)
To: netdev; +Cc: David Heidelberg, oe-linux-nfc, gio, stable
nfcmrvl_nci_unregister_dev() destroys the NCI workqueues before draining
the firmware receive worker. That worker can still submit NCI commands,
while the later abort can release firmware state that it is processing.
The firmware timeout can also complete the same download in parallel with
the receive worker.
Add active and stopping states protected by the firmware receive lock.
Once completion or teardown starts, reject new firmware receives and wait
for active receive entry points. Shut down the firmware timer, cancel the
receive worker, and purge its queue before releasing firmware state.
Perform this shutdown before unregistering the NCI device so its
command workqueues remain available until the firmware worker has finished.
Queue timeout completion on the same single-thread workqueue and claim
completion under the receive lock so download state is released once. Use
a local copy of the firmware name while reporting completion so a following
request cannot change the notification. Unregister the NCI device before
withdrawing the UART back-pointers so core admission is closed first.
Process one receive frame per work invocation so queued timeout work cannot
be starved by a continuously replenished receive queue.
Initialize the firmware timer with the receive workqueue so the shutdown
path is valid even when a download was never started. Serialize the
stopping transition with firmware start through the NFC device lock.
Use non-synchronous timeout-work cancellation because completion can run
from the timeout work itself.
OpenAI Codex, gpt-5.6-sol assisted a source audit of the UART teardown
path.
Fixes: 3194c6870158 ("NFC: nfcmrvl: add firmware download support")
Cc: stable@vger.kernel.org
Assisted-by: LLM KASAN
Signed-off-by: Shubham Antil <shubham@octane.security>
---
drivers/nfc/nfcmrvl/fw_dnld.c | 199 +++++++++++++++++++++++++---------
drivers/nfc/nfcmrvl/fw_dnld.h | 8 ++
drivers/nfc/nfcmrvl/main.c | 5 +-
3 files changed, 156 insertions(+), 56 deletions(-)
diff --git a/drivers/nfc/nfcmrvl/fw_dnld.c b/drivers/nfc/nfcmrvl/fw_dnld.c
index 8b9d5257320d..d3dec74a333d 100644
--- a/drivers/nfc/nfcmrvl/fw_dnld.c
+++ b/drivers/nfc/nfcmrvl/fw_dnld.c
@@ -92,11 +92,29 @@ static struct sk_buff *alloc_lc_skb(struct nfcmrvl_private *priv, uint8_t plen)
static void fw_dnld_over(struct nfcmrvl_private *priv, u32 error)
{
- if (priv->fw_dnld.fw) {
- release_firmware(priv->fw_dnld.fw);
- priv->fw_dnld.fw = NULL;
- priv->fw_dnld.header = NULL;
- priv->fw_dnld.binary_config = NULL;
+ struct nfcmrvl_fw_dnld *fw_dnld = &priv->fw_dnld;
+ char name[NFC_FIRMWARE_NAME_MAXSIZE + 1];
+ unsigned long flags;
+
+ spin_lock_irqsave(&fw_dnld->rx_lock, flags);
+ if (!fw_dnld->active) {
+ spin_unlock_irqrestore(&fw_dnld->rx_lock, flags);
+ return;
+ }
+ fw_dnld->active = false;
+ spin_unlock_irqrestore(&fw_dnld->rx_lock, flags);
+
+ strscpy(name, fw_dnld->name);
+ wait_event(fw_dnld->rx_wait, !READ_ONCE(fw_dnld->rx_active));
+ timer_delete_sync(&fw_dnld->timer);
+ /* Timeout and receive work share a single-thread workqueue. */
+ cancel_work(&fw_dnld->timeout_work);
+
+ if (fw_dnld->fw) {
+ release_firmware(fw_dnld->fw);
+ fw_dnld->fw = NULL;
+ fw_dnld->header = NULL;
+ fw_dnld->binary_config = NULL;
}
atomic_set(&priv->ndev->cmd_cnt, 0);
@@ -104,8 +122,7 @@ static void fw_dnld_over(struct nfcmrvl_private *priv, u32 error)
if (timer_pending(&priv->ndev->cmd_timer))
timer_delete_sync(&priv->ndev->cmd_timer);
- if (timer_pending(&priv->fw_dnld.timer))
- timer_delete_sync(&priv->fw_dnld.timer);
+ skb_queue_purge(&fw_dnld->rx_q);
nfc_info(priv->dev, "FW loading over (%d)]\n", error);
@@ -114,7 +131,7 @@ static void fw_dnld_over(struct nfcmrvl_private *priv, u32 error)
nfcmrvl_chip_halt(priv);
}
- nfc_fw_download_done(priv->ndev->nfc_dev, priv->fw_dnld.name, error);
+ nfc_fw_download_done(priv->ndev->nfc_dev, name, error);
}
static void fw_dnld_timeout(struct timer_list *t)
@@ -122,6 +139,18 @@ static void fw_dnld_timeout(struct timer_list *t)
struct nfcmrvl_private *priv = timer_container_of(priv, t,
fw_dnld.timer);
+ queue_work(priv->fw_dnld.rx_wq, &priv->fw_dnld.timeout_work);
+}
+
+static void fw_dnld_timeout_work(struct work_struct *work)
+{
+ struct nfcmrvl_fw_dnld *fw_dnld = container_of(work,
+ struct nfcmrvl_fw_dnld,
+ timeout_work);
+ struct nfcmrvl_private *priv = container_of(fw_dnld,
+ struct nfcmrvl_private,
+ fw_dnld);
+
nfc_err(priv->dev, "FW loading timeout");
priv->fw_dnld.state = STATE_RESET;
fw_dnld_over(priv, -ETIMEDOUT);
@@ -395,6 +424,7 @@ static int process_state_boot(struct nfcmrvl_private *priv,
static void fw_dnld_rx_work(struct work_struct *work)
{
+ unsigned long flags;
int ret;
struct sk_buff *skb;
struct nfcmrvl_fw_dnld *fw_dnld = container_of(work,
@@ -404,53 +434,64 @@ static void fw_dnld_rx_work(struct work_struct *work)
struct nfcmrvl_private,
fw_dnld);
- while ((skb = skb_dequeue(&fw_dnld->rx_q))) {
- nfc_send_to_raw_sock(priv->ndev->nfc_dev, skb,
- RAW_PAYLOAD_NCI, NFC_DIRECTION_RX);
- switch (fw_dnld->state) {
- case STATE_RESET:
- ret = process_state_reset(priv, skb);
- break;
- case STATE_INIT:
- ret = process_state_init(priv, skb);
- break;
- case STATE_SET_REF_CLOCK:
- ret = process_state_set_ref_clock(priv, skb);
- break;
- case STATE_SET_HI_CONFIG:
- ret = process_state_set_hi_config(priv, skb);
- break;
- case STATE_OPEN_LC:
- ret = process_state_open_lc(priv, skb);
- break;
- case STATE_FW_DNLD:
- ret = process_state_fw_dnld(priv, skb);
- break;
- case STATE_CLOSE_LC:
- ret = process_state_close_lc(priv, skb);
- break;
- case STATE_BOOT:
- ret = process_state_boot(priv, skb);
- break;
- default:
- ret = -EFAULT;
- }
+ skb = skb_dequeue(&fw_dnld->rx_q);
+ if (!skb)
+ return;
- kfree_skb(skb);
+ nfc_send_to_raw_sock(priv->ndev->nfc_dev, skb,
+ RAW_PAYLOAD_NCI, NFC_DIRECTION_RX);
+ switch (fw_dnld->state) {
+ case STATE_RESET:
+ ret = process_state_reset(priv, skb);
+ break;
+ case STATE_INIT:
+ ret = process_state_init(priv, skb);
+ break;
+ case STATE_SET_REF_CLOCK:
+ ret = process_state_set_ref_clock(priv, skb);
+ break;
+ case STATE_SET_HI_CONFIG:
+ ret = process_state_set_hi_config(priv, skb);
+ break;
+ case STATE_OPEN_LC:
+ ret = process_state_open_lc(priv, skb);
+ break;
+ case STATE_FW_DNLD:
+ ret = process_state_fw_dnld(priv, skb);
+ break;
+ case STATE_CLOSE_LC:
+ ret = process_state_close_lc(priv, skb);
+ break;
+ case STATE_BOOT:
+ ret = process_state_boot(priv, skb);
+ break;
+ default:
+ ret = -EFAULT;
+ }
- if (ret != 0) {
- nfc_err(priv->dev, "FW loading error");
- fw_dnld_over(priv, ret);
- break;
- }
+ kfree_skb(skb);
+
+ if (ret) {
+ nfc_err(priv->dev, "FW loading error");
+ fw_dnld_over(priv, ret);
}
+
+ spin_lock_irqsave(&fw_dnld->rx_lock, flags);
+ if (!fw_dnld->stopping && fw_dnld->active &&
+ !skb_queue_empty(&fw_dnld->rx_q))
+ queue_work(fw_dnld->rx_wq, &fw_dnld->rx_work);
+ spin_unlock_irqrestore(&fw_dnld->rx_lock, flags);
}
int nfcmrvl_fw_dnld_init(struct nfcmrvl_private *priv)
{
char name[32];
+ spin_lock_init(&priv->fw_dnld.rx_lock);
+ init_waitqueue_head(&priv->fw_dnld.rx_wait);
INIT_WORK(&priv->fw_dnld.rx_work, fw_dnld_rx_work);
+ INIT_WORK(&priv->fw_dnld.timeout_work, fw_dnld_timeout_work);
+ timer_setup(&priv->fw_dnld.timer, fw_dnld_timeout, 0);
snprintf(name, sizeof(name), "%s_nfcmrvl_fw_dnld_rx_wq",
dev_name(&priv->ndev->nfc_dev->dev));
priv->fw_dnld.rx_wq = create_singlethread_workqueue(name);
@@ -462,12 +503,24 @@ int nfcmrvl_fw_dnld_init(struct nfcmrvl_private *priv)
void nfcmrvl_fw_dnld_deinit(struct nfcmrvl_private *priv)
{
+ nfcmrvl_fw_dnld_abort(priv);
destroy_workqueue(priv->fw_dnld.rx_wq);
}
void nfcmrvl_fw_dnld_recv_frame(struct nfcmrvl_private *priv,
struct sk_buff *skb)
{
+ unsigned long flags;
+
+ spin_lock_irqsave(&priv->fw_dnld.rx_lock, flags);
+ if (priv->fw_dnld.stopping || !priv->fw_dnld.active) {
+ spin_unlock_irqrestore(&priv->fw_dnld.rx_lock, flags);
+ kfree_skb(skb);
+ return;
+ }
+ priv->fw_dnld.rx_active++;
+ spin_unlock_irqrestore(&priv->fw_dnld.rx_lock, flags);
+
/* Discard command timer */
if (timer_pending(&priv->ndev->cmd_timer))
timer_delete_sync(&priv->ndev->cmd_timer);
@@ -475,13 +528,38 @@ void nfcmrvl_fw_dnld_recv_frame(struct nfcmrvl_private *priv,
/* Allow next command */
atomic_set(&priv->ndev->cmd_cnt, 1);
- /* Queue and trigger rx work */
- skb_queue_tail(&priv->fw_dnld.rx_q, skb);
- queue_work(priv->fw_dnld.rx_wq, &priv->fw_dnld.rx_work);
+ spin_lock_irqsave(&priv->fw_dnld.rx_lock, flags);
+ if (!priv->fw_dnld.stopping && priv->fw_dnld.active) {
+ /* Queue and trigger rx work */
+ skb_queue_tail(&priv->fw_dnld.rx_q, skb);
+ queue_work(priv->fw_dnld.rx_wq, &priv->fw_dnld.rx_work);
+ skb = NULL;
+ }
+ priv->fw_dnld.rx_active--;
+ if (!priv->fw_dnld.rx_active &&
+ (priv->fw_dnld.stopping || !priv->fw_dnld.active))
+ wake_up_all(&priv->fw_dnld.rx_wait);
+ spin_unlock_irqrestore(&priv->fw_dnld.rx_lock, flags);
+ kfree_skb(skb);
}
void nfcmrvl_fw_dnld_abort(struct nfcmrvl_private *priv)
{
+ struct nfcmrvl_fw_dnld *fw_dnld = &priv->fw_dnld;
+ unsigned long flags;
+
+ device_lock(&priv->ndev->nfc_dev->dev);
+ spin_lock_irqsave(&fw_dnld->rx_lock, flags);
+ fw_dnld->stopping = true;
+ spin_unlock_irqrestore(&fw_dnld->rx_lock, flags);
+ device_unlock(&priv->ndev->nfc_dev->dev);
+
+ wait_event(fw_dnld->rx_wait, !READ_ONCE(fw_dnld->rx_active));
+ timer_shutdown_sync(&fw_dnld->timer);
+ cancel_work_sync(&fw_dnld->rx_work);
+ cancel_work_sync(&fw_dnld->timeout_work);
+ skb_queue_purge(&fw_dnld->rx_q);
+
fw_dnld_over(priv, -EHOSTDOWN);
}
@@ -489,6 +567,7 @@ int nfcmrvl_fw_dnld_start(struct nci_dev *ndev, const char *firmware_name)
{
struct nfcmrvl_private *priv = nci_get_drvdata(ndev);
struct nfcmrvl_fw_dnld *fw_dnld = &priv->fw_dnld;
+ unsigned long flags;
int res;
if (!priv->support_fw_dnld)
@@ -497,6 +576,13 @@ int nfcmrvl_fw_dnld_start(struct nci_dev *ndev, const char *firmware_name)
if (!firmware_name || !firmware_name[0])
return -EINVAL;
+ spin_lock_irqsave(&fw_dnld->rx_lock, flags);
+ if (fw_dnld->stopping) {
+ spin_unlock_irqrestore(&fw_dnld->rx_lock, flags);
+ return -ENODEV;
+ }
+ spin_unlock_irqrestore(&fw_dnld->rx_lock, flags);
+
strscpy(fw_dnld->name, firmware_name);
/*
@@ -520,6 +606,7 @@ int nfcmrvl_fw_dnld_start(struct nci_dev *ndev, const char *firmware_name)
firmware_name, fw_dnld->header->magic,
fw_dnld->header->phy);
release_firmware(fw_dnld->fw);
+ fw_dnld->fw = NULL;
fw_dnld->header = NULL;
return -EINVAL;
}
@@ -532,11 +619,6 @@ int nfcmrvl_fw_dnld_start(struct nci_dev *ndev, const char *firmware_name)
fw_dnld->binary_config = &fw_dnld->header->firmware;
}
- /* Configure a timer for timeout */
- timer_setup(&priv->fw_dnld.timer, fw_dnld_timeout, 0);
- mod_timer(&priv->fw_dnld.timer,
- jiffies + msecs_to_jiffies(FW_DNLD_TIMEOUT));
-
/* Ronfigure HI to be sure that it is the bootrom values */
priv->if_ops->nci_update_config(priv,
&fw_dnld->header->bootrom.config);
@@ -546,8 +628,19 @@ int nfcmrvl_fw_dnld_start(struct nci_dev *ndev, const char *firmware_name)
/* First, reset the chip */
priv->fw_dnld.state = STATE_RESET;
+
+ spin_lock_irqsave(&fw_dnld->rx_lock, flags);
+ fw_dnld->active = true;
+ spin_unlock_irqrestore(&fw_dnld->rx_lock, flags);
+
nfcmrvl_chip_reset(priv);
+ spin_lock_irqsave(&fw_dnld->rx_lock, flags);
+ if (fw_dnld->active)
+ mod_timer(&fw_dnld->timer,
+ jiffies + msecs_to_jiffies(FW_DNLD_TIMEOUT));
+ spin_unlock_irqrestore(&fw_dnld->rx_lock, flags);
+
/* Now wait for CORE_RESET_NTF or timeout */
return 0;
diff --git a/drivers/nfc/nfcmrvl/fw_dnld.h b/drivers/nfc/nfcmrvl/fw_dnld.h
index 7c4d91b01910..6bd8d580cd6a 100644
--- a/drivers/nfc/nfcmrvl/fw_dnld.h
+++ b/drivers/nfc/nfcmrvl/fw_dnld.h
@@ -8,6 +8,8 @@
#ifndef __NFCMRVL_FW_DNLD_H__
#define __NFCMRVL_FW_DNLD_H__
+#include <linux/spinlock.h>
+#include <linux/wait.h>
#include <linux/workqueue.h>
#define NFCMRVL_FW_MAGIC 0x88888888
@@ -72,7 +74,13 @@ struct nfcmrvl_fw_dnld {
struct workqueue_struct *rx_wq;
struct work_struct rx_work;
+ struct work_struct timeout_work;
struct sk_buff_head rx_q;
+ spinlock_t rx_lock; /* protects stopping, active, and rx_active */
+ wait_queue_head_t rx_wait;
+ unsigned int rx_active;
+ bool stopping;
+ bool active;
struct timer_list timer;
};
diff --git a/drivers/nfc/nfcmrvl/main.c b/drivers/nfc/nfcmrvl/main.c
index 27fadd64155e..86cf65396253 100644
--- a/drivers/nfc/nfcmrvl/main.c
+++ b/drivers/nfc/nfcmrvl/main.c
@@ -185,13 +185,12 @@ void nfcmrvl_nci_unregister_dev(struct nfcmrvl_private *priv)
{
struct nci_dev *ndev = priv->ndev;
+ nfcmrvl_fw_dnld_abort(priv);
nci_unregister_device(ndev);
+
if (priv->if_ops->nci_quiesce)
priv->if_ops->nci_quiesce(priv);
- if (atomic_read(&priv->ndev->nfc_dev->fw_download_in_progress))
- nfcmrvl_fw_dnld_abort(priv);
-
nfcmrvl_fw_dnld_deinit(priv);
nci_free_device(ndev);
--
2.43.0
^ permalink raw reply related [flat|nested] 7+ messages in thread