Hi Tony,
Am 20.06.2018 um 07:40 schrieb Tony Lindgren tony@atomide.com:
- H. Nikolaus Schaller hns@goldelico.com [180620 04:58]:
Problem seems to be:
- before driver is probed pinmux does a group = devm_kzalloc
- this is added to radix tree
- driver probe fails for some reason
- devres_release_all(driver) ==> does a kfree(group)
This should not happen as the devm alloc is done for the pin controller instance, not for a consumer driver instance.
Ah, yes. I missed that.
So we either free it in pinctrl-single.c somehow, or do you have maybe deferred probing happending for some additional pinctrl instance we don't have in the mainline kernel?
Not that we are aware of. The bug does not seem to depend on out-of-tree drivers.
To try to figure out where we free something, do you see pcs_dt_free_map() get called for example?
Good idea.
- someone reuses the memory area defined by kzalloc
- other driver is probed and wants to check if selector exists
- scans through radix tree (in pinctrl_generic_group_name_to_selector)
- finds NULLified memory area [or maybe other stale data!]
- strcmp(NULL)
This happens despite our checking for duplicates.
So we should not fix #9 but #4 to properly remove groups from radix tree and make sure that it is skipped during scanning for selectors (this is what a NULL test #9 finally would do - the test alone isn't enough).
To do this properly we should allow the pin controller driver to specify the selector based on a register offset or similar and get rid of the numbered indexing.
Yes, the indexing is probably not the best approach.
Andy already pointed to a location where the cleanup should take place:
I think there is a simple way to clean up pinctrl stuff on failed probe. See https://elixir.bootlin.com/linux/v4.18-rc1/source/drivers/base/dd.c#L416
We only bind pins, and do not perform any actions when failure happens later on.
Or have we missed this patch?
Hmm that seems like a separate yet another issue though.
Regards,
Tony
BR and thanks, Nikolaus