Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.35 as permitted sender) client-ip=23.128.96.35;
Date:   Tue, 21 Nov 2023 20:39:55 +0000
Mime-Version: 1.0
Message-ID: <20231121203954.173364-2-rdbabiera@google.com>
Subject: [PATCH v1] usb: typec: class: fix typec_altmode_put_partner to put plugs
From:   RD Babiera <rdbabiera@google.com>
To:     heikki.krogerus@linux.intel.com, gregkh@linuxfoundation.org
Cc:     linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org,
        badhri@google.com, RD Babiera <rdbabiera@google.com>,
        stable@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

When releasing an Alt Mode, typec_altmode_release called by a plug device
will not release the plug Alt Mode, meaning that a port will hold a
reference to a plug Alt Mode even if the port partner is unregistered.
As a result, typec_altmode_get_plug() can return an old plug altmode.

Currently, typec_altmode_put_partner does not raise issues
when unregistering a partner altmode. Looking at the current
implementation:

> static void typec_altmode_put_partner(struct altmode *altmode)
> {
>	struct altmode *partner = altmode->partner;

When called by the partner Alt Mode, then partner evaluates to the port's
Alt Mode. When called by the plug Alt Mode, this also evaluates to the
port's Alt Mode.

>	struct typec_altmode *adev;
>
>	if (!partner)
>		return;
>
>	adev = &partner->adev;

This always evaluates to the port's typec_altmode

>	if (is_typec_plug(adev->dev.parent)) {
>		struct typec_plug *plug = to_typec_plug(adev->dev.parent);
>
>		partner->plug[plug->index] = NULL;

If the routine is called to put the plug's Alt mode and altmode refers to
the plug, then adev referring to the port can never be a typec_plug. If
altmode refers to the port, adev will always refer to the port partner,
which runs the block below.

>	} else {
>		partner->partner = NULL;
>	}
>	put_device(&adev->dev);
> }

When calling typec_altmode_set_partner, a registration always calls
get_device() on the port partner or the plug being registered, therefore
typec_altmode_put_partner should put_device() the same device. By changing
adev to altmode->adev, we make sure to put the correct device and properly
unregister plugs. The reason port partners are always properly
unregistered is because even when adev refers to the port, the port
partner gets nullified in the else block. The port device currently gets
put().

Fixes: 8a37d87d72f0 ("usb: typec: Bus type for alternate modes")
Cc: stable@vger.kernel.org
Signed-off-by: RD Babiera <rdbabiera@google.com>
---
 drivers/usb/typec/class.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
index 2e0451bd336e..803be1943445 100644
--- a/drivers/usb/typec/class.c
+++ b/drivers/usb/typec/class.c
@@ -267,7 +267,7 @@ static void typec_altmode_put_partner(struct altmode *altmode)
 	if (!partner)
 		return;
 
-	adev = &partner->adev;
+	adev = &altmode->adev;
 
 	if (is_typec_plug(adev->dev.parent)) {
 		struct typec_plug *plug = to_typec_plug(adev->dev.parent);

base-commit: b85ea95d086471afb4ad062012a4d73cd328fa86
-- 
2.43.0.rc1.413.gea7ed67945-goog