diff mbox series

[25/28] coresight: etm3x: Smatch: Fix potential NULL pointer dereference

Message ID 20190619195318.19254-26-mathieu.poirier@linaro.org (mailing list archive)
State Mainlined
Commit 020601622323d02e09cebe05e7976b3d4b44e05d
Headers show
Series coresight: next v5.2-rc5 (V2) | expand

Commit Message

Mathieu Poirier June 19, 2019, 7:53 p.m. UTC 
From: Suzuki K Poulose <suzuki.poulose@arm.com>

Based on the following report from  Smatch tool, make sure we have a
valid drvdata before we dereference it to find the real dev.

The patch 21d26b905c05: "coresight: etm: Clean up device specific
data" from May 22, 2019, leads to the following Smatch complaint:

    ./drivers/hwtracing/coresight/coresight-etm3x.c:460 etm_get_trace_id()
    warn: variable dereferenced before check 'drvdata' (see line 458)

./drivers/hwtracing/coresight/coresight-etm3x.c
   457		int trace_id = -1;
   458		struct device *etm_dev = drvdata->csdev->dev.parent;
                                         ^^^^^^^^^
New dereference

   459
   460		if (!drvdata)
                    ^^^^^^^^
Checked too late.  Delete the check?

   461			goto out;
   462

Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
Cc: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
---
 drivers/hwtracing/coresight/coresight-etm3x.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Greg KH June 20, 2019, 6:04 a.m. UTC | #1 
On Wed, Jun 19, 2019 at 01:53:15PM -0600, Mathieu Poirier wrote:
> From: Suzuki K Poulose <suzuki.poulose@arm.com>
> 
> Based on the following report from  Smatch tool, make sure we have a
> valid drvdata before we dereference it to find the real dev.
> 
> The patch 21d26b905c05: "coresight: etm: Clean up device specific
> data" from May 22, 2019, leads to the following Smatch complaint:
> 
>     ./drivers/hwtracing/coresight/coresight-etm3x.c:460 etm_get_trace_id()
>     warn: variable dereferenced before check 'drvdata' (see line 458)
> 
> ./drivers/hwtracing/coresight/coresight-etm3x.c
>    457		int trace_id = -1;
>    458		struct device *etm_dev = drvdata->csdev->dev.parent;
>                                          ^^^^^^^^^
> New dereference
> 
>    459
>    460		if (!drvdata)
>                     ^^^^^^^^
> Checked too late.  Delete the check?
> 
>    461			goto out;
>    462
> 
> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
> Cc: Dan Carpenter <dan.carpenter@oracle.com>
> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
> Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
> ---
>  drivers/hwtracing/coresight/coresight-etm3x.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/hwtracing/coresight/coresight-etm3x.c b/drivers/hwtracing/coresight/coresight-etm3x.c
> index bed729140718..225c2982e4fe 100644
> --- a/drivers/hwtracing/coresight/coresight-etm3x.c
> +++ b/drivers/hwtracing/coresight/coresight-etm3x.c
> @@ -455,11 +455,12 @@ int etm_get_trace_id(struct etm_drvdata *drvdata)
>  {
>  	unsigned long flags;
>  	int trace_id = -1;
> -	struct device *etm_dev = drvdata->csdev->dev.parent;
> +	struct device *etm_dev;
>  
>  	if (!drvdata)
>  		goto out;
>  
> +	etm_dev = drvdata->csdev->dev.parent;
>  	if (!local_read(&drvdata->mode))
>  		return drvdata->traceid;
>  
> -- 
> 2.17.1
> 

Looks like 5.2 and possibly stable material, right?
Mathieu Poirier June 20, 2019, 9:38 p.m. UTC | #2 
On Thu, 20 Jun 2019 at 00:05, Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Wed, Jun 19, 2019 at 01:53:15PM -0600, Mathieu Poirier wrote:
> > From: Suzuki K Poulose <suzuki.poulose@arm.com>
> >
> > Based on the following report from  Smatch tool, make sure we have a
> > valid drvdata before we dereference it to find the real dev.
> >
> > The patch 21d26b905c05: "coresight: etm: Clean up device specific
> > data" from May 22, 2019, leads to the following Smatch complaint:
> >
> >     ./drivers/hwtracing/coresight/coresight-etm3x.c:460 etm_get_trace_id()
> >     warn: variable dereferenced before check 'drvdata' (see line 458)
> >
> > ./drivers/hwtracing/coresight/coresight-etm3x.c
> >    457                int trace_id = -1;
> >    458                struct device *etm_dev = drvdata->csdev->dev.parent;
> >                                          ^^^^^^^^^
> > New dereference
> >
> >    459
> >    460                if (!drvdata)
> >                     ^^^^^^^^
> > Checked too late.  Delete the check?
> >
> >    461                        goto out;
> >    462
> >
> > Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
> > Cc: Dan Carpenter <dan.carpenter@oracle.com>
> > Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
> > Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
> > ---
> >  drivers/hwtracing/coresight/coresight-etm3x.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/hwtracing/coresight/coresight-etm3x.c b/drivers/hwtracing/coresight/coresight-etm3x.c
> > index bed729140718..225c2982e4fe 100644
> > --- a/drivers/hwtracing/coresight/coresight-etm3x.c
> > +++ b/drivers/hwtracing/coresight/coresight-etm3x.c
> > @@ -455,11 +455,12 @@ int etm_get_trace_id(struct etm_drvdata *drvdata)
> >  {
> >       unsigned long flags;
> >       int trace_id = -1;
> > -     struct device *etm_dev = drvdata->csdev->dev.parent;
> > +     struct device *etm_dev;
> >
> >       if (!drvdata)
> >               goto out;
> >
> > +     etm_dev = drvdata->csdev->dev.parent;
> >       if (!local_read(&drvdata->mode))
> >               return drvdata->traceid;
> >
> > --
> > 2.17.1
> >
>
> Looks like 5.2 and possibly stable material, right?

This patch is fixing code that was introduced by patch 12/45 [1] of
the original set - it should really be applied on top you
char-misc-next for the 5.3 merge window.

[1]. https://www.spinics.net/lists/arm-kernel/msg736150.html
Greg KH June 21, 2019, 6:59 a.m. UTC | #3 
On Thu, Jun 20, 2019 at 03:38:58PM -0600, Mathieu Poirier wrote:
> On Thu, 20 Jun 2019 at 00:05, Greg KH <gregkh@linuxfoundation.org> wrote:
> >
> > On Wed, Jun 19, 2019 at 01:53:15PM -0600, Mathieu Poirier wrote:
> > > From: Suzuki K Poulose <suzuki.poulose@arm.com>
> > >
> > > Based on the following report from  Smatch tool, make sure we have a
> > > valid drvdata before we dereference it to find the real dev.
> > >
> > > The patch 21d26b905c05: "coresight: etm: Clean up device specific
> > > data" from May 22, 2019, leads to the following Smatch complaint:
> > >
> > >     ./drivers/hwtracing/coresight/coresight-etm3x.c:460 etm_get_trace_id()
> > >     warn: variable dereferenced before check 'drvdata' (see line 458)
> > >
> > > ./drivers/hwtracing/coresight/coresight-etm3x.c
> > >    457                int trace_id = -1;
> > >    458                struct device *etm_dev = drvdata->csdev->dev.parent;
> > >                                          ^^^^^^^^^
> > > New dereference
> > >
> > >    459
> > >    460                if (!drvdata)
> > >                     ^^^^^^^^
> > > Checked too late.  Delete the check?
> > >
> > >    461                        goto out;
> > >    462
> > >
> > > Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
> > > Cc: Dan Carpenter <dan.carpenter@oracle.com>
> > > Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
> > > Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
> > > ---
> > >  drivers/hwtracing/coresight/coresight-etm3x.c | 3 ++-
> > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/hwtracing/coresight/coresight-etm3x.c b/drivers/hwtracing/coresight/coresight-etm3x.c
> > > index bed729140718..225c2982e4fe 100644
> > > --- a/drivers/hwtracing/coresight/coresight-etm3x.c
> > > +++ b/drivers/hwtracing/coresight/coresight-etm3x.c
> > > @@ -455,11 +455,12 @@ int etm_get_trace_id(struct etm_drvdata *drvdata)
> > >  {
> > >       unsigned long flags;
> > >       int trace_id = -1;
> > > -     struct device *etm_dev = drvdata->csdev->dev.parent;
> > > +     struct device *etm_dev;
> > >
> > >       if (!drvdata)
> > >               goto out;
> > >
> > > +     etm_dev = drvdata->csdev->dev.parent;
> > >       if (!local_read(&drvdata->mode))
> > >               return drvdata->traceid;
> > >
> > > --
> > > 2.17.1
> > >
> >
> > Looks like 5.2 and possibly stable material, right?
> 
> This patch is fixing code that was introduced by patch 12/45 [1] of
> the original set - it should really be applied on top you
> char-misc-next for the 5.3 merge window.
> 
> [1]. https://www.spinics.net/lists/arm-kernel/msg736150.html

Ok, can you resend it then?

thanks,
greg k-h
diff mbox series

Patch

diff --git a/drivers/hwtracing/coresight/coresight-etm3x.c b/drivers/hwtracing/coresight/coresight-etm3x.c
index bed729140718..225c2982e4fe 100644
--- a/drivers/hwtracing/coresight/coresight-etm3x.c
+++ b/drivers/hwtracing/coresight/coresight-etm3x.c
@@ -455,11 +455,12 @@  int etm_get_trace_id(struct etm_drvdata *drvdata)
 {
 	unsigned long flags;
 	int trace_id = -1;
-	struct device *etm_dev = drvdata->csdev->dev.parent;
+	struct device *etm_dev;
 
 	if (!drvdata)
 		goto out;
 
+	etm_dev = drvdata->csdev->dev.parent;
 	if (!local_read(&drvdata->mode))
 		return drvdata->traceid;