Re: [PATCH] coresight: Fix support for sparsely populated ports

[Suzuki K Poulose]

Hi Mathieu,

On 04/10/2020 07:17 PM, Mathieu Poirier wrote:
> Hi Suzuki,
>
> On Thu, Apr 09, 2020 at 12:03:16PM +0100, Suzuki K Poulose wrote:
> > 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.

...

> > @@ -672,10 +687,14 @@ static int acpi_coresight_parse_graph(struct acpi_device *adev,
> >   			return dir;
> >   
> >   		if (dir == ACPI_CORESIGHT_LINK_MASTER) {
> > -			pdata->nr_outport++;
> > +			if (ptr->outport > pdata->nr_outport)
> > +				pdata->nr_outport = ptr->outport;
> >   			ptr++;
> >   		} else {
> > -			pdata->nr_inport++;
> > +			WARN_ON(pdata->nr_inport == ptr->child_port);
> > +			/* Do not move the ptr for input connections */
> > +			if (ptr->child_port > pdata->nr_inport)
> > +				pdata->nr_inport = ptr->child_port;
>
> How you are using the current ptr as a scratch pad for input port was definitely
> a brain twister this morning...  I would certainly appreciate a richer comment
> so that I (or anyone else) doesn't have to go through the same process the next
> time around.

Sure, it deserves a better comment. I will add something like :

	/*
	 * We don't track input connection details for a device,
	 * except for the highest input port number. Thus we could
	 * reuse the current record as a scratch pad and reuse it
	 * by not moving the ptr ahead.
	 */



> >   /**
> > - * struct coresight_platform_data - data harvested from the DT specification
> > - * @nr_inport:	number of input ports for this component.
> > - * @nr_outport:	number of output ports for this component.
> > - * @conns:	Array of nr_outport connections from this component
> > + * struct coresight_platform_data - data harvested from the firmware
> > + * specification.
> > + *
> > + * @nr_inport:	Number of elements for the input connections.
> > + * @nr_outport:	Number of elements for the output connections.
> > + * @conns:	Sparse arrray of nr_outport connections from this component.
>
> s/arrray/array
>
> Please rebase your work on my the coresight-next branch.  Other than the above
> this patch looks fine to me.

Thanks for the heads up, will do.

Cheers
Suzuki