Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1420321 > unrolled thread

Re: [PATCH v2 4/9] coresight: Fix csdev connections initialisation

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH v2 4/9] coresight: Fix csdev connections initialisation Mathieu Poirier <mathieu.poirier@linaro.org> - 2016-06-12 22:40 +0200
    Re: [PATCH v2 4/9] coresight: Fix csdev connections initialisation Suzuki K Poulose <Suzuki.Poulose@arm.com> - 2016-06-13 11:00 +0200
      Re: [PATCH v2 4/9] coresight: Fix csdev connections initialisation Mathieu Poirier <mathieu.poirier@linaro.org> - 2016-06-13 16:40 +0200

#1420321 — Re: [PATCH v2 4/9] coresight: Fix csdev connections initialisation

On 6 June 2016 at 03:11, Suzuki K Poulose <suzuki.poulose@arm.com> 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 <mathieu.poirier@linaro.org>
> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
> ---
>  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
>

[toc] | [next] | [standalone]

#1420632

On 12/06/16 21:39, Mathieu Poirier wrote:
> On 6 June 2016 at 03:11, Suzuki K Poulose <suzuki.poulose@arm.com> 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.

Ok, fill fix that. Btw, do we need that check ? I am tempted to keep it there,
just to make sure we don't end up in something similar in the future.

Cheers
Suzuki

[toc] | [prev] | [next] | [standalone]

#1420937

On 13 June 2016 at 02:54, Suzuki K Poulose <Suzuki.Poulose@arm.com> wrote:
> On 12/06/16 21:39, Mathieu Poirier wrote:
>>
>> On 6 June 2016 at 03:11, Suzuki K Poulose <suzuki.poulose@arm.com> 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.
>
>
> Ok, fill fix that. Btw, do we need that check ? I am tempted to keep it
> there,
> just to make sure we don't end up in something similar in the future.

If it becomes an issue in the future we'll know what to do :o)

Thanks,
Mathieu

>
> Cheers
> Suzuki

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web