Re: [PATCH v8 03/16] power: sequencing: Add pwrseq_get_state()

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [Low] Data race when reading desc->powered_on locklessly in pwrseq_get_state()

Pre-existing issues:
- [High] TOCTOU race in pwrseq_enable() and pwrseq_disable() causes permanent resource leak or WARN()
--

commit 5b89b7211c4aa2df354664ebde1b55a332d08712
Author: Chen-Yu Tsai [off-list ref]
Date:   Fri Sep 4 08:41:42 2026 +0000

    power: sequencing: Add pwrseq_get_state()
    
    The power sequencing consumer API already does power on state tracking
    internally. Expose the state to consumers through pwrseq_get_state()
    so that they don't have to reimplement it locally.

[Severity: High]
This is a pre-existing issue, but could the lockless checks of desc->powered_on
lead to a resource leak or a WARN() due to a TOCTOU race?

If a consumer calls pwrseq_enable() from two threads concurrently, could they
both bypass the lockless check:

drivers/power/sequencing/core.c:pwrseq_enable() {
    ...
    if (!desc || desc->powered_on)
        return 0;
    ...
}

and then both acquire state_lock and double-increment the provider unit's
enable_count?

A subsequent single pwrseq_disable() would then clear desc->powered_on:

drivers/power/sequencing/core.c:pwrseq_disable() {
    ...
    if (!desc || !desc->powered_on)
        return 0;
    ...
}

causing future disables to return early locklessly. Would this leave the
unit's enable_count permanently elevated?

Additionally, if pwrseq_disable() races with the lockless post_enable() error
rollback path, could it trigger the WARN(1) in pwrseq_unit_disable() due to
underflowing enable_count?

 }
 EXPORT_SYMBOL_GPL(pwrseq_disable);
 
+/**
+ * pwrseq_get_state() - Queries the last requested state of the power sequencer.

[ ... ]

+int pwrseq_get_state(struct pwrseq_desc *desc)
+{
+	if (!desc)
+		return -EINVAL;
+
+	return desc->powered_on ? PWRSEQ_STATE_ON : PWRSEQ_STATE_OFF;

[Severity: Low]
Does this lockless read of a state variable that is concurrently modified by
writers without READ_ONCE() cause a data race?

Since pwrseq_get_state() can be called concurrently with pwrseq_enable() or
pwrseq_disable(), kernel concurrency rules generally require READ_ONCE() for
lockless reads of shared variables to prevent compiler optimization issues.

+}
+EXPORT_SYMBOL_GPL(pwrseq_get_state);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260904084158.1341550-1-wenst@chromium.org?part=3