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

[Suzuki K Poulose]

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@xxxxxxx>
> > ---
> > Â.../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