linux-kernel - Re: [alsa-devel] [PATCH] Sound: soc: samsung

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: <0adef6d5-13bd-5e08-ca53-c8a5969578a2@samsung.com>
Date:   Mon, 30 Jan 2017 14:34:23 +0100
From:   Sylwester Nawrocki <s.nawrocki@...sung.com>
To:     Shailendra Verma <shailendra.v@...sung.com>,
        p.shailesh@...sung.com, ashish.kalra@...sung.com,
        Shailendra Verma <shailendra.capricorn@...il.com>
Cc:     Krzysztof Kozlowski <krzk@...nel.org>,
        Sangbeom Kim <sbkim73@...sung.com>,
        Liam Girdwood <lgirdwood@...il.com>,
        Mark Brown <broonie@...nel.org>,
        Jaroslav Kysela <perex@...ex.cz>,
        Takashi Iwai <tiwai@...e.com>, alsa-devel@...a-project.org,
        linux-kernel@...r.kernel.org
Subject: Re: [alsa-devel] [PATCH] Sound: soc: samsung - Fix possible NULL
 derefrence.

On 01/30/2017 06:04 AM, Shailendra Verma wrote:
> of_device_get_match_data could return NULL, and so can cause
> a NULL pointer dereference later.
> 
> Signed-off-by: Shailendra Verma <shailendra.v@...sung.com>
> ---
>  sound/soc/samsung/i2s.c |    7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/sound/soc/samsung/i2s.c b/sound/soc/samsung/i2s.c
> index 7825bff..b5acce2 100644
> --- a/sound/soc/samsung/i2s.c
> +++ b/sound/soc/samsung/i2s.c
> @@ -1224,8 +1224,13 @@ static int samsung_i2s_probe(struct platform_device *pdev)
>  	const struct samsung_i2s_dai_data *i2s_dai_data;
>  	int ret;
>  
> -	if (IS_ENABLED(CONFIG_OF) && pdev->dev.of_node)
> +	if (IS_ENABLED(CONFIG_OF) && pdev->dev.of_node) {
>  		i2s_dai_data = of_device_get_match_data(&pdev->dev);
> +		if (!i2s_dai_data) {
> +			dev_err(&pdev->dev, "no device match found\n");
> +			return -ENODEV;
> +		}
> +	}

As Javier commented on similar patch for gpu/drm/exynos i2s_dai_data
can't be NULL as for DT case all struct of_device_id::data entries
are initialized in this driver and the probe callback can only be
called when device's compatible string got matched with one of entries
in the OF device id table.

>  	else
>  		i2s_dai_data = (struct samsung_i2s_dai_data *)
>  				platform_get_device_id(pdev)->driver_data;
> 

Similarly in non-dt case all struct_device_id::driver_data entries
are initialized so i2s_dai_data normally also cannot be NULL here.
If we were adding a sanity check for i2s_dai_data, it would need to
be here to cover both dt and non-dt.

I don't think this patch is necessary.  The error log above is slightly
misleading as actual device/driver matching happens already before
probe() is called.

-- 
Thanks,
Sylwester