[PATCH v4] usb: typec: tcpci: acknowledge masked alerts and quiesce the IRQ

[PATCH v4] usb: typec: tcpci: acknowledge masked alerts and quiesce the IRQ

[Yaohan Li]

tcpci_irq() reads TCPC_ALERT and uses the raw status for the event
callbacks, while only the IRQ return value is masked with
tcpci->alert_mask.  A masked alert is still latched in TCPC_ALERT even
though it does not assert Alert# (TCPCI spec 4.4.3), so the handler can
run TCPM callbacks for events that the port has masked.  More
importantly, controllers such as RT1711H enable additional alerts
(FAULT and RX_BUF_OVF) in TCPC_ALERT_MASK that are not part of the
generic software mask.  Dropping those bits before the acknowledge
write leaves a level-triggered Alert# asserted and can cause an IRQ
storm.

Acknowledge every non-RX_STATUS bit from the raw alert value, not only
the bits enabled in the software mask, then dispatch only the filtered
status.  RX_STATUS is still cleared only after the message has been
read successfully.

tcpci_remove() unregisters the TCPM port, which destroys its kthread
worker, while the devm-managed threaded IRQ can still be running and
queue work on that worker.  Disable the hardware mask first, so an
alert that fires during the write is still handled by the live TCPM
port; then publish the quiesced software state and use
synchronize_irq() to wait for handlers that already entered the normal
path.  A handler that runs in the quiesced state acknowledges latched
alerts without dispatching them to TCPM; when the alert register can be
read and acknowledged, this prevents a software-induced level-triggered
storm.  If the alert access itself fails, that is the existing
alert-read failure path; this change does not add an unbounded retry.

The IRQ handler can also call tcpm_tcpc_reset(), which calls
tcpci_init() and publishes the normal alert mask again.  Protect the
software mask with alert_mask_lock and an alerts_quiesced state:
tcpci_init() only publishes the mask while alerts are not quiesced.
Apply the same quiesce ordering on the probe error path when the final
ALERT_MASK enable write fails, including an attempt to disable the
hardware mask in case the controller partially accepted the failed
write.

The masked-status handling and the unquiesced teardown date back to the
original TCPCI driver.  The TCPM port worker was later migrated to
kthread_work, and the probe IRQ ordering was changed separately; the
Fixes tags record both origins.

A KASAN-enabled test reproduces a use-after-free when the IRQ
handler races with TCPM port removal. With this change, teardown
waits for the handler and no use-after-free is reported.

Fault-injection tests also cover masked alerts, reset during
removal, and failure to update the hardware alert mask. Real TCPCI
hardware and PD traffic remain untested.

Fixes: 74e656d6b055 ("staging: typec: Type-C Port Controller Interface driver (tcpci)")
Fixes: 862a9c0f6848 ("usb: typec: tcpci: fix NULL pointer issue on shared irq case")
Signed-off-by: Yaohan Li <shunsheng1218@gmail.com>
---
Changes in v4:
- Acknowledge every non-RX_STATUS bit from the raw alert value before
  filtering; masked hardware alerts (e.g. RT1711H FAULT/RX_BUF_OVF) are
  cleared without being dispatched to TCPM.
- Add a quiesced IRQ path that acknowledges latched alerts without
  touching TCPM.  Disable the hardware mask first, then quiesce the
  software state, then synchronize_irq(); a failed hardware mask write
  does not leave a level-triggered line storming when the alert
  register can still be read and acknowledged.
- Attempt to disable the hardware mask on the probe enable-mask failure
  path as well, before quiescing.
- Handle ALERT read/acknowledge failures in the quiesced path without
  dispatching to TCPM; do not add an unbounded retry.
- Keep alert_mask_lock/alerts_quiesced so tcpci_init() cannot restore
  the mask during teardown.
- Update tests: RT1711H-style masked FAULT, masked TX acknowledgment,
  quiesced shared IRQ acknowledgment, probe partial mask acceptance,
  quiesced ALERT read/write failure; rerun reset-race, inject,
  postsync, maskfail, probefail, CC/TX/RX.

AI tools assisted with source review, patch iteration and test harness
development. The author reviewed the resulting code and test results.

v3: https://lore.kernel.org/r/20261112084014.2564834-1-shunsheng1218@gmail.com/
v2: https://lore.kernel.org/r/20261106091725.1334097-1-shunsheng1218@gmail.com/
v1: https://lore.kernel.org/r/179378589227.74015.8259496684787732118@gmail.com/

Test details (not part of the commit log):
- Unpatched: a handler parked in tcpm_cc_change()/kthread_queue_work()
  while the device is unbound produces a slab-use-after-free on the
  destroyed TCPM worker.
- Patched: synchronize_irq() waits for the parked handler; the handler
  queues on the still-live worker, then the worker is destroyed; no
  report.
- Normal CC, TX and RX events still reach the corresponding TCPM
  callbacks.
- Masked/unmasked mix: with TX_SUCCESS masked and CC unmasked in the
  same ALERT, only tcpm_cc_change() runs; the masked TX bit is
  acknowledged and cleared, not left latched.
- RT1711H-style masked FAULT: FAULT is acknowledged from the raw
  status, CC is dispatched, and the alert register is left clear.
- Shared IRQ after synchronize_irq(), with a stale alert: the quiesced
  handler acknowledges it, returns IRQ_HANDLED, and does not call
  TCPM.
- Hardware ALERT_MASK = 0 write forced to fail during removal, with a
  continuous alert stream: the quiesced handler acknowledges events
  but no TCPM callback runs after the worker is destroyed.
- Probe final enable-mask failure: quiesce before unregistering.
- Reset-race: the handler is parked in tcpci_init() while remove
  quiesces and waits; the guarded code keeps alert_mask at zero, a
  shared IRQ is acknowledged without TCPM, and no report appears.
- Probe final enable-mask failure with the hardware mask left non-zero:
  the error cleanup writes the mask back to zero before quiescing.
- Quiesced handler ALERT read failure returns IRQ_NONE; acknowledgment
  write failure returns IRQ_HANDLED; neither path dispatches to TCPM or
  touches the destroyed worker.

 drivers/usb/typec/tcpm/tcpci.c | 94 ++++++++++++++++++++++++++++++----
 1 file changed, 83 insertions(+), 11 deletions(-)

diff --git a/drivers/usb/typec/tcpm/tcpci.c b/drivers/usb/typec/tcpm/tcpci.c
index 6717ac914c6a6..54a23776e838d 100644
--- a/drivers/usb/typec/tcpm/tcpci.c
+++ b/drivers/usb/typec/tcpm/tcpci.c
@@ -39,6 +39,8 @@ struct tcpci {
 	struct regmap *regmap;
 	unsigned int alert_mask;
 	unsigned int rx_type_mask;
+	spinlock_t alert_mask_lock; /* protects alert_mask and alerts_quiesced */
+	bool alerts_quiesced;
 
 	bool controls_vbus;
 
@@ -648,10 +650,21 @@ static bool tcpci_attempt_vconn_swap_discovery(struct tcpc_dev *tcpc)
 	return false;
 }
 
+static void tcpci_quiesce_alerts(struct tcpci *tcpci)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave(&tcpci->alert_mask_lock, flags);
+	tcpci->alerts_quiesced = true;
+	WRITE_ONCE(tcpci->alert_mask, 0);
+	spin_unlock_irqrestore(&tcpci->alert_mask_lock, flags);
+}
+
 static int tcpci_init(struct tcpc_dev *tcpc)
 {
 	struct tcpci *tcpci = tcpc_to_tcpci(tcpc);
 	unsigned long timeout = jiffies + msecs_to_jiffies(2000); /* XXX */
+	unsigned long flags;
 	unsigned int reg;
 	int ret;
 
@@ -710,29 +723,60 @@ static int tcpci_init(struct tcpc_dev *tcpc)
 			return ret;
 	}
 
-	tcpci->alert_mask = reg;
+	spin_lock_irqsave(&tcpci->alert_mask_lock, flags);
+	if (!tcpci->alerts_quiesced)
+		WRITE_ONCE(tcpci->alert_mask, reg);
+	spin_unlock_irqrestore(&tcpci->alert_mask_lock, flags);
 
 	return 0;
 }
 
 irqreturn_t tcpci_irq(struct tcpci *tcpci)
 {
-	u16 status;
+	u16 status = 0;
 	int ret;
-	int irq_ret;
+	int irq_ret = 0;
 	unsigned int raw;
 
-	tcpci_read16(tcpci, TCPC_ALERT, &status);
-	irq_ret = status & tcpci->alert_mask;
+	/*
+	 * During teardown, acknowledge a latched alert without dispatching
+	 * it to TCPM.  A level-triggered Alert# must not keep retriggering
+	 * after the software mask was cleared or the hardware mask write
+	 * failed; the port may already be going away.
+	 */
+	if (READ_ONCE(tcpci->alerts_quiesced)) {
+		ret = tcpci_read16(tcpci, TCPC_ALERT, &status);
+		if (ret < 0)
+			return IRQ_NONE;
+		if (status) {
+			ret = tcpci_write16(tcpci, TCPC_ALERT, status);
+			if (ret < 0)
+				return IRQ_HANDLED;
+			irq_ret = 1;
+		}
+		return IRQ_RETVAL(irq_ret);
+	}
+
+	if (!READ_ONCE(tcpci->alert_mask))
+		return IRQ_NONE;
 
-process_status:
 	/*
-	 * Clear alert status for everything except RX_STATUS, which shouldn't
-	 * be cleared until we have successfully retrieved message.
+	 * Acknowledge every non-RX_STATUS bit from the raw alert value, not
+	 * only the bits enabled in the software mask.  Some controllers
+	 * enable additional alerts in TCPC_ALERT_MASK (for example RT1711H
+	 * enables FAULT and RX_BUF_OVF); dropping those bits before the
+	 * write would leave a level-triggered Alert# asserted.  RX_STATUS is
+	 * cleared only after the message has been read successfully.
 	 */
+	tcpci_read16(tcpci, TCPC_ALERT, &status);
 	if (status & ~TCPC_ALERT_RX_STATUS)
 		tcpci_write16(tcpci, TCPC_ALERT,
 			      status & ~TCPC_ALERT_RX_STATUS);
+	irq_ret = status;
+
+	status &= READ_ONCE(tcpci->alert_mask);
+
+process_status:
 
 	if (status & TCPC_ALERT_CC_STATUS)
 		tcpm_cc_change(tcpci->port);
@@ -806,8 +850,13 @@ irqreturn_t tcpci_irq(struct tcpci *tcpci)
 		tcpm_pd_transmit_complete(tcpci->port, TCPC_TX_FAILED);
 
 	tcpci_read16(tcpci, TCPC_ALERT, &status);
+	if (status & ~TCPC_ALERT_RX_STATUS)
+		tcpci_write16(tcpci, TCPC_ALERT,
+			      status & ~TCPC_ALERT_RX_STATUS);
+	irq_ret |= status;
 
-	if (status & tcpci->alert_mask)
+	status &= READ_ONCE(tcpci->alert_mask);
+	if (status)
 		goto process_status;
 
 	return IRQ_RETVAL(irq_ret);
@@ -854,6 +903,7 @@ struct tcpci *tcpci_register_port(struct device *dev, struct tcpci_data *data)
 	tcpci->dev = dev;
 	tcpci->data = data;
 	tcpci->regmap = data->regmap;
+	spin_lock_init(&tcpci->alert_mask_lock);
 
 	tcpci->tcpc.init = tcpci_init;
 	tcpci->tcpc.get_vbus = tcpci_get_vbus;
@@ -972,12 +1022,23 @@ static int tcpci_probe(struct i2c_client *client)
 	/* Enable chip interrupts at last */
 	err = tcpci_write16(chip->tcpci, TCPC_ALERT_MASK, chip->tcpci->alert_mask);
 	if (err < 0)
-		goto unregister_port;
+		goto quiesce_irq;
 
 	device_set_wakeup_capable(chip->tcpci->dev, true);
 
 	return 0;
 
+quiesce_irq:
+	/*
+	 * The enable write failed, but the controller may have accepted it
+	 * partially or kept a non-zero mask.  Try to disable the hardware
+	 * mask as well; keep the original probe error.
+	 */
+	if (tcpci_write16(chip->tcpci, TCPC_ALERT_MASK, 0) < 0)
+		dev_warn(&client->dev,
+			 "Failed to disable alerts during probe cleanup\n");
+	tcpci_quiesce_alerts(chip->tcpci);
+	synchronize_irq(client->irq);
 unregister_port:
 	tcpci_unregister_port(chip->tcpci);
 	return err;
@@ -988,10 +1049,21 @@ static void tcpci_remove(struct i2c_client *client)
 	struct tcpci_chip *chip = i2c_get_clientdata(client);
 	int err;
 
-	/* Disable chip interrupts before unregistering port */
+	/*
+	 * Disable the hardware mask first: an alert that fires while this
+	 * write is in progress is still handled by the live TCPM port.
+	 * Then publish the quiesced software state.  If the hardware write
+	 * failed, tcpci_irq() still acknowledges latched alerts in the
+	 * quiesced state without dispatching them to TCPM; when the alert
+	 * register can be read and acknowledged, this prevents a software
+	 * induced level-triggered storm.  synchronize_irq() waits for
+	 * handlers that already entered the normal path.
+	 */
 	err = tcpci_write16(chip->tcpci, TCPC_ALERT_MASK, 0);
 	if (err < 0)
 		dev_warn(&client->dev, "Failed to disable irqs (%pe)\n", ERR_PTR(err));
+	tcpci_quiesce_alerts(chip->tcpci);
+	synchronize_irq(client->irq);
 
 	tcpci_unregister_port(chip->tcpci);
 }
-- 
2.34.1

Re: [PATCH v4] usb: typec: tcpci: acknowledge masked alerts and quiesce the IRQ

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] usb: typec: tcpci: race in tcpci_irq() with
  tcpci_quiesce_alerts() skips alert ack
--

commit b4f078603381d8a805defdf53c50acdc29717943
Author: Yaohan Li <shunsheng1218@gmail.com>

usb: typec: tcpci: acknowledge masked alerts and quiesce the IRQ

This commit acknowledges all non-RX_STATUS alert bits before filtering
against the software alert_mask, and quiesces and synchronizes the IRQ
during teardown and probe error cleanup before unregistering the TCPM
port.

> diff --git a/drivers/usb/typec/tcpm/tcpci.c b/drivers/usb/typec/tcpm/tcpci.c
> index c6165466c54b6..b11c30e67055f 100644
> --- a/drivers/usb/typec/tcpm/tcpci.c
> +++ b/drivers/usb/typec/tcpm/tcpci.c

[ ... ]

> @@ -648,10 +650,21 @@ static bool tcpci_attempt_vconn_swap_discovery(struct tcpc_dev *tcpc)
>  	return false;
>  }
>  
> +static void tcpci_quiesce_alerts(struct tcpci *tcpci)
> +{
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&tcpci->alert_mask_lock, flags);
> +	tcpci->alerts_quiesced = true;
> +	WRITE_ONCE(tcpci->alert_mask, 0);
> +	spin_unlock_irqrestore(&tcpci->alert_mask_lock, flags);
> +}

[ ... ]

> @@ -710,29 +723,60 @@ static int tcpci_init(struct tcpc_dev *tcpc)

[ ... ]

>  irqreturn_t tcpci_irq(struct tcpci *tcpci)
>  {
> -	u16 status;
> +	u16 status = 0;
>  	int ret;
> -	int irq_ret;
> +	int irq_ret = 0;
>  	unsigned int raw;
>  
> -	tcpci_read16(tcpci, TCPC_ALERT, &status);
> -	irq_ret = status & tcpci->alert_mask;
> +	/*
> +	 * During teardown, acknowledge a latched alert without dispatching
> +	 * it to TCPM.  A level-triggered Alert# must not keep retriggering
> +	 * after the software mask was cleared or the hardware mask write
> +	 * failed; the port may already be going away.
> +	 */
> +	if (READ_ONCE(tcpci->alerts_quiesced)) {
> +		ret = tcpci_read16(tcpci, TCPC_ALERT, &status);
> +		if (ret < 0)
> +			return IRQ_NONE;
> +		if (status) {
> +			ret = tcpci_write16(tcpci, TCPC_ALERT, status);
> +			if (ret < 0)
> +				return IRQ_HANDLED;
> +			irq_ret = 1;
> +		}
> +		return IRQ_RETVAL(irq_ret);
> +	}
> +
> +	if (!READ_ONCE(tcpci->alert_mask))
> +		return IRQ_NONE;
>  
> -process_status:
>  	/*
> -	 * Clear alert status for everything except RX_STATUS, which shouldn't
> -	 * be cleared until we have successfully retrieved message.
> +	 * Acknowledge every non-RX_STATUS bit from the raw alert value, not
> +	 * only the bits enabled in the software mask.  Some controllers
> +	 * enable additional alerts in TCPC_ALERT_MASK (for example RT1711H
> +	 * enables FAULT and RX_BUF_OVF); dropping those bits before the
> +	 * write would leave a level-triggered Alert# asserted.  RX_STATUS is
> +	 * cleared only after the message has been read successfully.
>  	 */
> +	tcpci_read16(tcpci, TCPC_ALERT, &status);
>  	if (status & ~TCPC_ALERT_RX_STATUS)
>  		tcpci_write16(tcpci, TCPC_ALERT,
>  			      status & ~TCPC_ALERT_RX_STATUS);
> +	irq_ret = status;
> +
> +	status &= READ_ONCE(tcpci->alert_mask);
> +
> +process_status:

[ ... ]

> @@ -806,8 +850,13 @@ irqreturn_t tcpci_irq(struct tcpci *tcpci)
>  		tcpm_pd_transmit_complete(tcpci->port, TCPC_TX_FAILED);
>  
>  	tcpci_read16(tcpci, TCPC_ALERT, &status);
> +	if (status & ~TCPC_ALERT_RX_STATUS)
> +		tcpci_write16(tcpci, TCPC_ALERT,
> +			      status & ~TCPC_ALERT_RX_STATUS);
> +	irq_ret |= status;
>  
> -	if (status & tcpci->alert_mask)
> +	status &= READ_ONCE(tcpci->alert_mask);
> +	if (status)
>  		goto process_status;
>  
>  	return IRQ_RETVAL(irq_ret);

[Severity: Medium]
Can tcpci_irq() race with tcpci_quiesce_alerts() and return without
acknowledging TCPC_ALERT?

CPU0                              CPU1
tcpci_irq()
  READ_ONCE(tcpci->alerts_quiesced)
    -> false
                                  tcpci_remove()
                                    tcpci_quiesce_alerts()
                                      tcpci->alerts_quiesced = true;
                                      WRITE_ONCE(tcpci->alert_mask, 0);
  if (!READ_ONCE(tcpci->alert_mask))
    return IRQ_NONE;              <-- skips TCPC_ALERT read and ack

Because tcpci_irq() reads tcpci->alerts_quiesced and tcpci->alert_mask
in separate unlocked steps, a concurrent tcpci_quiesce_alerts() can zero
tcpci->alert_mask after the quiesced check has already passed.

A second window exists just below and in the loop tail before jumping to
process_status: if tcpci_irq() reads TCPC_ALERT with TCPC_ALERT_RX_STATUS
set, the upfront write clears only status & ~TCPC_ALERT_RX_STATUS, and if
tcpci_quiesce_alerts() zeroes tcpci->alert_mask before
status &= READ_ONCE(tcpci->alert_mask), TCPC_ALERT_RX_STATUS is masked
out and never cleared in the RX handler.  Similarly, if tcpci_init()
fails before publishing tcpci->alert_mask (tcpm_init() ignores the
return value of port->tcpc->init()), tcpci->alert_mask remains 0 and
tcpci_irq() returns IRQ_NONE without acknowledging hardware alerts on
subdrivers such as tcpci_rt1711h that program TCPC_ALERT_MASK
independently.  In all of these cases, a latched alert can keep a
level-triggered Alert# line asserted and cause an interrupt storm or
spurious IRQ disablement.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261113084928.211885-1-shunsheng1218@gmail.com?part=1