From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932999AbcFLUj3 (ORCPT ); Sun, 12 Jun 2016 16:39:29 -0400 Received: from mail-yw0-f176.google.com ([209.85.161.176]:36745 "EHLO mail-yw0-f176.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932360AbcFLUj2 (ORCPT ); Sun, 12 Jun 2016 16:39:28 -0400 MIME-Version: 1.0 In-Reply-To: <1465204301-24184-5-git-send-email-suzuki.poulose@arm.com> References: <1465204301-24184-1-git-send-email-suzuki.poulose@arm.com> <1465204301-24184-5-git-send-email-suzuki.poulose@arm.com> From: Mathieu Poirier Date: Sun, 12 Jun 2016 14:39:27 -0600 Message-ID: Subject: Re: [PATCH v2 4/9] coresight: Fix csdev connections initialisation To: Suzuki K Poulose Cc: "linux-arm-kernel@lists.infradead.org" , "linux-kernel@vger.kernel.org" Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 6 June 2016 at 03:11, Suzuki K Poulose wrote: > This is a cleanup patch. > > coresight_device->conns holds an array to point to the devices > connected to the OUT ports of a component. Sinks, e.g ETR, do not > have an OUT port (nr_outport = 0), as it streams the trace to > memory via AXI. > > At coresight_register() we do : > > conns = kcalloc(csdev->nr_outport, sizeof(*conns), GFP_KERNEL); > if (!conns) { > ret = -ENOMEM; > goto err_kzalloc_conns; > } > > For ETR, since the total size requested for kcalloc is zero, the return > value is, ZERO_SIZE_PTR ( != NULL). Hence, csdev->conns = ZERO_SIZE_PTR > which cannot be verified later to contain a valid pointer. The code which > accesses the csdev->conns is bounded by the csdev->nr_outport check, > hence we don't try to dereference the ZERO_SIZE_PTR. This patch cleans > up the csdev->conns and csdev->refcnt, initialisation to make sure we This patch no longer deals with csdev->refcnt. > initialise it properly(i.e, either NULL or valid conns array). > > Cc: Mathieu Poirier > Signed-off-by: Suzuki K Poulose > --- > drivers/hwtracing/coresight/coresight.c | 24 ++++++++++++++---------- > 1 file changed, 14 insertions(+), 10 deletions(-) > > diff --git a/drivers/hwtracing/coresight/coresight.c b/drivers/hwtracing/coresight/coresight.c > index 0fdaaf4..49eb749 100644 > --- a/drivers/hwtracing/coresight/coresight.c > +++ b/drivers/hwtracing/coresight/coresight.c > @@ -890,7 +890,7 @@ struct coresight_device *coresight_register(struct coresight_desc *desc) > int nr_refcnts = 1; > atomic_t *refcnts = NULL; > struct coresight_device *csdev; > - struct coresight_connection *conns; > + struct coresight_connection *conns = NULL; > > csdev = kzalloc(sizeof(*csdev), GFP_KERNEL); > if (!csdev) { > @@ -918,16 +918,20 @@ struct coresight_device *coresight_register(struct coresight_desc *desc) > > csdev->nr_inport = desc->pdata->nr_inport; > csdev->nr_outport = desc->pdata->nr_outport; > - conns = kcalloc(csdev->nr_outport, sizeof(*conns), GFP_KERNEL); > - if (!conns) { > - ret = -ENOMEM; > - goto err_kzalloc_conns; > - } > > - for (i = 0; i < csdev->nr_outport; i++) { > - conns[i].outport = desc->pdata->outports[i]; > - conns[i].child_name = desc->pdata->child_names[i]; > - conns[i].child_port = desc->pdata->child_ports[i]; > + /* Initialise connections if there is at least one outport */ > + if (csdev->nr_outport) { > + conns = kcalloc(csdev->nr_outport, sizeof(*conns), GFP_KERNEL); > + if (!conns) { > + ret = -ENOMEM; > + goto err_kzalloc_conns; > + } > + > + for (i = 0; i < csdev->nr_outport; i++) { > + conns[i].outport = desc->pdata->outports[i]; > + conns[i].child_name = desc->pdata->child_names[i]; > + conns[i].child_port = desc->pdata->child_ports[i]; > + } > } > > csdev->conns = conns; > -- > 1.9.1 > From mboxrd@z Thu Jan 1 00:00:00 1970 From: mathieu.poirier@linaro.org (Mathieu Poirier) Date: Sun, 12 Jun 2016 14:39:27 -0600 Subject: [PATCH v2 4/9] coresight: Fix csdev connections initialisation In-Reply-To: <1465204301-24184-5-git-send-email-suzuki.poulose@arm.com> References: <1465204301-24184-1-git-send-email-suzuki.poulose@arm.com> <1465204301-24184-5-git-send-email-suzuki.poulose@arm.com> Message-ID: To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On 6 June 2016 at 03:11, Suzuki K Poulose wrote: > This is a cleanup patch. > > coresight_device->conns holds an array to point to the devices > connected to the OUT ports of a component. Sinks, e.g ETR, do not > have an OUT port (nr_outport = 0), as it streams the trace to > memory via AXI. > > At coresight_register() we do : > > conns = kcalloc(csdev->nr_outport, sizeof(*conns), GFP_KERNEL); > if (!conns) { > ret = -ENOMEM; > goto err_kzalloc_conns; > } > > For ETR, since the total size requested for kcalloc is zero, the return > value is, ZERO_SIZE_PTR ( != NULL). Hence, csdev->conns = ZERO_SIZE_PTR > which cannot be verified later to contain a valid pointer. The code which > accesses the csdev->conns is bounded by the csdev->nr_outport check, > hence we don't try to dereference the ZERO_SIZE_PTR. This patch cleans > up the csdev->conns and csdev->refcnt, initialisation to make sure we This patch no longer deals with csdev->refcnt. > initialise it properly(i.e, either NULL or valid conns array). > > Cc: Mathieu Poirier > Signed-off-by: Suzuki K Poulose > --- > drivers/hwtracing/coresight/coresight.c | 24 ++++++++++++++---------- > 1 file changed, 14 insertions(+), 10 deletions(-) > > diff --git a/drivers/hwtracing/coresight/coresight.c b/drivers/hwtracing/coresight/coresight.c > index 0fdaaf4..49eb749 100644 > --- a/drivers/hwtracing/coresight/coresight.c > +++ b/drivers/hwtracing/coresight/coresight.c > @@ -890,7 +890,7 @@ struct coresight_device *coresight_register(struct coresight_desc *desc) > int nr_refcnts = 1; > atomic_t *refcnts = NULL; > struct coresight_device *csdev; > - struct coresight_connection *conns; > + struct coresight_connection *conns = NULL; > > csdev = kzalloc(sizeof(*csdev), GFP_KERNEL); > if (!csdev) { > @@ -918,16 +918,20 @@ struct coresight_device *coresight_register(struct coresight_desc *desc) > > csdev->nr_inport = desc->pdata->nr_inport; > csdev->nr_outport = desc->pdata->nr_outport; > - conns = kcalloc(csdev->nr_outport, sizeof(*conns), GFP_KERNEL); > - if (!conns) { > - ret = -ENOMEM; > - goto err_kzalloc_conns; > - } > > - for (i = 0; i < csdev->nr_outport; i++) { > - conns[i].outport = desc->pdata->outports[i]; > - conns[i].child_name = desc->pdata->child_names[i]; > - conns[i].child_port = desc->pdata->child_ports[i]; > + /* Initialise connections if there is at least one outport */ > + if (csdev->nr_outport) { > + conns = kcalloc(csdev->nr_outport, sizeof(*conns), GFP_KERNEL); > + if (!conns) { > + ret = -ENOMEM; > + goto err_kzalloc_conns; > + } > + > + for (i = 0; i < csdev->nr_outport; i++) { > + conns[i].outport = desc->pdata->outports[i]; > + conns[i].child_name = desc->pdata->child_names[i]; > + conns[i].child_port = desc->pdata->child_ports[i]; > + } > } > > csdev->conns = conns; > -- > 1.9.1 >