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

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] [day] [month] [year] [list]

Date:   Tue, 14 Apr 2020 11:25:57 +0100
From:   Suzuki K Poulose <suzuki.poulose@....com>
To:     mathieu.poirier@...aro.org
Cc:     linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
        coresight@...ts.linaro.org, saiprakash.ranjan@...eaurora.org,
        leo.yan@...aro.org, linux-arm-msm@...r.kernel.org,
        swboyd@...omium.org, mike.leach@...aro.org
Subject: Re: [PATCH] coresight: Fix support for sparsely populated ports

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