Linux USB
 help / color / mirror / Atom feed
* [PATCH] usb: typec: rt1719: unregister the port on probe failure
@ 2026-11-04  2:15 Wentao Liang
  2026-11-04  2:30 ` sashiko-bot
  2026-11-04  7:12 ` Krzysztof Kozlowski
  0 siblings, 2 replies; 3+ messages in thread
From: Wentao Liang @ 2026-11-04  2:15 UTC (permalink / raw)
  To: Heikki Krogerus, Greg Kroah-Hartman; +Cc: linux-usb

typec_register_port() returns a port that has to be released with
typec_unregister_port().  probe() stores it in data->port and
rt1719_remove() unregisters it, but when setting up the attach state or
the interrupt enable fails after registration, the error path only drops
the role switch and the fwnode, leaking the registered Type-C port.

Add a label that unregisters the port before the existing cleanup and
use it on both post-registration error paths.

Fixes: 25d29b980912 ("usb: typec: rt1719: Add support for Richtek RT1719")
Cc: stable@vger.kernel.org
Assisted-by: DeepSeek-V4.1-Flash and OpenAI Codex
Signed-off-by: Wentao Liang <vulab@iscas.ac.cn>
---
 drivers/usb/typec/rt1719.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/typec/rt1719.c b/drivers/usb/typec/rt1719.c
index 0b0c23a0b014..61fa865637c7 100644
--- a/drivers/usb/typec/rt1719.c
+++ b/drivers/usb/typec/rt1719.c
@@ -902,13 +902,13 @@ static int rt1719_probe(struct i2c_client *i2c)
 	ret = rt1719_init_attach_state(data);
 	if (ret) {
 		dev_err(&i2c->dev, "Failed to init attach state (%d)\n", ret);
-		goto err_role_put;
+		goto err_unregister_port;
 	}
 
 	ret = rt1719_irq_init(data);
 	if (ret) {
 		dev_err(&i2c->dev, "Failed to init irq\n");
-		goto err_role_put;
+		goto err_unregister_port;
 	}
 
 	fwnode_handle_put(fwnode);
@@ -917,6 +917,8 @@ static int rt1719_probe(struct i2c_client *i2c)
 
 	return 0;
 
+err_unregister_port:
+	typec_unregister_port(data->port);
 err_role_put:
 	usb_role_switch_put(data->role_sw);
 err_fwnode_put:
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] usb: typec: rt1719: unregister the port on probe failure
  2026-11-04  2:15 [PATCH] usb: typec: rt1719: unregister the port on probe failure Wentao Liang
@ 2026-11-04  2:30 ` sashiko-bot
  2026-11-04  7:12 ` Krzysztof Kozlowski
  1 sibling, 0 replies; 3+ messages in thread
From: sashiko-bot @ 2026-11-04  2:30 UTC (permalink / raw)
  To: Wentao Liang; +Cc: linux-usb

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] usb: typec: rt1719: missing typec_unregister_partner() in rt1719_probe()
--

commit aeb856a2c764adde23b2f8df2e3537e93dbc9fee
Author: Wentao Liang <vulab@iscas.ac.cn>

usb: typec: rt1719: unregister the port on probe failure

This commit adds an err_unregister_port error path label in rt1719_probe()
to call typec_unregister_port(data->port) if rt1719_init_attach_state() or
rt1719_irq_init() fails after the Type-C port has been registered.

> diff --git a/drivers/usb/typec/rt1719.c b/drivers/usb/typec/rt1719.c
> index 0b0c23a0b014b..61fa865637c70 100644
> --- a/drivers/usb/typec/rt1719.c
> +++ b/drivers/usb/typec/rt1719.c
> @@ -902,13 +902,13 @@ static int rt1719_probe(struct i2c_client *i2c)
>  	ret = rt1719_init_attach_state(data);
>  	if (ret) {
>  		dev_err(&i2c->dev, "Failed to init attach state (%d)\n", ret);
> -		goto err_role_put;
> +		goto err_unregister_port;
>  	}
>  
>  	ret = rt1719_irq_init(data);
>  	if (ret) {
>  		dev_err(&i2c->dev, "Failed to init irq\n");
> -		goto err_role_put;
> +		goto err_unregister_port;
>  	}
>  
>  	fwnode_handle_put(fwnode);
> @@ -917,6 +917,8 @@ static int rt1719_probe(struct i2c_client *i2c)
>  
>  	return 0;
>  
> +err_unregister_port:
> +	typec_unregister_port(data->port);

[Severity: High]
Does err_unregister_port in drivers/usb/typec/rt1719.c:rt1719_probe() also
need to call typec_unregister_partner(data->partner) before
typec_unregister_port(data->port)?

Before rt1719_irq_init(data) is called, rt1719_init_attach_state(data) runs:

drivers/usb/typec/rt1719.c:rt1719_init_attach_state() {
	...
	if (conn_info & RT1719_ATTACHDEV_MASK)
		rt1719_attach(data);

	if (conn_info & RT1719_PE_EXP_CONTRACT) {
		rt1719_update_source_pdos(data);
		rt1719_update_pwr_opmode(data);
	}
	...
}

If a partner is already attached at probe time, rt1719_attach() or
rt1719_update_pwr_opmode() calls rt1719_register_partner(data), which
registers data->partner as a child device of data->port via
typec_register_partner(data->port, &data->partner_desc) with
partner->identity pointing to &data->partner_ident.

If rt1719_irq_init(data) then fails and jumps to err_unregister_port,
typec_unregister_port(data->port) is called while data->partner is still
registered. Because typec_unregister_port() only calls
device_unregister(&port->dev) and does not unregister child partner devices,
data->partner remains registered in sysfs holding its parent reference on
data->port->dev (leaking both data->partner and data->port), and its
partner->identity pointer dangles into the devm-allocated struct rt1719_data
once rt1719_probe() returns an error.

Since typec_unregister_partner() safely ignores NULL or ERR_PTR pointers,
should typec_unregister_partner(data->partner) be called right before
typec_unregister_port(data->port), matching rt1719_remove()?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261104021514.1468596-1-vulab@iscas.ac.cn?part=1

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] usb: typec: rt1719: unregister the port on probe failure
  2026-11-04  2:15 [PATCH] usb: typec: rt1719: unregister the port on probe failure Wentao Liang
  2026-11-04  2:30 ` sashiko-bot
@ 2026-11-04  7:12 ` Krzysztof Kozlowski
  1 sibling, 0 replies; 3+ messages in thread
From: Krzysztof Kozlowski @ 2026-11-04  7:12 UTC (permalink / raw)
  To: Wentao Liang; +Cc: Heikki Krogerus, Greg Kroah-Hartman, linux-usb


On Wed, 04 Nov 2026 02:15:14 +0000, Wentao Liang wrote:
> typec_register_port() returns a port that has to be released with
> typec_unregister_port().  probe() stores it in data->port and
> rt1719_remove() unregisters it, but when setting up the attach state or
> the interrupt enable fails after registration, the error path only drops
> the role switch and the fwnode, leaking the registered Type-C port.
> 
> Add a label that unregisters the port before the existing cleanup and
> use it on both post-registration error paths.
> 
> Fixes: 25d29b980912 ("usb: typec: rt1719: Add support for Richtek RT1719")
> Cc: stable@vger.kernel.org
> Assisted-by: DeepSeek-V4.1-Flash and OpenAI Codex
> Signed-off-by: Wentao Liang <vulab@iscas.ac.cn>
> ---
>  drivers/usb/typec/rt1719.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 

Multiple things here:
1. Your team ignored completely previous feedback. Multiple times.

2. You use multiple identities with this email address, thus I doubt that we
   speak with an actual person.

3. Your bot does not even bother running checkpatch, right?

4. Nothing of this was reproduced nor tested.

5. Finally, the same feedback:
You sent multiple independent patches, to multiple independent
subsystems. The amount of these patches clearly suggest this was
AI generated and most likely not tested.

More importantly, you sent all this work without properly organizing
relevant patches into patchsets. This makes reviewing difficult
and might cause multiple reviewers to address the same issue.
Replying to the entire set is impossible and requires handling each
patch independently, instead of applying or discarding the set.
Maintainers also won't see the bigger picture of your work. Quite
worrying.

This is on the verge of hostile patch: bomb us with so many
contributions, we won't be able to handle them in efficient manner,
like responding ONCE to ask you to slow down.  Considering all this
is untested and LLM generated, I have even more doubts whether this
should be considered for review.

Please read kernel documentation BEFORE posting more work. It will
explain you how to identify subsystems, how to organize your work per
subsystem (so a patchset grouping multiple patches with a short cover
letter), how to document the issue discovery method and what was
or was not tested.

Best regards,
Krzysztof





^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-11-04  7:12 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-11-04  2:15 [PATCH] usb: typec: rt1719: unregister the port on probe failure Wentao Liang
2026-11-04  2:30 ` sashiko-bot
2026-11-04  7:12 ` Krzysztof Kozlowski

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox