Messages in this thread | | | From | Fei Shao <> | Date | Mon, 19 Jun 2023 17:05:47 +0800 | Subject | Re: [PATCH v2] clk: Fix memory leak in devm_clk_notifier_register() |
| |
On Mon, Jun 19, 2023 at 4:48 PM Dan Carpenter <dan.carpenter@linaro.org> wrote: > > On Mon, Jun 19, 2023 at 11:22:53AM +0800, Fei Shao wrote: > > devm_clk_notifier_register() allocates a devres resource for clk > > notifier but didn't register that to the device, so the notifier didn't > > get unregistered on device detach and the allocated resource was leaked. > > > > Fix the issue by registering the resource through devres_add(). > > > > Fixes: 6d30d50d037d ("clk: add devm variant of clk_notifier_register") > > Signed-off-by: Fei Shao <fshao@chromium.org> > > --- > > > > Reviewed-by: Dan Carpenter <dan.carpenter@linaro.org> > > How did you find this bug? > > I can think of some ways to find this bug with static analysis. >
It was actually detected by kmemleak on an unreleased Chromebook device. I added the trace snippet in the message at first but removed that before sending this. Maybe I shouldn't have.
I can resend a v3 to add that back if that's preferable. What do you think?
Regards, Fei
> KTODO: static analysis: look at unused parameters > > Both GCC and Clang have a warning for unused parameters. I think the > last time I looked at GCC it had a lot of false positives for functions > which were called as pointers but hopefully that has been fixed now? > Smatch does not have a check for this. If someone were to write it, > I would probably the check under the --pedantic flag so it would be > turned off by default. > > regards, > dan carpenter >
| |