Re: [PATCH v3 2/2] iio: accel: kxsd9: fix runtime PM leaks and unchecked returns

[biren pandya]

On Sun, Jul 12, 2026 at 4:38 AM Jonathan Cameron <jic23@xxxxxxxxxx> wrote:
>
> On Mon,  6 Jul 2026 13:16:52 +0530
> Biren Pandya <birenpandya@xxxxxxxxx> wrote:
>
> > The kxsd9 driver previously used manual pm_runtime_get_sync() and
> > pm_runtime_put_autosuspend() calls in various return paths, which is
> > prone to memory leaks if a return path is missed. Specifically, in
> > kxsd9_write_raw(), if the user provides a non-zero integer component
> > for scale, the function previously returned -EINVAL directly, leaking
> > the runtime PM usage counter.
>
> This has become too complex. For the fix, can we just close that one
> problem case.  Then follow up with the refactors and hardening after?
>
> The minimal fix is probably the code move you have to check that
> the request is valid before doing anything with runtime pm.

Thanks Jonathan, Andy — agreed on all of it.

I've just sent out v4 as a fresh thread. It contains just the minimal
leak fix: moving the mask/val validation ahead of
pm_runtime_get_sync() in write_raw()
so the -EINVAL paths stop leaking the usage counter. Nothing else is
in that patch.

> I don't have time now to do a full driver review (too many patches
> waiting) but in general this looks like it could do with some
> 'modernization'.   I feel justified in being particularly rude
> about this one as a much younger me wrote it :)

The macro conversion, the per-case PM scoping in read_raw(), and the
write_scale() cleanup (ARRAY_SIZE, drop foundit, return 0,
reverse-xmas-tree) I will send as a separate follow-up series once the
fix is in, so it doesn't hold up the bug.

> Andy's point is valid about the reorder.  I don't think it is
> a bug, but I'm fairly sure it is unnecessary as nothing else is
> going to mess with the reference count in this window.
>
> I'm also not sure I care about an underflow if we are in the
> weird corners of runtime pm failing on remove.
> Note that it doesn't actually fail if you drop it too far (despite
> what sashiko thinks).  Runtime pm counters are always >= 0.

I've dropped the remove() rework entirely from v4 — you're both right
that put_noidle() floors at 0 natively,
so the reorder is unnecessary and the underflow concern wasn't real.
Still learning exactly where the line sits between "fix" and "cleanup"
for stable.
Thanks for the patience and the detailed checklist for the upcoming
modernization series!