| Message ID | 20190619195318.19254-27-mathieu.poirier@linaro.org (mailing list archive) |
|---|---|
| State | Mainlined |
| Commit | 17c20f329a139e90e4b049b811ca6d1a2ae967e0 |
| Headers | show |
| Series | coresight: next v5.2-rc5 (V2) | expand |
On Wed, Jun 19, 2019 at 01:53:16PM -0600, Mathieu Poirier wrote: > From: Suzuki K Poulose <suzuki.poulose@arm.com> > > Based on the following report from Smatch, fix the potential > NULL pointer dereference check. > > The patch 743256e214e8: "coresight: tmc: Clean up device specific > data" from May 22, 2019, leads to the following Smatch complaint: > > drivers/hwtracing/coresight/coresight-tmc-etr.c:625 tmc_etr_free_flat_buf() > warn: variable dereferenced before check 'flat_buf' (see line 623) > > drivers/hwtracing/coresight/coresight-tmc-etr.c > 622 struct etr_flat_buf *flat_buf = etr_buf->private; > 623 struct device *real_dev = flat_buf->dev->parent; > ^^^^^^^^^^ > The patch introduces a new NULL check > > 624 > 625 if (flat_buf && flat_buf->daddr) > ^^^^^^^^ > but the existing code assumed it can be NULL. > > 626 dma_free_coherent(real_dev, flat_buf->size, > 627 flat_buf->vaddr, flat_buf->daddr); > > Cc: Dan Carpenter <dan.carpenter@oracle.com> > Cc: Mathieu Poirier <mathieu.poirier@linaro.org> > Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com> > Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org> > --- > drivers/hwtracing/coresight/coresight-tmc-etr.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c > index 5d2bf6d18961..17006705287a 100644 > --- a/drivers/hwtracing/coresight/coresight-tmc-etr.c > +++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c > @@ -620,11 +620,13 @@ static int tmc_etr_alloc_flat_buf(struct tmc_drvdata *drvdata, > static void tmc_etr_free_flat_buf(struct etr_buf *etr_buf) > { > struct etr_flat_buf *flat_buf = etr_buf->private; > - struct device *real_dev = flat_buf->dev->parent; > > - if (flat_buf && flat_buf->daddr) > + if (flat_buf && flat_buf->daddr) { > + struct device *real_dev = flat_buf->dev->parent; > + > dma_free_coherent(real_dev, flat_buf->size, > flat_buf->vaddr, flat_buf->daddr); > + } > kfree(flat_buf); > } > > -- > 2.17.1 > Again, 5.2-final and stable...
On Thu, 20 Jun 2019 at 00:05, Greg KH <gregkh@linuxfoundation.org> wrote: > > On Wed, Jun 19, 2019 at 01:53:16PM -0600, Mathieu Poirier wrote: > > From: Suzuki K Poulose <suzuki.poulose@arm.com> > > > > Based on the following report from Smatch, fix the potential > > NULL pointer dereference check. > > > > The patch 743256e214e8: "coresight: tmc: Clean up device specific > > data" from May 22, 2019, leads to the following Smatch complaint: > > > > drivers/hwtracing/coresight/coresight-tmc-etr.c:625 tmc_etr_free_flat_buf() > > warn: variable dereferenced before check 'flat_buf' (see line 623) > > > > drivers/hwtracing/coresight/coresight-tmc-etr.c > > 622 struct etr_flat_buf *flat_buf = etr_buf->private; > > 623 struct device *real_dev = flat_buf->dev->parent; > > ^^^^^^^^^^ > > The patch introduces a new NULL check > > > > 624 > > 625 if (flat_buf && flat_buf->daddr) > > ^^^^^^^^ > > but the existing code assumed it can be NULL. > > > > 626 dma_free_coherent(real_dev, flat_buf->size, > > 627 flat_buf->vaddr, flat_buf->daddr); > > > > Cc: Dan Carpenter <dan.carpenter@oracle.com> > > Cc: Mathieu Poirier <mathieu.poirier@linaro.org> > > Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com> > > Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org> > > --- > > drivers/hwtracing/coresight/coresight-tmc-etr.c | 6 ++++-- > > 1 file changed, 4 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c > > index 5d2bf6d18961..17006705287a 100644 > > --- a/drivers/hwtracing/coresight/coresight-tmc-etr.c > > +++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c > > @@ -620,11 +620,13 @@ static int tmc_etr_alloc_flat_buf(struct tmc_drvdata *drvdata, > > static void tmc_etr_free_flat_buf(struct etr_buf *etr_buf) > > { > > struct etr_flat_buf *flat_buf = etr_buf->private; > > - struct device *real_dev = flat_buf->dev->parent; > > > > - if (flat_buf && flat_buf->daddr) > > + if (flat_buf && flat_buf->daddr) { > > + struct device *real_dev = flat_buf->dev->parent; > > + > > dma_free_coherent(real_dev, flat_buf->size, > > flat_buf->vaddr, flat_buf->daddr); > > + } > > kfree(flat_buf); > > } > > > > -- > > 2.17.1 > > > > Again, 5.2-final and stable... So is this one, if addresses a deficiency introduced in patch 8/45 [1]. I have a new set ready for the other ones you flagged. Thanks, Mathieu [1]. https://www.spinics.net/lists/arm-kernel/msg736144.html
On Thu, Jun 20, 2019 at 03:42:18PM -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:16PM -0600, Mathieu Poirier wrote: > > > From: Suzuki K Poulose <suzuki.poulose@arm.com> > > > > > > Based on the following report from Smatch, fix the potential > > > NULL pointer dereference check. > > > > > > The patch 743256e214e8: "coresight: tmc: Clean up device specific > > > data" from May 22, 2019, leads to the following Smatch complaint: > > > > > > drivers/hwtracing/coresight/coresight-tmc-etr.c:625 tmc_etr_free_flat_buf() > > > warn: variable dereferenced before check 'flat_buf' (see line 623) > > > > > > drivers/hwtracing/coresight/coresight-tmc-etr.c > > > 622 struct etr_flat_buf *flat_buf = etr_buf->private; > > > 623 struct device *real_dev = flat_buf->dev->parent; > > > ^^^^^^^^^^ > > > The patch introduces a new NULL check > > > > > > 624 > > > 625 if (flat_buf && flat_buf->daddr) > > > ^^^^^^^^ > > > but the existing code assumed it can be NULL. > > > > > > 626 dma_free_coherent(real_dev, flat_buf->size, > > > 627 flat_buf->vaddr, flat_buf->daddr); > > > > > > Cc: Dan Carpenter <dan.carpenter@oracle.com> > > > Cc: Mathieu Poirier <mathieu.poirier@linaro.org> > > > Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com> > > > Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org> > > > --- > > > drivers/hwtracing/coresight/coresight-tmc-etr.c | 6 ++++-- > > > 1 file changed, 4 insertions(+), 2 deletions(-) > > > > > > diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c > > > index 5d2bf6d18961..17006705287a 100644 > > > --- a/drivers/hwtracing/coresight/coresight-tmc-etr.c > > > +++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c > > > @@ -620,11 +620,13 @@ static int tmc_etr_alloc_flat_buf(struct tmc_drvdata *drvdata, > > > static void tmc_etr_free_flat_buf(struct etr_buf *etr_buf) > > > { > > > struct etr_flat_buf *flat_buf = etr_buf->private; > > > - struct device *real_dev = flat_buf->dev->parent; > > > > > > - if (flat_buf && flat_buf->daddr) > > > + if (flat_buf && flat_buf->daddr) { > > > + struct device *real_dev = flat_buf->dev->parent; > > > + > > > dma_free_coherent(real_dev, flat_buf->size, > > > flat_buf->vaddr, flat_buf->daddr); > > > + } > > > kfree(flat_buf); > > > } > > > > > > -- > > > 2.17.1 > > > > > > > Again, 5.2-final and stable... > > So is this one, if addresses a deficiency introduced in patch 8/45 > [1]. I have a new set ready for the other ones you flagged. Ok, can you please resend? thanks, greg k-h
diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c index 5d2bf6d18961..17006705287a 100644 --- a/drivers/hwtracing/coresight/coresight-tmc-etr.c +++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c @@ -620,11 +620,13 @@ static int tmc_etr_alloc_flat_buf(struct tmc_drvdata *drvdata, static void tmc_etr_free_flat_buf(struct etr_buf *etr_buf) { struct etr_flat_buf *flat_buf = etr_buf->private; - struct device *real_dev = flat_buf->dev->parent; - if (flat_buf && flat_buf->daddr) + if (flat_buf && flat_buf->daddr) { + struct device *real_dev = flat_buf->dev->parent; + dma_free_coherent(real_dev, flat_buf->size, flat_buf->vaddr, flat_buf->daddr); + } kfree(flat_buf); }