linux-kernel - Re: [PATCH] ASoC: SOF: Fix null pointer dereference in sof_pci

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] [day] [month] [year] [list]

Message-ID: <74ba8770-6174-4820-8dc4-e97e5814e665@linux.intel.com>
Date:   Tue, 5 Dec 2023 11:44:16 +0200
From:   Péter Ujfalusi <peter.ujfalusi@...ux.intel.com>
To:     Kunwu Chan <chentao@...inos.cn>,
        pierre-louis.bossart@...ux.intel.com, lgirdwood@...il.com,
        yung-chuan.liao@...ux.intel.com, ranjani.sridharan@...ux.intel.com,
        daniel.baluta@....com, kai.vehmanen@...ux.intel.com,
        broonie@...nel.org, perex@...ex.cz, tiwai@...e.com,
        chao.song@...el.com
Cc:     kunwu.chan@...mail.com, sound-open-firmware@...a-project.org,
        linux-sound@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] ASoC: SOF: Fix null pointer dereference in sof_pci_probe



On 05/12/2023 11:10, Kunwu Chan wrote:
> It's my bad, i'll follow your suggestion in v2 patch:
> 1. remove 'Fixes: 46207ca24545' label
> 
> 2. rename subject to 'ASoC: SOF: Add null pointer check to sof_pci_probe'
> 
> 3. when sof_pdata->fw_filename_prefix is NULL,don't use it to dev_dbg.
> Or just use a
> if (sof_pdata->fw_filename_prefix)
>         dev_dbg(dev,
>                 "Platform uses community key, changed fw path to %s\n",
>                 sof_pdata->fw_filename_prefix);
> else
>     dev_dbg(dev,
>                 "Platform uses community key, changed fw path to %s/%s\n",
>                  sof_pdata->desc->default_fw_path[sof_pdata->ipc_type],
>                  "community");
> 
> Is it okay to modify it like this?

The code has been rewritten and all of these issues have been
eliminated, fixed in core SOF.

> 
> 
> Thanks,
> Kunwu
> 
> On 2023/12/4 19:34, Péter Ujfalusi wrote:
>>
>>
>> On 23/11/2023 17:54, Kunwu Chan wrote:
>>> devm_kasprintf() returns a pointer to dynamically allocated memory
>>> which can be NULL upon failure.
>>>
>>> Fixes: 46207ca24545 ("ASoC: SOF: pci: change the default firmware
>>> path when the community key is used")
>>> Fixes: 25bbc0c59ee1 ("ASoC: SOF: Add path definition for external
>>> firmware libraries")
>>> Signed-off-by: Kunwu Chan <chentao@...inos.cn>
>>
>> This patch cannot fix both commits as at 46207ca24545 the lib path was
>> not present.
>> Please don't bundle unrelated fixes, it makes maintainers life harder.
>>
>> As a note: if the allocation fails it is not going to cause null
>> dereference as the code checks !sof_pdata->*_prefix to see if it is
>> provided (override) or not (use default).
>> At most it will cause the kernel to 'ignore' the path override, but if
>> this allocation fails this is least of our problem ;)
>>
>>> ---
>>>   sound/soc/sof/sof-pci-dev.c | 8 ++++++++
>>>   1 file changed, 8 insertions(+)
>>>
>>> diff --git a/sound/soc/sof/sof-pci-dev.c b/sound/soc/sof/sof-pci-dev.c
>>> index 64b326e3ef85..75a9bfa5bfbe 100644
>>> --- a/sound/soc/sof/sof-pci-dev.c
>>> +++ b/sound/soc/sof/sof-pci-dev.c
>>> @@ -282,6 +282,10 @@ int sof_pci_probe(struct pci_dev *pci, const
>>> struct pci_device_id *pci_id)
>>>               devm_kasprintf(dev, GFP_KERNEL, "%s/%s",
>>>                         
>>> sof_pdata->desc->default_fw_path[sof_pdata->ipc_type],
>>>                          "community");
>>> +        if (!sof_pdata->fw_filename_prefix) {
>>> +            ret = -ENOMEM;   
>>> +            goto out;
>>> +        }
>>>             dev_dbg(dev,
>>>               "Platform uses community key, changed fw path to %s\n",
>>> @@ -303,6 +307,10 @@ int sof_pci_probe(struct pci_dev *pci, const
>>> struct pci_device_id *pci_id)
>>>                   devm_kasprintf(dev, GFP_KERNEL, "%s/%s",
>>>                      
>>> sof_pdata->desc->default_lib_path[sof_pdata->ipc_type],
>>>                       "community");
>>> +            if (!sof_pdata->fw_lib_prefix) {
>>> +                ret = -ENOMEM;
>>> +                goto out;
>>> +            }
>>>                 dev_dbg(dev,
>>>                   "Platform uses community key, changed fw_lib path
>>> to %s\n",
>>

-- 
Péter