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

Message-ID: <1e1430fb-d1d6-476d-a2d1-64ce2c86daa6@linux.intel.com>
Date:   Mon, 4 Dec 2023 13:34:37 +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 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