From: Vladimir Oltean <olteanv@gmail.com>
To: David Carlier <devnexen@gmail.com>
Cc: sven@kernel.org, j@jannau.net, neal@gompa.dev, vkoul@kernel.org,
neil.armstrong@linaro.org, marcan@marcan.st,
p.zabel@pengutronix.de, asahi@lists.linux.dev,
linux-arm-kernel@lists.infradead.org,
linux-phy@lists.infradead.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2] phy: apple: atc: Fix typec switch/mux leak on unbind
Date: Fri, 8 May 2026 23:01:11 +0300 [thread overview]
Message-ID: <20260508200111.kfl2a6u6gzacsvu4@skbuf> (raw)
In-Reply-To: <20260507163746.108086-1-devnexen@gmail.com>
On Thu, May 07, 2026 at 05:37:46PM +0100, David Carlier wrote:
> atcphy_probe_switch() and atcphy_probe_mux() discard the pointers
> returned by typec_switch_register() and typec_mux_register(). The
> platform driver has no .remove callback, so when the driver unbinds
> (e.g. via sysfs unbind) neither typec_switch_unregister() nor
> typec_mux_unregister() is called. The framework reference taken in
> typec_switch_register() (device_initialize() + device_add() in
> drivers/usb/typec/mux.c) is therefore never dropped and the
> typec_switch_dev / typec_mux_dev objects stay live forever, with
> their sysfs entries under the typec_mux class also left behind. A
> subsequent rebind cannot recreate them with the same fwnode-derived
> name.
>
> Save the registered handles and unregister them through
> devm_add_action_or_reset() so framework registration is torn down
> in step with the driver's other devm-managed state. While here,
> drop struct apple_atcphy::sw and ::mux: they were declared with the
> consumer-side types (typec_switch *, typec_mux *) instead of the
> provider-side types and were never assigned.
>
> Scope of the fix
> ----------------
> This patch fixes the registration leak only. It does not close the
> use-after-free window that arises when a consumer that obtained a
> reference via fwnode_typec_switch_get() / fwnode_typec_mux_get()
> outlives the provider unbind: such consumers keep the underlying
> typec_switch_dev / typec_mux_dev alive past device_unregister(),
> and a later typec_switch_set() / typec_mux_set() still invokes the
> registered atcphy_sw_set() / atcphy_mux_set(), which dereferences
> the freed apple_atcphy through typec_{switch,mux}_get_drvdata().
>
> On Apple Silicon the relevant consumers are the typec port and the
> cd321x controller registered by drivers/usb/typec/tipd/core.c.
> Cable plug / orientation events and alt-mode transitions trigger
> the .set callbacks via:
>
> tps6598x_interrupt() drivers/usb/typec/tipd/core.c
> tps6598x_handle_plug_event()
> tps6598x_connect()/_disconnect()
> typec_set_orientation() drivers/usb/typec/class.c
> typec_switch_set(port->sw) drivers/usb/typec/mux.c
> atcphy_sw_set() drivers/phy/apple/atc.c
>
> cd321x_update_work() drivers/usb/typec/tipd/core.c
> cd321x_typec_update_mode()
> typec_mux_set(cd321x->mux) drivers/usb/typec/mux.c
> atcphy_mux_set() drivers/phy/apple/atc.c
Ok, so the claim from v1 that this patch fixes crashes from these
code paths is not correct, since there is nothing that would make the
typec port drop its references acquired via typec_switch_get() and
typec_mux_get().
> Closing that window requires framework support for invalidating
> consumer-held references on provider unbind. The same
> consumer-survives-provider pattern has been discussed for the PHY
> framework [1] and is out of scope here.
>
> [1] https://lore.kernel.org/linux-phy/aZejMSJ9qqRWb2pX@google.com/
>
> Fixes: 8e98ca1e74db ("phy: apple: Add Apple Type-C PHY")
> Signed-off-by: David Carlier <devnexen@gmail.com>
> ---
The commit message is much better. But there is a checkpatch issue which
appears to be valid, see:
commit 931d5c36c7369b65adb9e3d197a8d3a8a913db8c
Author: Joe Perches <joe@perches.com>
Date: Fri Jan 16 09:42:52 2026 -0800
checkpatch: add an invalid patch separator test
Some versions of tools that apply patches incorrectly allow lines that
start with 3 dashes and have additional content on the same line.
Checkpatch will now emit an ERROR on these lines and optionally convert
those lines from dashes to equals with --fix.
Link: https://lkml.kernel.org/r/6ec1ed08328340db42655287afd5fa4067316b11.camel@perches.com
Signed-off-by: Joe Perches <joe@perches.com>
Suggested-by: Ian Rogers <irogers@google.com>
Cc: Andy Whitcroft <apw@canonical.com>
Cc: Dwaipayan Ray <dwaipayanray1@gmail.com>
Cc: Kuan-Wei Chiu <visitorckw@gmail.com>
Cc: Lukas Bulwahn <lukas.bulwahn@gmail.com>
Cc: Namhyung kim <namhyung@kernel.org>
Cc: Stehen Rothwell <sfr@canb.auug.org.au>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
I don't have such tooling (git am from version 2.43.0 applies the patch
without discarding the text beneath "Scope of the fix" just fine), but
the commit is from Jan 2026, so that tooling must still exist somewhere.
So please resent with different formatting somehow (either a space
before the title, or replace the ---- with ==== or ~~~~, whatever).
With that addressed, please add:
Reviewed-by: Vladimir Oltean <olteanv@gmail.com>
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
next prev parent reply other threads:[~2026-05-08 20:01 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-05-07 16:37 [PATCH v2] phy: apple: atc: Fix typec switch/mux leak on unbind David Carlier
2026-05-08 20:01 ` Vladimir Oltean [this message]
2026-05-08 20:19 ` [PATCH v3] " David Carlier
2026-05-08 23:06 ` Joshua Peisach
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=20260508200111.kfl2a6u6gzacsvu4@skbuf \
--to=olteanv@gmail.com \
--cc=asahi@lists.linux.dev \
--cc=devnexen@gmail.com \
--cc=j@jannau.net \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-phy@lists.infradead.org \
--cc=marcan@marcan.st \
--cc=neal@gompa.dev \
--cc=neil.armstrong@linaro.org \
--cc=p.zabel@pengutronix.de \
--cc=sven@kernel.org \
--cc=vkoul@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