[PATCH] soc: mediatek: SVS: fix device reference leak in svs_add_device_link()

[PATCH] soc: mediatek: SVS: fix device reference leak in svs_add_device_link()

[Coia Prant]

svs_add_device_link() calls svs_get_subsys_device(), which returns a
device with a reference taken by of_find_device_by_node(). If either of
the subsequent checks fails, the function returns an error pointer
without dropping that reference, and the caller only checks IS_ERR() and
bails out, so the reference is leaked on every -EPROBE_DEFER.

Both error paths can be hit in practice: device_link_add() returns NULL
when the link would create a reverse dependency or on allocation
failure, and the supplier status check fails whenever the supplier has
not bound yet.

Drop the reference before returning on both paths.

While at it, note that the return value of device_link_add() is only
used to detect failure. Managed links are owned by the driver core, so
the driver must not keep or dereference the struct device_link pointer
to inspect the supplier state; read dev->links.status directly instead.

Fixes: 681a02e95000 ("soc: mediatek: SVS: introduce MTK SVS engine")
Cc: stable@vger.kernel.org
Signed-off-by: Coia Prant <coiaprant@gmail.com>
---
 drivers/soc/mediatek/mtk-svs.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/soc/mediatek/mtk-svs.c b/drivers/soc/mediatek/mtk-svs.c
index 99edecb204f25..fecb09b9185c2 100644
--- a/drivers/soc/mediatek/mtk-svs.c
+++ b/drivers/soc/mediatek/mtk-svs.c
@@ -2145,21 +2145,21 @@ static struct device *svs_add_device_link(struct svs_platform *svsp,
 					  const char *node_name)
 {
 	struct device *dev;
-	struct device_link *sup_link;
 
 	dev = svs_get_subsys_device(svsp, node_name);
 	if (IS_ERR(dev))
 		return dev;
 
-	sup_link = device_link_add(svsp->dev, dev,
-				   DL_FLAG_AUTOREMOVE_CONSUMER);
-	if (!sup_link) {
-		dev_err(svsp->dev, "sup_link is NULL\n");
+	if (!device_link_add(svsp->dev, dev, DL_FLAG_AUTOREMOVE_CONSUMER)) {
+		dev_err(svsp->dev, "device_link_add returns NULL\n");
+		put_device(dev);
 		return ERR_PTR(-EINVAL);
 	}
 
-	if (sup_link->supplier->links.status != DL_DEV_DRIVER_BOUND)
+	if (READ_ONCE(dev->links.status) != DL_DEV_DRIVER_BOUND) {
+		put_device(dev);
 		return ERR_PTR(-EPROBE_DEFER);
+	}
 
 	return dev;
 }
-- 
2.47.3

Re: [PATCH] soc: mediatek: SVS: fix device reference leak in svs_add_device_link()

[Markus Elfring]

…
> Drop the reference before returning on both paths.
…

How do you think about to increase the application of scope-based resource management?
https://elixir.bootlin.com/linux/v7.3/source/include/linux/device.h#L1363

Regards,
Markus

Re: [PATCH] soc: mediatek: SVS: fix device reference leak in svs_add_device_link()

[Coia Prant]

On October 25, 2026 3:58:06 PM GMT+08:00, Markus Elfring <Markus.Elfring@web.de> wrote:
>…
>> Drop the reference before returning on both paths.
>…
>
>How do you think about to increase the application of scope-based resource management?
>https://elixir.bootlin.com/linux/v7.3/source/include/linux/device.h#L1363
>
>Regards,
>Markus

Hi Markus,

Thanks for the suggestion. To be upfront: I'm not the maintainer of this
driver, I just ran into the leak while reading through the code and
wanted to send a minimal fix.

__free(), DEFINE_FREE() and return_ptr() only appeared in v6.5, while
this driver dates back to v5.19 and the fix is tagged for stable (first
LTS is v6.1). Converting to scope-based cleanup here would not compile
on 6.1.y and would force the stable team to rewrite the backport.

So I'd prefer to keep the explicit put_device() in this patch and leave
the __free(put_device) conversion to whoever owns the driver, as a
separate cleanup when the stable backport is no longer a concern.

Regards,
Coia

Re: soc: mediatek: SVS: fix device reference leak in svs_add_device_link()

[Markus Elfring]

…
> __free(), DEFINE_FREE() and return_ptr() only appeared in v6.5, while
> this driver dates back to v5.19 and the fix is tagged for stable (first
> LTS is v6.1). Converting to scope-based cleanup here would not compile
> on 6.1.y and would force the stable team to rewrite the backport.
> 
> So I'd prefer to keep the explicit put_device() in this patch and leave
> the __free(put_device) conversion to whoever owns the driver, as a
> separate cleanup when the stable backport is no longer a concern.

Would you eventually like to become more involved with corresponding
source code transformations?

Regards,
Markus

Re: soc: mediatek: SVS: fix device reference leak in svs_add_device_link()

[Coia Prant]

On October 25, 2026 7:42:51 PM GMT+08:00, Markus Elfring <Markus.Elfring@web.de> wrote:
>…
>> __free(), DEFINE_FREE() and return_ptr() only appeared in v6.5, while
>> this driver dates back to v5.19 and the fix is tagged for stable (first
>> LTS is v6.1). Converting to scope-based cleanup here would not compile
>> on 6.1.y and would force the stable team to rewrite the backport.
>> 
>> So I'd prefer to keep the explicit put_device() in this patch and leave
>> the __free(put_device) conversion to whoever owns the driver, as a
>> separate cleanup when the stable backport is no longer a concern.
>
>Would you eventually like to become more involved with corresponding
>source code transformations?
>
>Regards,
>Markus

Hi Markus,

Thanks, that's kind of you. I'll keep it in mind, but I can't promise
consistent bandwidth — my kernel time comes in bursts.

PCS is where most of my remaining TODO list is, so that's where I'll
spend the next chunk of time. If I get through that and still have room,
I'll take a look at the other device_link_add() call sites that have the
same missing-put_device pattern. There seem to be a few.

Regards,
Coia

Re: soc: mediatek: SVS: fix device reference leak in svs_add_device_link()

[Markus Elfring]

…
> PCS is where most of my remaining TODO list is, so that's where I'll
> spend the next chunk of time. If I get through that and still have room,
> I'll take a look at the other () call sites that have the
> same missing-put_device pattern. There seem to be a few.…

Would you be looking for further software improvements?
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/scripts/coccinelle/free/put_device.cocci?h=next-20261023#n2
https://elixir.bootlin.com/linux/v7.3/A/ident/device_link_add

Regards,
Markus