Re: [PATCH] pinctrl: single: Fix memleak in pcs_dt_node_to_map

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




在 2023/7/6 12:07, Tony Lindgren 写道:
Hi,

* guomengqi (A) <guomengqi3@xxxxxxxxxx> [230706 03:21]:
在 2023/7/4 17:18, Linus Walleij 写道:
On Mon, Jul 3, 2023 at 10:24 AM Guo Mengqi <guomengqi3@xxxxxxxxxx> wrote:

In a reliability test which repeatedly load and remove a module,
I found some kmalloc-256 memory leaks in pinctrl-single.

pcs_dt_node_to_map() will recognize a dt_node and
make a mapping for it. Along the way some pinctrl functions and groups
are registered in pinctrl-single controller. These functions/groups are
registered once and not removed during the system lifetime.

When the client module loads again, pcs_dt_node_to_map() fail to consider
this situation, create the same set of resources, and does not release or
use them.

To fix this, add a check at the start of pcs_parse_one_pinctrl_entry/
pcs_parse_bits_in_pinctrl_entry. If the target is found,
then all the resource allocation and parsing work can be skipped,
just set the mapping with existing function/group information.

Fixes: 8b8b091bf07f ("pinctrl: Add one-register-per-pin type device tree
based pinctrl driver")

Signed-off-by: Guo Mengqi <guomengqi3@xxxxxxxxxx>
Good catch!

I expect Tony to review the patch in-depth.
Thank you :)
Thanks for looking into it. I wonder if we can rely on naming for
pinmux_func_name_to_selector() though. Can things change in a way where
we need to release everything and reparse? Mostly wondering what happens
with DT overlays?

Hi

Let me confirm, you mean when the pin controller dtsi changed at runtime, some functions and groups can change silently while the dt-node name remains same, so the old data needs to be released and reparsed, right?

I don't know much about DT overlays. I can look deeper into revelant codes, maybe do some experiments too.

My guess now is DT overlay will first remove the old parsed nodes, then create new ones. If so, the modification to pcs_dt_node_to_map() in this patch is not affected.


Regards,

Mengqi

-static int pinmux_func_name_to_selector(struct pinctrl_dev *pctldev,
+int pinmux_func_name_to_selector(struct pinctrl_dev *pctldev,
                                          const char *function)
   {
          const struct pinmux_ops *ops = pctldev->desc->pmxops;
It appears you need to add EXPORT_SYMBOL_GPL() for this function
so the module can build. (This is why the build robot complains.)
Yes, it happens when config=M. I will send a v2 patch later to fix this.
That change might be worth doing in any case if there is need for it.

Regards,

Tony

.



[Index of Archives]     [Linux Arm (vger)]     [ARM Kernel]     [ARM MSM]     [Linux Tegra]     [Linux WPAN Networking]     [Linux Wireless Networking]     [Maemo Users]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux