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

[Sai Prakash Ranjan]

Hi Suzuki,

On 2020-04-09 14:47, Suzuki K Poulose wrote:
> 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.

I don't see the ETR probe failure and the KASAN warning anymore with 
this change.

Tested-by: Sai Prakash Ranjan <saiprakash.ranjan@xxxxxxxxxxxxxx>

Thanks,
Sai

--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a 
member
of Code Aurora Forum, hosted by The Linux Foundation