linux-kernel - Re: [PATCH] ASoC: Intel: Handle device properties with software node API

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 12 Apr 2021 15:36:20 -0500
From:   Pierre-Louis Bossart <pierre-louis.bossart@...ux.intel.com>
To:     Heikki Krogerus <heikki.krogerus@...ux.intel.com>
Cc:     Cezary Rojewski <cezary.rojewski@...el.com>,
        Kai Vehmanen <kai.vehmanen@...ux.intel.com>,
        linux-kernel@...r.kernel.org, Jie Yang <yang.jie@...ux.intel.com>,
        Takashi Iwai <tiwai@...e.com>,
        Liam Girdwood <liam.r.girdwood@...ux.intel.com>,
        Hans de Goede <hdegoede@...hat.com>,
        Mark Brown <broonie@...nel.org>, alsa-devel@...a-project.org,
        Bard Liao <yung-chuan.liao@...ux.intel.com>
Subject: Re: [PATCH] ASoC: Intel: Handle device properties with software node
 API

Hi Heikki,

>>> diff --git a/sound/soc/intel/boards/bytcht_es8316.c b/sound/soc/intel/boards/bytcht_es8316.c
>>> index 06df2d46d910b..4a9817a95928c 100644
>>> --- a/sound/soc/intel/boards/bytcht_es8316.c
>>> +++ b/sound/soc/intel/boards/bytcht_es8316.c
>>> @@ -544,7 +544,7 @@ static int snd_byt_cht_es8316_mc_probe(struct platform_device *pdev)
>>>    		props[cnt++] = PROPERTY_ENTRY_BOOL("everest,jack-detect-inverted");
>>>    	if (cnt) {
>>> -		ret = device_add_properties(codec_dev, props);
>>> +		ret = device_create_managed_software_node(codec_dev, props, NULL);
>>
>> I don't think this is correct. This is using the codec_dev device, but this
>> property is created in the machine driver - different device. I think the
>> proper fix is to remove the property in the machine driver .remove()
>> callback, as done below for the SoundWire case, and not to rely on devm_
>> with the wrong device.
>>
>> there was a thread between July and October on this in
>> https://github.com/thesofproject/linux/pull/2306/
>>
>> It seems that we need to restart this work.
>>
>> Heikki, do you mind if I take your patches (keeping you as the author) and
>> rework+test them with the SOF tree + CI ?
> 
> OK by me (though, I'm not sure about the author part after that).

I took the code and split it in two for BYT/CHT (modified to remove 
devm_) and SoundWire parts (added as is).

https://github.com/thesofproject/linux/pull/2810

Both cases result in a refcount error on device_remove_sof when removing 
the platform device. I don't understand the code well enough to figure 
out what happens, but it's likely a case of the software node being 
removed twice?

kernel: [  872.695132] refcount_t: underflow; use-after-free.
kernel: [  872.695153] WARNING: CPU: 7 PID: 16086 at lib/refcount.c:28 
refcount_warn_saturate+0xa6/0xf0

<snip>

kernel: [  872.695222] CPU: 7 PID: 16086 Comm: rmmod Not tainted 
5.12.0-rc4-pr2810-5522-default #439c50f6
kernel: [  872.695225] Hardware name: Intel Corporation Tiger Lake 
Client Platform/TigerLake U DDR4 SODIMM RVP, BIOS 
TGLSFWI1.R00.3264.A00.2006251828 06/25/2020
kernel: [  872.695226] RIP: 0010:refcount_warn_saturate+0xa6/0xf0

< snip>

kernel: [  872.695244] Call Trace:
kernel: [  872.695248]  sof_sdw_rt711_exit+0x2d/0x40 [snd_soc_sof_sdw]
kernel: [  872.695253]  mc_remove+0xa8/0xe0 [snd_soc_sof_sdw]
kernel: [  872.695256]  ? rt711_rtd_init+0x170/0x170 [snd_soc_sof_sdw]
kernel: [  872.695259]  platform_remove+0x1a/0x40
kernel: [  872.695264]  device_release_driver_internal+0xf1/0x1d0
kernel: [  872.695267]  bus_remove_device+0xed/0x160
kernel: [  872.695269]  device_del+0x187/0x3e0
kernel: [  872.695272]  platform_device_del.part.0+0xe/0x60
kernel: [  872.695274]  platform_device_unregister+0x17/0x30
kernel: [  872.695277]  snd_sof_device_remove+0x53/0xf0 [snd_sof]
kernel: [  872.695283]  sof_pci_remove+0x15/0x40 [snd_sof_pci]
kernel: [  872.695286]  pci_device_remove+0x36/0xa0
kernel: [  872.695290]  device_release_driver_internal+0xf1/0x1d0
kernel: [  872.695292]  driver_detach+0x42/0x90
kernel: [  872.695294]  bus_remove_driver+0x56/0xd0
kernel: [  872.695296]  pci_unregister_driver+0x36/0x80
kernel: [  872.695299]  __x64_sys_delete_module+0x18f/0x250