Re: [PATCH v2 1/4] driver core: Check for supplier requiring PM at link creation

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] core: missing registration check in device_link_add() for no_pm devices
--

commit b2ba746c71946921fa9666578a3ca843070e29f1
Author: Alejandro Lucero <alucerop@amd.com>

driver core: Check for supplier requiring PM at link creation

This commit updates device_link_add() to skip the PM initialization
check for devices that do not require PM, allowing links to be
established for such devices.

> diff --git a/drivers/base/core.c b/drivers/base/core.c
> index 4c0c373998a19..bf0513beafad4 100644
> --- a/drivers/base/core.c
> +++ b/drivers/base/core.c

[ ... ]

> @@ -840,7 +840,7 @@ struct device_link *device_link_add(struct device *consumer,
>  	 * SYNC_STATE_ONLY link, we don't check for reverse dependencies
>  	 * because it only affects sync_state() callbacks.
>  	 */
> -	if (!device_pm_initialized(supplier)
> +	if ((!device_pm_not_required(supplier) && !device_pm_initialized(supplier))

[Severity: Medium]
Does this inadvertently bypass the registration check for devices where
device_pm_not_required() is true?

The comment immediately above states: "If the supplier has not been fully
registered yet ... return NULL". Previously, device_pm_initialized()
implicitly enforced this because it wraps device_is_registered().

If a caller invokes device_link_add() with a supplier device that has
no_pm set to true but has not been successfully registered, bypassing this
check entirely means the unregistered supplier is allowed through. Since an
unregistered device will never go through device_del(), device_links_purge()
will never run to clean up the link. Can this result in a permanent leak
of the struct device_link and device refcounts?

Would it make sense to explicitly add a !device_is_registered(supplier)
fallback check here to ensure the core API contract is maintained?

>  	    || (!(flags & DL_FLAG_SYNC_STATE_ONLY) &&
>  		  device_is_dependent(consumer, supplier))) {
>  		link = NULL;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261001132023.17032-1-alucerop@amd.com?part=1