linux-kernel - Re: [PATCH] i2c: rcar: fix a possible NULL dereference

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:	Thu, 12 Nov 2015 08:44:47 +0100
From:	Uwe Kleine-König 
	<u.kleine-koenig@...gutronix.de>
To:	LABBE Corentin <clabbe.montjoie@...il.com>
Cc:	wsa@...-dreams.de, linux-i2c@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] i2c: rcar: fix a possible NULL dereference

Hello,

On Thu, Nov 12, 2015 at 08:25:09AM +0100, LABBE Corentin wrote:
> of_match_device could return NULL, and so cause a NULL pointer
> dereference later.
> 
> Reported-by: coverity (CID 1130036)
> Signed-off-by: LABBE Corentin <clabbe.montjoie@...il.com>
> ---
>  drivers/i2c/busses/i2c-rcar.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/i2c/busses/i2c-rcar.c b/drivers/i2c/busses/i2c-rcar.c
> index b0ae560..d2bdbda 100644
> --- a/drivers/i2c/busses/i2c-rcar.c
> +++ b/drivers/i2c/busses/i2c-rcar.c
> @@ -639,6 +639,7 @@ static int rcar_i2c_probe(struct platform_device *pdev)
>  	struct device *dev = &pdev->dev;
>  	u32 bus_speed;
>  	int irq, ret;
> +	const struct of_device_id *of_id;
>  
>  	priv = devm_kzalloc(dev, sizeof(struct rcar_i2c_priv), GFP_KERNEL);
>  	if (!priv)
> @@ -653,7 +654,10 @@ static int rcar_i2c_probe(struct platform_device *pdev)
>  	bus_speed = 100000; /* default 100 kHz */
>  	of_property_read_u32(dev->of_node, "clock-frequency", &bus_speed);
>  
> -	priv->devtype = (enum rcar_i2c_type)of_match_device(rcar_i2c_dt_ids, dev)->data;
> +	of_id = of_match_device(rcar_i2c_dt_ids, dev);
> +	if (!of_id)
> +		return -ENODEV;
> +	priv->devtype = (enum rcar_i2c_type)of_id->data;

This is nearly an open coding of of_device_get_match_data. Maybe using

	priv->devtype = (enum rcar_i2c_type)of_device_get_match_data(dev)

if good enough? 

Other than that, the NULL pointer dereference should only happen if the
device was bound using the driver name. That might be worth to point out
in the commit log. So maybe make (in a separate patch) the probe
function fail when probed by name?

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/