linux-kernel - Re: [PATCH 10/19] coresight: trbe: Always check fault action when updating buffer

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 for Android: free password hash cracker in your pocket

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

Message-ID: <be0b9b11-19d6-4d8c-910c-17773a4a2df0@arm.com>
Date: Tue, 2 Dec 2025 12:00:15 +0000
From: Suzuki K Poulose <suzuki.poulose@....com>
To: Leo Yan <leo.yan@....com>, Mike Leach <mike.leach@...aro.org>,
 James Clark <james.clark@...aro.org>,
 Anshuman Khandual <anshuman.khandual@....com>,
 Yeoreum Yun <yeoreum.yun@....com>, Will Deacon <will@...nel.org>,
 Mark Rutland <mark.rutland@....com>, Tamas Petz <tamas.petz@....com>,
 Tamas Zsoldos <tamas.zsoldos@....com>,
 Arnaldo Carvalho de Melo <acme@...nel.org>,
 Namhyung Kim <namhyung@...nel.org>, Jiri Olsa <jolsa@...nel.org>,
 Ian Rogers <irogers@...gle.com>, Adrian Hunter <adrian.hunter@...el.com>
Cc: coresight@...ts.linaro.org, linux-arm-kernel@...ts.infradead.org,
 linux-kernel@...r.kernel.org, linux-perf-users@...r.kernel.org
Subject: Re: [PATCH 10/19] coresight: trbe: Always check fault action when
 updating buffer

On 01/12/2025 11:22, Leo Yan wrote:
> The current code checks the fault action only via the IRQ status bit,
> which is unreliable due to possible hardware latency.
> 
> Move the fault action check out of the IRQ status condition.  This also
> causes the buffer size to be calculated for non-WRAP and fault cases,
> which is fine since the write pointer is trusted for the calculation.
> 
> Signed-off-by: Leo Yan <leo.yan@....com>
> ---
>   drivers/hwtracing/coresight/coresight-trbe.c | 17 +++--------------
>   1 file changed, 3 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/hwtracing/coresight/coresight-trbe.c
> index f56ecdeaa6596afb440e4d53732e08a85f9bf89d..e579ea98523c24d23a0cd265dcdd0a46b52b52da 100644
> --- a/drivers/hwtracing/coresight/coresight-trbe.c
> +++ b/drivers/hwtracing/coresight/coresight-trbe.c
> @@ -806,7 +806,6 @@ static unsigned long arm_trbe_update_buffer(struct coresight_device *csdev,
>   	enum trbe_fault_action act;
>   	unsigned long size, status;
>   	unsigned long flags;
> -	bool wrap = false;
>   
>   	WARN_ON(buf->cpudata != cpudata);
>   	WARN_ON(cpudata->cpu != smp_processor_id());
> @@ -858,21 +857,11 @@ static unsigned long arm_trbe_update_buffer(struct coresight_device *csdev,
>   		 */
>   		clr_trbe_irq();
>   		isb();
> -
> -		act = trbe_get_fault_act(handle, status);
> -		/*
> -		 * If this was not due to a WRAP event, we have some
> -		 * errors and as such buffer is empty.
> -		 */
> -		if (act != TRBE_FAULT_ACT_WRAP) {
> -			size = 0;
> -			goto done;
> -		}
> -
> -		wrap = true;
>   	}
>   
> -	size = trbe_get_trace_size(handle, buf, wrap);
> +	act = trbe_get_fault_act(handle, status);

Also act is valid only when there is an IRQ ?

> +
> +	size = trbe_get_trace_size(handle, buf, act == TRBE_FAULT_ACT_WRAP);

We have certain assumptions in trbe_get_trace_size(), which may be
broken with this change.

e.g., if we get a fatal error, we don't detect that the buffer is empty
and may trigger a WARN_ON() on systems with CPU erratum as below ?

>         /*                                                                      
>          * If the TRBE is affected by the following erratum, we must fill           
>          * the space we skipped with IGNORE packets. And we are always          
>          * guaranteed to have at least a PAGE_SIZE space in the buffer.         
>          */                                                                     
>         if (trbe_has_erratum(buf->cpudata, TRBE_WORKAROUND_OVERWRITE_FILL_MODE) &&
>             !WARN_ON(size < overwrite_skip))                                    
>                 __trbe_pad_buf(buf, start_off, overwrite_skip); 



Suzuki


>   
>   done:
>   	local_irq_restore(flags);
>