Re: [alsa-devel] [PATCH v2] ASoC: max98090: save and restore SHDN when changing sensitive registers

[Marek Szyprowski <m.szyprowski@samsung.com>]

Hi All,

On 12.12.2019 19:05, Tzung-Bi Shih wrote:
> On Thu, Dec 12, 2019 at 10:09 PM Marek Szyprowski
> <m.szyprowski@samsung.com> wrote:
>> -> #1 (&card->controls_rwsem){++++}:
>>          snd_ctl_add_replace+0x3c/0x84
>>          dapm_create_or_share_kcontrol+0x24c/0x2e0
>>          snd_soc_dapm_new_widgets+0x308/0x594
>>          snd_soc_bind_card+0x80c/0xac8
>>          devm_snd_soc_register_card+0x34/0x6c
>>          asoc_simple_probe+0x244/0x4a0
>>          platform_drv_probe+0x6c/0xa4
>>          really_probe+0x200/0x490
>>          driver_probe_device+0x78/0x1f8
>>          bus_for_each_drv+0x74/0xb8
>>          __device_attach+0xd4/0x16c
>>          bus_probe_device+0x88/0x90
>>          deferred_probe_work_func+0x3c/0xd0
>>          process_one_work+0x22c/0x7c4
>>          worker_thread+0x44/0x524
>>          kthread+0x130/0x164
>>          ret_from_fork+0x14/0x20
>>          0x0
> A key observation here is: the card registration got deferred.

Right. The deferred probe is caused by missing regulator on the first try.

>> -> #0 (&card->dapm_mutex){+.+.}:
>>          lock_acquire+0xe8/0x270
>>          __mutex_lock+0x9c/0xb18
>>          mutex_lock_nested+0x1c/0x24
>>          max98090_shdn_save+0x1c/0x28
>>          max98090_put_enum_double+0x20/0x40
>>          snd_ctl_ioctl+0x190/0xbb8
>>          do_vfs_ioctl+0xb0/0xab0
>>          ksys_ioctl+0x34/0x5c
>>          ret_fast_syscall+0x0/0x28
>>          0xbe9094dc
> And this is an ioctl( ) on a control (e.g. controlC0).
>
> I have no enough resources to test and trace the code temporarily.
> But is it possible:
> - snd_card_new( ) succeed in snd_soc_bind_card( ), so that userspace
> can see the control
> - code in later snd_soc_bind_card( ) decided to defer the probe
> - soc_cleanup_card_resources( ) may forget to clean the control?  (not
> sure about this)
> Then, when the card is instantiating next time, some userspace program
> tries to ioctl( ) to get the deadlock possibility and the NULL
> dereference.

I've tried to debug this issue, but without much progress.

Here is what I've noticed:

1. This NULL ptr dereference happens on snd_ctl_elem_write(), with a 
valid (at least previously registered) kcontrol object.

2. The kcontrol ->put method is routed to 
max98090_dapm_put_enum_double(), in which the function 
snd_soc_kcontrol_component() returns random/buggy component pointer, 
what then causes the NULL ptr dereference.

3. The component object has been registered via the following function call:

(snd_ctl_add) from [<c0829030>] (dapm_create_or_share_kcontrol+0x24c/0x2e0)
(dapm_create_or_share_kcontrol) from [<c08293cc>] 
(snd_soc_dapm_new_widgets+0x308/0x594)
(snd_soc_dapm_new_widgets) from [<c0820a64>] (snd_soc_bind_card+0x80c/0xac8)
(snd_soc_bind_card) from [<c083217c>] (devm_snd_soc_register_card+0x34/0x6c)
(devm_snd_soc_register_card) from [<c084772c>] 
(odroid_audio_probe+0x288/0x34c)
(odroid_audio_probe) from [<c05e2b68>] (platform_drv_probe+0x6c/0xa4)
(platform_drv_probe) from [<c05e02f0>] (really_probe+0x200/0x490)
(really_probe) from [<c05e0754>] (driver_probe_device+0x78/0x1f8)
(driver_probe_device) from [<c05de1fc>] (bus_for_each_drv+0x74/0xb8)
(bus_for_each_drv) from [<c05e0050>] (__device_attach+0xd4/0x16c)
(__device_attach) from [<c05df1c0>] (bus_probe_device+0x88/0x90)
(bus_probe_device) from [<c05df6d8>] (deferred_probe_work_func+0x3c/0xd0)
(deferred_probe_work_func) from [<c0149824>] (process_one_work+0x22c/0x7c4)
(process_one_work) from [<c0149e00>] (worker_thread+0x44/0x524)
(worker_thread) from [<c0150dbc>] (kthread+0x130/0x164)
(kthread) from [<c01010b4>] (ret_from_fork+0x14/0x20)

4. kcontrol->id.name during the registration is "LBENL Mux".

5. It looks that the max98090 related kcontrols are registered in the 
system only once, so it doesn't look like an issue with stale object 
from the previous probe() try.

I hope that the above observations helps a bit. The ASoC framework is so 
complex, that I've never ever tried to learn its basic concepts.

Best regards

-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland