Re: [PATCH v2 03/11] ASoC: SDCA: expose class SoundWire probe/remove/read_prop as library

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

New issues:
- [Medium] sdca_class_probe() queues an async work item but does not tie its cancellation to devres, creating a potential use-after-free hazard for caller drivers on probe failure.

Pre-existing issues:
- [High] The PM runtime usage counter is permanently leaked if devm_pm_runtime_enable() fails during probe.
- [High] The PM runtime usage counter is permanently leaked if the deferred boot work is cancelled before execution.
- [High] The return value of sdw_slave_read_prop() is ignored, masking potential memory allocation failures and leaving properties uninitialized.
- [High] If the runtime force suspend fails during a system suspend transition, the device's interrupt is left permanently disabled.
--

commit 1cb538c9d508b4520bb99ff1157d3ff8f38843e9
Author: Srinivas Kandagatla [off-list ref]

ASoC: SDCA: expose class SoundWire probe/remove/read_prop as library

Split the internal class_sdw_probe/class_sdw_remove/class_read_prop
functions into caller-friendly library helpers. The class_sdw_probe and
class_sdw_remove callbacks of the built-in class_sdw_driver are now thin
wrappers that defer to the exported helpers.

[ ... ]

[Severity: High]
This is a pre-existing issue, but is it safe to ignore the return value of
sdw_slave_read_prop() here?

If sdw_slave_read_prop() returns -ENOMEM upon memory exhaustion, ignoring it
might cause the SoundWire core to unknowingly operate on uninitialized
properties (e.g., null pointers for src_dpn_prop), leading to null pointer
dereferences during enumeration.

[ ... ]

[Severity: Medium]
Does sdca_class_probe() create a use-after-free hazard for caller drivers on
probe failure?

If a caller allocates sdca_class_drv via devm, calls sdca_class_probe(), and
encounters an error before its own probe finishes, the async boot_work is left
queued:

    queue_work(system_long_wq, &drv->boot_work);
    return 0;

If the caller unwinds its devres allocations without manually cancelling the
work, the background class_boot_work() could dereference the freed drv pointer.
Should a devm action be added to guarantee the cancellation of boot_work?

[Severity: High]
This is a pre-existing issue, but does sdca_class_probe() leak the PM runtime
usage counter if devm_pm_runtime_enable() fails?

In sdca_class_probe(), pm_runtime_get_noresume(dev) increments the usage
counter. If devm_pm_runtime_enable(dev) fails immediately after, the function
returns directly without calling pm_runtime_put_noidle() to decrement the
usage counter:

    pm_runtime_get_noresume(dev);

    ret = devm_pm_runtime_enable(dev);
    if (ret)
        return ret;

Could this permanently prevent the device from entering runtime suspend?

[ ... ]
+
+/**
+ * sdca_class_remove - SDCA class SoundWire slave remove helper
+ * @drv: caller-owned sdca_class_drv (the one handed to sdca_class_probe()).
+ *
+ * Cancels the deferred boot work so devres can safely free @drv and the
+ * embedding codec priv without racing class_boot_work.  Codec-specific
+ * SoundWire drivers that call sdca_class_probe() must call this from
+ * their .remove with the same drv pointer they passed to probe.
+ */
+void sdca_class_remove(struct sdca_class_drv *drv)
+{

[Severity: High]
This is a pre-existing issue, but does this leak the PM runtime usage counter
if the deferred boot work is cancelled before execution?

sdca_class_probe() increments the PM usage count via pm_runtime_get_noresume().
The decrement exclusively occurs inside the deferred class_boot_work(). If the
device is removed and cancel_work_sync() successfully prevents the work from
running, the return value of cancel_work_sync() is ignored and the decrement
never occurs.

[Severity: High]
This is a pre-existing issue, but does sdca_class_system_suspend() leave the
device's interrupt permanently disabled if runtime force suspend fails?

In sdca_class_system_suspend():

    disable_irq(drv->sdw->irq);

    ret = pm_runtime_force_suspend(drv->dev);
    if (ret) {
        dev_err(drv->dev, "failed to force suspend: %d\n", ret);
        return ret;
    }

If pm_runtime_force_suspend() fails, the error is returned immediately and the
system suspend transition is aborted. However, enable_irq(drv->sdw->irq) is not
called in this error path, which might render the device unresponsive to
interrupts.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260907083727.733705-1-srinivas.kandagatla@oss.qualcomm.com?part=3