From: Marek Szyprowski <m.szyprowski@samsung.com> To: Tzung-Bi Shih <tzungbi@google.com> Cc: Mark Brown <broonie@kernel.org>, ALSA development <alsa-devel@alsa-project.org>, Dylan Reid <dgreid@google.com>, Jimmy Cheng-Yi Chiang <cychiang@google.com>, Sylwester Nawrocki <s.nawrocki@samsung.com>, Linux Samsung SOC <linux-samsung-soc@vger.kernel.org>, Krzysztof Kozlowski <krzk@kernel.org> Subject: Re: [alsa-devel] [PATCH v2] ASoC: max98090: save and restore SHDN when changing sensitive registers Date: Tue, 17 Dec 2019 15:18:38 +0100 [thread overview] Message-ID: <901df54d-96ad-b25a-fc0d-5901076f6c09@samsung.com> (raw) In-Reply-To: <CA+Px+wXPa_cwdZUQfCx4jAhhj4Q9b7bNABUGazLKOJ7U5ae-mA@mail.gmail.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
WARNING: multiple messages have this Message-ID (diff)
From: Marek Szyprowski <m.szyprowski@samsung.com> To: Tzung-Bi Shih <tzungbi@google.com> Cc: ALSA development <alsa-devel@alsa-project.org>, Linux Samsung SOC <linux-samsung-soc@vger.kernel.org>, Jimmy Cheng-Yi Chiang <cychiang@google.com>, Krzysztof Kozlowski <krzk@kernel.org>, Mark Brown <broonie@kernel.org>, Sylwester Nawrocki <s.nawrocki@samsung.com>, Dylan Reid <dgreid@google.com> Subject: Re: [alsa-devel] [PATCH v2] ASoC: max98090: save and restore SHDN when changing sensitive registers Date: Tue, 17 Dec 2019 15:18:38 +0100 [thread overview] Message-ID: <901df54d-96ad-b25a-fc0d-5901076f6c09@samsung.com> (raw) In-Reply-To: <CA+Px+wXPa_cwdZUQfCx4jAhhj4Q9b7bNABUGazLKOJ7U5ae-mA@mail.gmail.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 _______________________________________________ Alsa-devel mailing list Alsa-devel@alsa-project.org https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
next prev parent reply other threads:[~2019-12-17 14:18 UTC|newest] Thread overview: 40+ messages / expand[flat|nested] mbox.gz Atom feed top [not found] <CGME20191128152110epcas3p2b205b4b55f6d8bfac42fcb8faaade93c@epcas3p2.samsung.com> 2019-11-28 15:19 ` [alsa-devel] [PATCH v2] ASoC: max98090: save and restore SHDN when changing sensitive registers Tzung-Bi Shih 2019-12-09 18:59 ` [alsa-devel] Applied "ASoC: max98090: save and restore SHDN when changing sensitive registers" to the asoc tree Mark Brown 2019-12-12 14:09 ` [alsa-devel] [PATCH v2] ASoC: max98090: save and restore SHDN when changing sensitive registers Marek Szyprowski 2019-12-12 14:09 ` Marek Szyprowski 2019-12-12 16:02 ` Marek Szyprowski 2019-12-12 16:02 ` Marek Szyprowski 2019-12-12 16:48 ` Mark Brown 2019-12-12 16:48 ` Mark Brown 2019-12-12 18:05 ` Tzung-Bi Shih 2019-12-12 18:05 ` Tzung-Bi Shih 2019-12-17 14:18 ` Marek Szyprowski [this message] 2019-12-17 14:18 ` Marek Szyprowski 2019-12-18 13:26 ` Mark Brown 2019-12-18 13:26 ` Mark Brown 2019-12-18 14:48 ` Marek Szyprowski 2019-12-18 14:48 ` Marek Szyprowski 2019-12-18 16:24 ` Mark Brown 2019-12-18 16:24 ` Mark Brown 2019-12-19 8:03 ` Marek Szyprowski 2019-12-19 8:03 ` Marek Szyprowski 2019-12-19 12:37 ` Mark Brown 2019-12-19 12:37 ` Mark Brown 2019-12-19 12:54 ` Marek Szyprowski 2019-12-19 12:54 ` Marek Szyprowski 2019-12-19 13:05 ` Mark Brown 2019-12-19 13:05 ` Mark Brown 2019-12-19 13:41 ` Marek Szyprowski 2019-12-19 13:41 ` Marek Szyprowski 2019-12-19 19:16 ` Mark Brown 2019-12-19 19:16 ` Mark Brown 2019-12-20 8:28 ` Marek Szyprowski 2019-12-20 8:28 ` Marek Szyprowski 2019-12-20 9:05 ` Marek Szyprowski 2019-12-20 9:05 ` Marek Szyprowski 2019-12-20 12:01 ` Mark Brown 2019-12-20 12:01 ` Mark Brown 2020-01-08 11:54 ` Marek Szyprowski 2020-01-08 11:54 ` Marek Szyprowski 2020-01-09 21:18 ` Mark Brown 2020-01-09 21:18 ` Mark Brown
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=901df54d-96ad-b25a-fc0d-5901076f6c09@samsung.com \ --to=m.szyprowski@samsung.com \ --cc=alsa-devel@alsa-project.org \ --cc=broonie@kernel.org \ --cc=cychiang@google.com \ --cc=dgreid@google.com \ --cc=krzk@kernel.org \ --cc=linux-samsung-soc@vger.kernel.org \ --cc=s.nawrocki@samsung.com \ --cc=tzungbi@google.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.