2017-06-12 22:50:54

by Gustavo A. R. Silva

Subject: Re: [PATCH] char: tmp: fix potential null pointer dereference

Hi Jarkko,

Please, see my comments below

Quoting Jarkko Sakkinen <[email protected]>:

> On Tue, May 30, 2017 at 04:51:23PM -0500, Gustavo A. R. Silva wrote:
>> NULL check at line 147: if (chip) {, implies chip might be NULL.
>> Function dev_get_drvdata() dereference pointer chip.
>> Move pointer priv assignment inside the IF block that checks
>> pointer chip.
>>
>> Addresses-Coverity-ID: 1397646
>> Signed-off-by: Gustavo A. R. Silva <[email protected]>
>
> It cannot be.
>

I got it.

> /Jarkko
>
>> ---
>> drivers/char/tpm/tpm_atmel.c | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/char/tpm/tpm_atmel.c b/drivers/char/tpm/tpm_atmel.c
>> index 0d322ab..0826efd 100644
>> --- a/drivers/char/tpm/tpm_atmel.c
>> +++ b/drivers/char/tpm/tpm_atmel.c
>> @@ -142,9 +142,10 @@ static struct platform_device *pdev;
>> static void atml_plat_remove(void)
>> {
>> struct tpm_chip *chip = dev_get_drvdata(&pdev->dev);
>> - struct tpm_atmel_priv *priv = dev_get_drvdata(&chip->dev);
>> + struct tpm_atmel_priv *priv;
>>
>> if (chip) {

So, this NULL check could be removed?

>> + priv = dev_get_drvdata(&chip->dev);
>> tpm_chip_unregister(chip);
>> if (priv->have_region)
>> atmel_release_region(priv->base, priv->region_size);
>> --
>> 2.5.0
>>

Thank you
--
Gustavo A. R. Silva

2017-06-13 18:03:26

by Jarkko Sakkinen

Subject: Re: [PATCH] char: tmp: fix potential null pointer dereference

On Mon, Jun 12, 2017 at 05:25:44PM -0500, Gustavo A. R. Silva wrote:
> Hi Jarkko,
>
> Please, see my comments below
>
> Quoting Jarkko Sakkinen <[email protected]>:
>
> > On Tue, May 30, 2017 at 04:51:23PM -0500, Gustavo A. R. Silva wrote:
> > > NULL check at line 147: if (chip) {, implies chip might be NULL.
> > > Function dev_get_drvdata() dereference pointer chip.
> > > Move pointer priv assignment inside the IF block that checks
> > > pointer chip.
> > >
> > > Addresses-Coverity-ID: 1397646
> > > Signed-off-by: Gustavo A. R. Silva <[email protected]>
> >
> > It cannot be.
> >
>
> I got it.
>
> > /Jarkko
> >
> > > ---
> > > drivers/char/tpm/tpm_atmel.c | 3 ++-
> > > 1 file changed, 2 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/char/tpm/tpm_atmel.c b/drivers/char/tpm/tpm_atmel.c
> > > index 0d322ab..0826efd 100644
> > > --- a/drivers/char/tpm/tpm_atmel.c
> > > +++ b/drivers/char/tpm/tpm_atmel.c
> > > @@ -142,9 +142,10 @@ static struct platform_device *pdev;
> > > static void atml_plat_remove(void)
> > > {
> > > struct tpm_chip *chip = dev_get_drvdata(&pdev->dev);
> > > - struct tpm_atmel_priv *priv = dev_get_drvdata(&chip->dev);
> > > + struct tpm_atmel_priv *priv;
> > >
> > > if (chip) {
>
> So, this NULL check could be removed?

Yes, this would be right way to fix it.

/Jarkko

2017-06-13 19:55:47

by Gustavo A. R. Silva

Subject: [PATCH] char: tpm: remove unnecessary NULL check

Remove unnecessary NULL check.
Pointer _chip_ cannot be NULL in this instance.

Addresses-Coverity-ID: 1397646
Cc: Jarkko Sakkinen <[email protected]>
Signed-off-by: Gustavo A. R. Silva <[email protected]>
---
drivers/char/tpm/tpm_atmel.c | 12 +++++-------
1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/drivers/char/tpm/tpm_atmel.c b/drivers/char/tpm/tpm_atmel.c
index 0d322ab..66a1452 100644
--- a/drivers/char/tpm/tpm_atmel.c
+++ b/drivers/char/tpm/tpm_atmel.c
@@ -144,13 +144,11 @@ static void atml_plat_remove(void)
struct tpm_chip *chip = dev_get_drvdata(&pdev->dev);
struct tpm_atmel_priv *priv = dev_get_drvdata(&chip->dev);

- if (chip) {
- tpm_chip_unregister(chip);
- if (priv->have_region)
- atmel_release_region(priv->base, priv->region_size);
- atmel_put_base_addr(priv->iobase);
- platform_device_unregister(pdev);
- }
+ tpm_chip_unregister(chip);
+ if (priv->have_region)
+ atmel_release_region(priv->base, priv->region_size);
+ atmel_put_base_addr(priv->iobase);
+ platform_device_unregister(pdev);
}

static SIMPLE_DEV_PM_OPS(tpm_atml_pm, tpm_pm_suspend, tpm_pm_resume);
--
2.5.0

2017-06-14 09:46:37

by Jarkko Sakkinen

Subject: Re: [PATCH] char: tpm: remove unnecessary NULL check

On Tue, Jun 13, 2017 at 02:55:42PM -0500, Gustavo A. R. Silva wrote:
> Remove unnecessary NULL check.
> Pointer _chip_ cannot be NULL in this instance.
>
> Addresses-Coverity-ID: 1397646
> Cc: Jarkko Sakkinen <[email protected]>
> Signed-off-by: Gustavo A. R. Silva <[email protected]>
> ---
> drivers/char/tpm/tpm_atmel.c | 12 +++++-------
> 1 file changed, 5 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/char/tpm/tpm_atmel.c b/drivers/char/tpm/tpm_atmel.c
> index 0d322ab..66a1452 100644
> --- a/drivers/char/tpm/tpm_atmel.c
> +++ b/drivers/char/tpm/tpm_atmel.c
> @@ -144,13 +144,11 @@ static void atml_plat_remove(void)
> struct tpm_chip *chip = dev_get_drvdata(&pdev->dev);
> struct tpm_atmel_priv *priv = dev_get_drvdata(&chip->dev);
>
> - if (chip) {
> - tpm_chip_unregister(chip);
> - if (priv->have_region)
> - atmel_release_region(priv->base, priv->region_size);
> - atmel_put_base_addr(priv->iobase);
> - platform_device_unregister(pdev);
> - }
> + tpm_chip_unregister(chip);
> + if (priv->have_region)
> + atmel_release_region(priv->base, priv->region_size);
> + atmel_put_base_addr(priv->iobase);
> + platform_device_unregister(pdev);
> }
>
> static SIMPLE_DEV_PM_OPS(tpm_atml_pm, tpm_pm_suspend, tpm_pm_resume);
> --
> 2.5.0
>

Thank you.

Reviewed-by: Jarkko Sakkinen <[email protected]>

/Jarkko

2017-06-19 00:35:47

by Jarkko Sakkinen

Subject: Re: [PATCH] char: tpm: remove unnecessary NULL check

On Tue, 2017-06-13 at 14:55 -0500, Gustavo A. R. Silva wrote:
> Remove unnecessary NULL check.
> Pointer _chip_ cannot be NULL in this instance.
> 
> Addresses-Coverity-ID: 1397646
> Cc: Jarkko Sakkinen <[email protected]>
> Signed-off-by: Gustavo A. R. Silva <[email protected]>

Reviewed-by: Jarkko Sakkinen <[email protected]>
Tested-by: Jarkko Sakkinen <[email protected]>
(compilation)

/Jarkko