Re: [PATCH v2 8/9] power: sequencing: Add an API to return the pwrseq device's 'dev' pointer

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Exposing the internal `struct device` breaks abstraction boundaries and bypasses subsystem locks, leading to a Use-After-Free on the device's `fwnode`.
- [Medium] The API returns `NULL` when disabled or passed a `NULL` descriptor, causing a null pointer dereference in property querying APIs.
--

commit 92d97440cc414d34602dcddead25760951c5cdce
Author: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>

power: sequencing: Add an API to return the pwrseq device's 'dev' pointer

This commit adds a new pwrseq_to_device() API to retrieve the underlying
device pointer from a power sequencing descriptor. The intent is to allow
consumers to query the power sequencer's device tree node for specific
properties.

> diff --git a/drivers/power/sequencing/core.c b/drivers/power/sequencing/core.c
> index 4dff71be11b60..96ad557297f5b 100644
> --- a/drivers/power/sequencing/core.c
> +++ b/drivers/power/sequencing/core.c
> @@ -965,6 +965,15 @@ int pwrseq_power_off(struct pwrseq_desc *desc)
>  }
>  EXPORT_SYMBOL_GPL(pwrseq_power_off);
>  
> +struct device *pwrseq_to_device(struct pwrseq_desc *desc)
> +{
> +	if (!desc)
> +		return NULL;
> +
> +	return &desc->pwrseq->dev;

Does exposing the internal device pointer bypass the subsystem locks and
registration checks?

The power sequencing framework internally protects device teardown by
wrapping device accesses in rw_lock and checking device_is_registered(),
such as in pwrseq_power_off():

	guard(rwsem_read)(&pwrseq->rw_lock);
	if (!device_is_registered(&pwrseq->dev))
		return -ENODEV;

If a consumer queries properties on the returned device directly, they might
do so concurrently with or after provider unregistration. In
pwrseq_device_unregister(), device_del() is called, which drops the
reference to the parent device. If the parent is subsequently freed, its
fwnode can be destroyed, leaving pwrseq->dev.fwnode as a dangling pointer.

To safely allow property queries without exposing the raw device pointer,
could the framework provide a dedicated wrapper function like
pwrseq_property_present() that internally holds the lock and checks
registration?

> +}
> +EXPORT_SYMBOL_GPL(pwrseq_to_device);

[ ... ]

> diff --git a/include/linux/pwrseq/consumer.h b/include/linux/pwrseq/consumer.h
> index 7d583b4f266e6..3c907c9e1885d 100644
> --- a/include/linux/pwrseq/consumer.h
> +++ b/include/linux/pwrseq/consumer.h

[ ... ]

> @@ -51,6 +53,11 @@ static inline int pwrseq_power_off(struct pwrseq_desc *desc)
>  	return -ENOSYS;
>  }
>  
> +static inline struct device *pwrseq_to_device(struct pwrseq_desc *desc)
> +{
> +	return NULL;

Will returning NULL here cause a NULL pointer dereference in consumers?

The commit message notes this API is intended for checking the existence of
specific properties on the device. However, functions like
device_property_present() do not accept a NULL device pointer and will
unconditionally dereference it (e.g., when calling __dev_fwnode_const()).

If a consumer uses optional power sequencers or dummy configurations when
CONFIG_POWER_SEQUENCING is disabled, passing the result to
device_property_present() could crash the kernel.

> +}
> +
>  #endif /* CONFIG_POWER_SEQUENCING */

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260507-pwrseq-m2-bt-v2-0-1740bd478539@oss.qualcomm.com?part=8