[Letux-kernel] [Gta04-owner] New LetuxOS Kernels

H. Nikolaus Schaller hns at goldelico.com
Wed Jun 20 08:13:12 CEST 2018


Hi Tony,

> Am 20.06.2018 um 07:40 schrieb Tony Lindgren <tony at atomide.com>:
> 
> * H. Nikolaus Schaller <hns at goldelico.com> [180620 04:58]:
>> Problem seems to be:
>> 
>> 1. before driver is probed pinmux does a group = devm_kzalloc
>> 2. this is added to radix tree
>> 3. driver probe fails for some reason
>> 4. 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.

> 
>> 5. someone reuses the memory area defined by kzalloc
>> 6. other driver is probed and wants to check if selector exists
>> 7. scans through radix tree (in pinctrl_generic_group_name_to_selector)
>> 8. finds NULLified memory area [or maybe other stale data!]
>> 9. 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




More information about the Letux-kernel mailing list