linux-kernel - Re: [RFC PATCH] coresight: dynamic-replicator: Fix handling of multiple connections

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <671892c3-b90d-73f0-4706-b74b40002260@arm.com>
Date:   Thu, 9 Apr 2020 10:17:03 +0100
From:   Suzuki K Poulose <suzuki.poulose@....com>
To:     saiprakash.ranjan@...eaurora.org
Cc:     mike.leach@...aro.org, mathieu.poirier@...aro.org,
        leo.yan@...aro.org, alexander.shishkin@...ux.intel.com,
        swboyd@...omium.org, linux-arm-kernel@...ts.infradead.org,
        linux-kernel@...r.kernel.org, linux-arm-msm@...r.kernel.org
Subject: Re: [RFC PATCH] coresight: dynamic-replicator: Fix handling of
 multiple connections

Hi Sai,


Thanks for the quick testing ! Please see below for the
tmc_etr probe failure.

On 04/09/2020 08:51 AM, Sai Prakash Ranjan wrote:
> Hi Suzuki,
> 
> On 2020-04-09 04:13, Suzuki K Poulose wrote:
>> On Tue, Apr 07, 2020 at 08:48:54PM +0530, Sai Prakash Ranjan wrote:
>>
>> Please find the untested patch below.
>>
>> ---8>---
>>
>> [untested] coresight: Fix support for sparse port numbers
>>
>> On some systems the firmware may not describe all the ports
>> connected to a component (e.g, for security reasons). This
>> could be especially problematic for "funnels" where we could
>> end up in modifying memory beyond the allocated space for
>> refcounts.
>>
>> e.g, for a funnel with input ports listed 0, 3, 5, nr_inport = 3.
>> However the we could access refcnts[5] while checking for
>> references.
>>
>> Signed-off-by: Suzuki K Poulose <suzuki.poulose@....com>
>> ---
>>  .../hwtracing/coresight/coresight-platform.c  | 74 ++++++++++++-------
>>  drivers/hwtracing/coresight/coresight.c       |  8 +-
>>  2 files changed, 56 insertions(+), 26 deletions(-)
>>
>> diff --git a/drivers/hwtracing/coresight/coresight-platform.c
>> b/drivers/hwtracing/coresight/coresight-platform.c
>> index 3c5bee429105..1c610d6e944b 100644
>> --- a/drivers/hwtracing/coresight/coresight-platform.c
>> +++ b/drivers/hwtracing/coresight/coresight-platform.c
>> @@ -67,6 +67,7 @@ static void of_coresight_get_ports_legacy(const

[...]

>> @@ -684,8 +702,13 @@ static int acpi_coresight_parse_graph(struct
>> acpi_device *adev,
>>          return rc;
>>
>>      /* Copy the connection information to the final location */
>> -    for (i = 0; i < pdata->nr_outport; i++)
>> -        pdata->conns[i] = conns[i];
>> +    for (i = 0; conns + i < ptr; i++) {
>> +        int port = conns[i].outport;
>> +
>> +        /* Duplicate output port */
>> +        WARN_ON(pdata->conns[port].child_fwnode);
>> +        pdata->conns[port] = conns[i];
>> +    }
>>
>>      devm_kfree(&adev->dev, conns);
>>      return 0;
>> @@ -787,6 +810,7 @@ coresight_get_platform_data(struct device *dev)
>>          goto error;
>>
>>      pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
>> +    pdata->nr_outport = pdata->nr_inport = -1;


Please could you remove this hunk and test it ? I forgot to update the
commit before I sent this over.


>>              /* Does it match this newly added device? */
>> @@ -1029,6 +1032,9 @@ static void coresight_fixup_device_conns(struct
>> coresight_device *csdev)
>>          struct coresight_connection *conn = &csdev->pdata->conns[i];
>>          struct device *dev = NULL;
>>
>> +        if (!conn->child_fwnode)
>> +            continue;
>> +
>>          dev = bus_find_device_by_fwnode(&coresight_bustype, 
>> conn->child_fwnode);
>>          if (dev) {
>>              conn->child_dev = to_coresight_device(dev);
>> @@ -1061,7 +1067,7 @@ static int coresight_remove_match(struct device
>> *dev, void *data)
>>      for (i = 0; i < iterator->pdata->nr_outport; i++) {
>>          conn = &iterator->pdata->conns[i];
>>
>> -        if (conn->child_dev == NULL)
>> +        if (conn->child_dev == NULL || conn->child_fwnode == NULL)
>>              continue;
>>
>>          if (csdev->dev.fwnode == conn->child_fwnode) {
> 
> 
> Thanks Suzuki, I don't see the KASAN warning anymore with this patch.
> But somehow tmc_etr probe fails with error -12(ENOMEM).

See the above suggestion.

Cheers
Suzuki