From: Shubham Antil <shubham@octane.security>
To: netdev@vger.kernel.org
Cc: David Heidelberg <david@ixit.cz>,
oe-linux-nfc@lists.linux.dev, gio@octane.security,
stable@vger.kernel.org
Subject: [PATCH net v6 3/6] nfc: nci: uart: quiesce callbacks during teardown
Date: Wed, 21 Oct 2026 08:54:40 +0530 [thread overview]
Message-ID: <20261021032443.44412-4-shubham@octane.security> (raw)
In-Reply-To: <20261021032443.44412-1-shubham@octane.security>
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
next prev parent reply other threads:[~2026-10-21 3:24 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
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 [this message]
2026-10-21 3:24 ` [PATCH net v6 4/6] nfc: nfcmrvl: publish UART state before registration 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
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20261021032443.44412-4-shubham@octane.security \
--to=shubham@octane.security \
--cc=david@ixit.cz \
--cc=gio@octane.security \
--cc=netdev@vger.kernel.org \
--cc=oe-linux-nfc@lists.linux.dev \
--cc=stable@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox