From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755437AbcHXN27 (ORCPT ); Wed, 24 Aug 2016 09:28:59 -0400 Received: from mail-wm0-f65.google.com ([74.125.82.65]:35374 "EHLO mail-wm0-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753548AbcHXN25 (ORCPT ); Wed, 24 Aug 2016 09:28:57 -0400 Date: Wed, 24 Aug 2016 14:39:11 +0200 From: Thierry Reding To: LABBE Corentin Cc: maxime.ripard@free-electrons.com, wens@csie.org, linux-pwm@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-sunxi@googlegroups.com, linux-kernel@vger.kernel.org Subject: Re: [PATCH v3] pwm: sun4i: fix a possible NULL dereference Message-ID: <20160824123911.GC3714@ulmo.ba.sec> References: <1472039038-4047-1-git-send-email-clabbe.montjoie@gmail.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="32u276st3Jlj2kUU" Content-Disposition: inline In-Reply-To: <1472039038-4047-1-git-send-email-clabbe.montjoie@gmail.com> User-Agent: Mutt/1.6.2 (2016-07-01) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org --32u276st3Jlj2kUU Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Wed, Aug 24, 2016 at 01:43:58PM +0200, LABBE Corentin wrote: > of_match_device could return NULL, and so cause a NULL pointer > dereference later. >=20 > For fixing this problem, we use of_device_get_match_data(), this will > simplify the code a little by using a standard function for > getting the match data. >=20 > Testing the return value of of_device_get_match_data is also necessary > for avoiding a second NULL deref on pwm->data. >=20 > Reported-by: coverity (CID 1324139) > Signed-off-by: LABBE Corentin > --- > Changes since v2: > - Add a test on pwm->data for avoiding a second NULL deref. > Changes since v1: > - Use of_device_get_match_data() >=20 > drivers/pwm/pwm-sun4i.c | 7 +++---- > 1 file changed, 3 insertions(+), 4 deletions(-) >=20 > diff --git a/drivers/pwm/pwm-sun4i.c b/drivers/pwm/pwm-sun4i.c > index 03a99a5..fb76c55 100644 > --- a/drivers/pwm/pwm-sun4i.c > +++ b/drivers/pwm/pwm-sun4i.c > @@ -309,9 +309,6 @@ static int sun4i_pwm_probe(struct platform_device *pd= ev) > struct resource *res; > u32 val; > int i, ret; > - const struct of_device_id *match; > - > - match =3D of_match_device(sun4i_pwm_dt_ids, &pdev->dev); > =20 > pwm =3D devm_kzalloc(&pdev->dev, sizeof(*pwm), GFP_KERNEL); > if (!pwm) > @@ -326,7 +323,9 @@ static int sun4i_pwm_probe(struct platform_device *pd= ev) > if (IS_ERR(pwm->clk)) > return PTR_ERR(pwm->clk); > =20 > - pwm->data =3D match->data; > + pwm->data =3D of_device_get_match_data(&pdev->dev); > + if (!pwm->data) > + return -EINVAL; I don't see how this would ever be NULL. You'd have to add an entry to the table that has a NULL data field, which is very unlikely to happen. If you really want to check for this, which I think isn't necessary, please wrap this in a WARN_ON() or something to make more noise when this completely unexpected case happens. Thierry --32u276st3Jlj2kUU Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAABCAAGBQJXvZVuAAoJEN0jrNd/PrOhLRcQAL7fchFrJJddfHTCC8/3v4xb te19lnpORc/irHin7WfB0epOmw+NXUCAclxJOkf2+P1NzLUCIKHAXeL7gdUK2cT/ hhssH1RCidnpqFRGEfGJZ5/VJnzNDQdQdX7ep6P9TJ0IkJ3W90ROVgb39gdk8kwj DJU+2lhvDDyg9E4ppQLcRHh1Cpm6wFIAgoom6o0gMssdDkGEakDPg+6v3BP8cxS9 0jUDhU0J95aY+sOCF/aw+AiMQPx3O5R5SVJn9aDaFCkLYjh1gi+YfAEmeusgPCsC 8yjMrpy1FwuzJcxPtKr0qu8iEPG8haJYD6wGoWfLwhZNjBUZKQJc0894V50V0Q/7 LfnAmZktlkGo92JZIcztwcB2qIYmLqI73hUkC6+LZJDq7uUbhi+PktIL4lOOsKei msSFrAXUMoBqKxlulLlCaicC8i4ZXYFwNvb64/pjA2hxbxgAGkb3NT/UvX9qsMsd xcVqIM3sidgz/PeZ8FiWZd5yOzlukYvqnbCsdfp84hxdPkeNuPcPB8aL51T255J+ d0sUN7gZ0VS1IIiITKyuEnRU780osan/9uWDjalsmDuhJN3J6qsFXQNZa3rprM+z uMrI0Twbq3zIBv2PBKU9FZzVrHS/CHKI0sU02oKARYReoSqeSO2jdJ120EaKkdOf H03UgYMn1AY8LVxqSLU1 =9/ng -----END PGP SIGNATURE----- --32u276st3Jlj2kUU--