Re: [PATCH 1/9] ALSA: hda: Do not unset preset when cleaning up codec

[Takashi Iwai <tiwai@suse.de>]

On Mon, 11 Jul 2022 10:25:17 +0200,
Cezary Rojewski wrote:
> 
> On 2022-07-09 6:34 PM, Takashi Iwai wrote:
> > On Wed, 06 Jul 2022 14:02:22 +0200,
> > Cezary Rojewski wrote:
> >> 
> >> snd_hda_codec_cleanup_for_unbind() unsets preset what interferes with
> >> module unloading and triggers null-ptr-deref. Preset is assigned only
> >> once, during device/driver matching whereas module reload and unload
> >> follow completely different path and may occur several times during
> >> runtime.
> > 
> > Hm, the driver reload/unload does unbind.  Keeping this field mean to
> > leave the pointer to the possibly freed object, no?
> > 
> > And if it's not cleared, where is this field cleared instead?
> 
> 
> avs-driver i.e. the bus driver takes responsibility for the codec
> device only. There is no real probe(), just the device creation and
> initialization of its fields. The rest is handled by the component
> driver (sound/soc/hda.c). If this field is cleared and the test is
> limited to reloading HDAudio codec module alone, we get a
> panic. Something similar to the stack found below my message.
> 
> In regard to the other question - are presets freed at all? It seems
> all of them are part of the static device-driver matching list. If so,
> the pointer is always valid.

When the codec driver is unbound and the module is unloaded, the whole
objects and symbols are gone.


> [  136.827856] RIP: 0010:hda_codec_probe+0x16c/0x560 [snd_soc_hda_codec]
> [  136.827929] Code: ff 85 c0 0f 88 5b 0b 00 00 4d 8d bc 24 d0 03 00
> 00 4c 89 ff e8 e5 a2 9e ca 49 8b 9c 24 d0 03 00 00 48 8d 7b 10 e8 d4
> a2 9e ca <48> 8b 73 10 4c 89 e7 e8 e8 7d fb ff 85 c0 0f 88 43 0b 00 00
> 4c 89
> [  136.828028] RSP: 0018:ffff888101af74d0 EFLAGS: 00010286
> [  136.828079] RAX: 0000000000000001 RBX: 0000000000000000 RCX:
> ffffffff8b4f1b1a
> [  136.828128] RDX: 0000000000000001 RSI: 0000000000000008 RDI:
> ffffffff8e323d20
> [  136.828175] RBP: ffff888101af7540 R08: 1ffffffff1c647a4 R09:
> fffffbfff1c647a5
> [  136.828224] R10: ffffffff8e323d27 R11: fffffbfff1c647a4 R12:
> ffff888102920000
> [  136.828272] R13: ffff88810812e428 R14: ffff888102925028 R15:
> ffff8881029203d0
> [  136.828323] FS:  00007f9049dd8540(0000) GS:ffff888227100000(0000)
> knlGS:0000000000000000
> [  136.828380] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [  136.828425] CR2: 0000000000000010 CR3: 000000010f086001 CR4:
> 00000000003706e0
> [  136.828474] DR0: 0000000000000000 DR1: 0000000000000000 DR2:
> 0000000000000000
> [  136.828520] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7:
> 0000000000000400
> [  136.828568] Call Trace:
> [  136.828593]  <TASK>
> [  136.828628]  snd_soc_component_probe+0x3a/0x60 [snd_soc_core]
> [  136.828981]  soc_probe_component+0x276/0x4a0 [snd_soc_core]
> [  136.829274]  snd_soc_bind_card+0x819/0x13d0 [snd_soc_core]
> [  136.829560]  ? __kasan_slab_alloc+0x32/0x90
> [  136.829614]  snd_soc_register_card+0x24e/0x260 [snd_soc_core]
> [  136.829900]  devm_snd_soc_register_card+0x48/0x90 [snd_soc_core]
> [  136.830204]  avs_hdaudio_probe+0x298/0x2c0 [snd_soc_avs_hdaudio]
> [  136.830269]  platform_probe+0x67/0x100
> [  136.830313]  really_probe+0x1ff/0x500
> [  136.830354]  __driver_probe_device+0xeb/0x240
> [  136.830397]  driver_probe_device+0x4e/0xf0
> [  136.830438]  __driver_attach+0xfd/0x210
> [  136.830478]  ? __device_attach_driver+0x170/0x170
> [  136.830520]  bus_for_each_dev+0xf9/0x150
> [  136.830557]  ? subsys_dev_iter_exit+0x10/0x10
> [  136.830597]  ? preempt_count_sub+0x18/0xc0
> [  136.830643]  driver_attach+0x2d/0x40
> [  136.830679]  bus_add_driver+0x28e/0x320
> [  136.830722]  driver_register+0xdc/0x170
> [  136.830763]  ? 0xffffffffc0428000
> [  136.830796]  __platform_driver_register+0x39/0x40
> [  136.830842]  avs_hdaudio_driver_init+0x1c/0x1000 [snd_soc_avs_hdaudio]
> [  136.830902]  do_one_initcall+0xa0/0x2e0
> [  136.830939]  ? initcall_blacklisted+0x170/0x170
> [  136.830981]  ? __kasan_kmalloc+0x88/0xa0
> [  136.831020]  ? kasan_poison+0x3c/0x50
> [  136.831059]  ? kasan_unpoison+0x28/0x50
> [  136.831100]  ? kasan_poison+0x3c/0x50
> [  136.831139]  ? __asan_register_globals+0x5e/0x70
> [  136.831187]  do_init_module+0xf6/0x350
> [  136.831228]  load_module+0x2bf5/0x2e30
> (...)

Hmm, in the Oops above, at which moment,
snd_hda_codec_cleanup_for_unbind() is called via which function?
Is it the unload of HD-audio codec driver during the probe of AVS
HD-audio?

The preset is assigned to the given HD-audio device object for the
attached codec driver.  Once after the codec driver gets unbound, you
must not access to this codec driver's methods any longer, hence we
clear the preset field.

So I wonder how the access to the codec->preset happens after the
codec unbind.


thanks,

Takashi