[PATCH v3] coresight: ETR: Fix ETR buffer use-after-free issue

[PATCH v3] coresight: ETR: Fix ETR buffer use-after-free issue

[Xiaoqi Zhuang]

When ETR is enabled as CS_MODE_SYSFS, if the buffer size is changed
and enabled again, currently sysfs_buf will point to the newly
allocated memory(buf_new) and free the old memory(buf_old). But the
etr_buf that is being used by the ETR remains pointed to buf_old, not
updated to buf_new. In this case, it will result in a memory
use-after-free issue.

Fix this by checking ETR's mode before updating and releasing buf_old,
if the mode is CS_MODE_SYSFS, then skip updating and releasing it.

Fixes: bd2767ec3df2 ("coresight: Fix run time warnings while reusing ETR buffer")
Signed-off-by: Xiaoqi Zhuang <xiaoqi.zhuang@oss.qualcomm.com>
---
Changes in v3:
- Add a fix tag for the fix patch.
- Link to v2: https://lore.kernel.org/r/20251021-fix_etr_issue-v2-1-80c40c9cac8c@oss.qualcomm.com

Changes in v2:
- Exit earlier to avoid allocating memory unnecessarily.
- Link to v1: https://lore.kernel.org/r/20251020-fix_etr_issue-v1-1-902ab51770b4@oss.qualcomm.com
---
 drivers/hwtracing/coresight/coresight-tmc-etr.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c
index b07fcdb3fe1a..800be06598c1 100644
--- a/drivers/hwtracing/coresight/coresight-tmc-etr.c
+++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c
@@ -1250,6 +1250,13 @@ static struct etr_buf *tmc_etr_get_sysfs_buffer(struct coresight_device *csdev)
 	 * with the lock released.
 	 */
 	raw_spin_lock_irqsave(&drvdata->spinlock, flags);
+
+	/*
+	 * If the ETR is already enabled, continue with the existing buffer.
+	 */
+	if (coresight_get_mode(csdev) == CS_MODE_SYSFS)
+		goto out;
+
 	sysfs_buf = READ_ONCE(drvdata->sysfs_buf);
 	if (!sysfs_buf || (sysfs_buf->size != drvdata->size)) {
 		raw_spin_unlock_irqrestore(&drvdata->spinlock, flags);

---
base-commit: 98ac9cc4b4452ed7e714eddc8c90ac4ae5da1a09
change-id: 20251020-fix_etr_issue-02c706dbc899

Best regards,
-- 
Xiaoqi Zhuang <xiaoqi.zhuang@oss.qualcomm.com>

Re: [PATCH v3] coresight: ETR: Fix ETR buffer use-after-free issue

[Xiaoqi Zhuang]

On 10/21/2025 16:45, Xiaoqi Zhuang wrote:
> When ETR is enabled as CS_MODE_SYSFS, if the buffer size is changed
> and enabled again, currently sysfs_buf will point to the newly
> allocated memory(buf_new) and free the old memory(buf_old). But the
> etr_buf that is being used by the ETR remains pointed to buf_old, not
> updated to buf_new. In this case, it will result in a memory
> use-after-free issue.
> 
> Fix this by checking ETR's mode before updating and releasing buf_old,
> if the mode is CS_MODE_SYSFS, then skip updating and releasing it.
> 
> Fixes: bd2767ec3df2 ("coresight: Fix run time warnings while reusing ETR buffer")
> Signed-off-by: Xiaoqi Zhuang <xiaoqi.zhuang@oss.qualcomm.com>
> ---
> Changes in v3:
> - Add a fix tag for the fix patch.
> - Link to v2: https://lore.kernel.org/r/20251021-fix_etr_issue-v2-1-80c40c9cac8c@oss.qualcomm.com
> 
> Changes in v2:
> - Exit earlier to avoid allocating memory unnecessarily.
> - Link to v1: https://lore.kernel.org/r/20251020-fix_etr_issue-v1-1-902ab51770b4@oss.qualcomm.com
> ---
>   drivers/hwtracing/coresight/coresight-tmc-etr.c | 7 +++++++
>   1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c
> index b07fcdb3fe1a..800be06598c1 100644
> --- a/drivers/hwtracing/coresight/coresight-tmc-etr.c
> +++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c
> @@ -1250,6 +1250,13 @@ static struct etr_buf *tmc_etr_get_sysfs_buffer(struct coresight_device *csdev)
>   	 * with the lock released.
>   	 */
>   	raw_spin_lock_irqsave(&drvdata->spinlock, flags);
> +
> +	/*
> +	 * If the ETR is already enabled, continue with the existing buffer.
> +	 */
> +	if (coresight_get_mode(csdev) == CS_MODE_SYSFS)
> +		goto out;
> +
>   	sysfs_buf = READ_ONCE(drvdata->sysfs_buf);
>   	if (!sysfs_buf || (sysfs_buf->size != drvdata->size)) {
>   		raw_spin_unlock_irqrestore(&drvdata->spinlock, flags);
> 
> ---
> base-commit: 98ac9cc4b4452ed7e714eddc8c90ac4ae5da1a09
> change-id: 20251020-fix_etr_issue-02c706dbc899
> 
> Best regards,


Gentle reminder.

-- 
Thanks and Regards,
Xiaoqi

Re: [PATCH v3] coresight: ETR: Fix ETR buffer use-after-free issue

[Leo Yan]

On Tue, Oct 21, 2025 at 04:45:25PM +0800, Xiaoqi Zhuang wrote:
> When ETR is enabled as CS_MODE_SYSFS, if the buffer size is changed
> and enabled again, currently sysfs_buf will point to the newly
> allocated memory(buf_new) and free the old memory(buf_old). But the
> etr_buf that is being used by the ETR remains pointed to buf_old, not
> updated to buf_new. In this case, it will result in a memory
> use-after-free issue.
> 
> Fix this by checking ETR's mode before updating and releasing buf_old,
> if the mode is CS_MODE_SYSFS, then skip updating and releasing it.
> 
> Fixes: bd2767ec3df2 ("coresight: Fix run time warnings while reusing ETR buffer")
> Signed-off-by: Xiaoqi Zhuang <xiaoqi.zhuang@oss.qualcomm.com>

Tested on my Juno board with below steps:

  1) Enable the first path: ETM2 -> ETR0
  echo 1 > /sys/bus/coresight/devices/tmc_etr0/enable_sink
  echo 1 > /sys/bus/coresight/devices/etm2/enable_source

  2) Enlarge buffer size from 1MiB to 4MiB
  cat /sys/bus/coresight/devices/tmc_etr0/buffer_size
  0x100000
  echo 0x400000 > /sys/bus/coresight/devices/tmc_etr0/buffer_size

  3) Enable the second path: ETM0 -> ETR0
  echo 1 > /sys/bus/coresight/devices/etm0/enable_source

  4) Disable paths
  echo 0 > /sys/bus/coresight/devices/etm0/enable_source
  echo 0 > /sys/bus/coresight/devices/etm2/enable_source

Without this patch, the oops will be triggered when disable paths.
I can confirm this patch does dismiss the issue.

Tested-by: Leo Yan <leo.yan@arm.com>

> ---
> Changes in v3:
> - Add a fix tag for the fix patch.
> - Link to v2: https://lore.kernel.org/r/20251021-fix_etr_issue-v2-1-80c40c9cac8c@oss.qualcomm.com
> 
> Changes in v2:
> - Exit earlier to avoid allocating memory unnecessarily.
> - Link to v1: https://lore.kernel.org/r/20251020-fix_etr_issue-v1-1-902ab51770b4@oss.qualcomm.com
> ---
>  drivers/hwtracing/coresight/coresight-tmc-etr.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c
> index b07fcdb3fe1a..800be06598c1 100644
> --- a/drivers/hwtracing/coresight/coresight-tmc-etr.c
> +++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c
> @@ -1250,6 +1250,13 @@ static struct etr_buf *tmc_etr_get_sysfs_buffer(struct coresight_device *csdev)
>  	 * with the lock released.
>  	 */
>  	raw_spin_lock_irqsave(&drvdata->spinlock, flags);
> +
> +	/*
> +	 * If the ETR is already enabled, continue with the existing buffer.
> +	 */
> +	if (coresight_get_mode(csdev) == CS_MODE_SYSFS)
> +		goto out;
> +
>  	sysfs_buf = READ_ONCE(drvdata->sysfs_buf);
>  	if (!sysfs_buf || (sysfs_buf->size != drvdata->size)) {
>  		raw_spin_unlock_irqrestore(&drvdata->spinlock, flags);
> 
> ---
> base-commit: 98ac9cc4b4452ed7e714eddc8c90ac4ae5da1a09
> change-id: 20251020-fix_etr_issue-02c706dbc899
> 
> Best regards,
> -- 
> Xiaoqi Zhuang <xiaoqi.zhuang@oss.qualcomm.com>
> 
> 

Re: [PATCH v3] coresight: ETR: Fix ETR buffer use-after-free issue

[Suzuki K Poulose]

On Tue, 21 Oct 2025 16:45:25 +0800, Xiaoqi Zhuang wrote:
> When ETR is enabled as CS_MODE_SYSFS, if the buffer size is changed
> and enabled again, currently sysfs_buf will point to the newly
> allocated memory(buf_new) and free the old memory(buf_old). But the
> etr_buf that is being used by the ETR remains pointed to buf_old, not
> updated to buf_new. In this case, it will result in a memory
> use-after-free issue.
> 
> [...]

Applied, thanks!

[1/1] coresight: ETR: Fix ETR buffer use-after-free issue
      https://git.kernel.org/coresight/c/35501ac3c7d4

Best regards,
-- 
Suzuki K Poulose <suzuki.poulose@arm.com>

Re: [PATCH v3] coresight: ETR: Fix ETR buffer use-after-free issue

[Mike Leach]

Hi,

Is this fixing the correct problem? If we prevent the buffer size from
being changed while the sink is active - which is probably what we
should do anyway as no real good can come from allowing this - then
the problem disappears.

Changing the buffer size while the sink is active should return -EBUSY;

Mike

On Wed, 5 Nov 2025 at 16:13, Suzuki K Poulose <suzuki.poulose@arm.com> wrote:
>
>
> On Tue, 21 Oct 2025 16:45:25 +0800, Xiaoqi Zhuang wrote:
> > When ETR is enabled as CS_MODE_SYSFS, if the buffer size is changed
> > and enabled again, currently sysfs_buf will point to the newly
> > allocated memory(buf_new) and free the old memory(buf_old). But the
> > etr_buf that is being used by the ETR remains pointed to buf_old, not
> > updated to buf_new. In this case, it will result in a memory
> > use-after-free issue.
> >
> > [...]
>
> Applied, thanks!
>
> [1/1] coresight: ETR: Fix ETR buffer use-after-free issue
>       https://git.kernel.org/coresight/c/35501ac3c7d4
>
> Best regards,
> --
> Suzuki K Poulose <suzuki.poulose@arm.com>



-- 
Mike Leach
Principal Engineer, ARM Ltd.
Manchester Design Centre. UK

Re: [PATCH v3] coresight: ETR: Fix ETR buffer use-after-free issue

[Suzuki K Poulose]

Hi Mike

On 06/11/2025 14:14, Mike Leach wrote:
> Hi,
> 
> Is this fixing the correct problem? If we prevent the buffer size from
> being changed while the sink is active - which is probably what we
> should do anyway as no real good can come from allowing this - then
> the problem disappears.

Good point. But this is completely fine for a running "sysfs" session,
as the values are not updated (unlike perf, where the session is
scheduled out and put back in ). So, I don't see why we can't change
the values while the sink is active ?


> 
> Changing the buffer size while the sink is active should return -EBUSY;
> 
> Mike
> 
> On Wed, 5 Nov 2025 at 16:13, Suzuki K Poulose <suzuki.poulose@arm.com> wrote:
>>
>>
>> On Tue, 21 Oct 2025 16:45:25 +0800, Xiaoqi Zhuang wrote:
>>> When ETR is enabled as CS_MODE_SYSFS, if the buffer size is changed
>>> and enabled again, currently sysfs_buf will point to the newly
>>> allocated memory(buf_new) and free the old memory(buf_old). But the
>>> etr_buf that is being used by the ETR remains pointed to buf_old, not
>>> updated to buf_new. In this case, it will result in a memory
>>> use-after-free issue.
>>>
>>> [...]
>>
>> Applied, thanks!
>>
>> [1/1] coresight: ETR: Fix ETR buffer use-after-free issue
>>        https://git.kernel.org/coresight/c/35501ac3c7d4
>>
>> Best regards,
>> --
>> Suzuki K Poulose <suzuki.poulose@arm.com>
> 
> 
> 

Re: [PATCH v3] coresight: ETR: Fix ETR buffer use-after-free issue

[Mike Leach]

Hi Suzuki,

On Fri, 7 Nov 2025 at 13:28, Suzuki K Poulose <suzuki.poulose@arm.com> wrote:
>
> Hi Mike
>
> On 06/11/2025 14:14, Mike Leach wrote:
> > Hi,
> >
> > Is this fixing the correct problem? If we prevent the buffer size from
> > being changed while the sink is active - which is probably what we
> > should do anyway as no real good can come from allowing this - then
> > the problem disappears.
>
> Good point. But this is completely fine for a running "sysfs" session,
> as the values are not updated (unlike perf, where the session is
> scheduled out and put back in ). So, I don't see why we can't change
> the values while the sink is active ?
>

Why would you want to? There is no reason i can think of, that a user
would need to alter parameters while the sink is running.

If this is a sysfs session and a user changes buffer sizes between
enable commands as per Leo's example sequence - the result is a silent
failure and confusion for the user as the captured buffer size is not
in fact what he just set.

Moreover, the sysfs files (not just the buffer size) write directly
into the internal drvdata structure, some of which are then used to
program the TMC registers  on enable - which is common code between
both sysfs and perf. Thus for perf you have a lovely race condition
and for sysfs you again end up with values that do not apply to the
session just run.

Seems more robust - not just for the sysfs buffer size. - to only
permit changes when halted.

I have sent a follow up patch which should make things clearer.

Regards

Mike


I've sent a follow up patch to address this - and the potential race
condition.
>
> >
> > Changing the buffer size while the sink is active should return -EBUSY;
> >
> > Mike
> >
> > On Wed, 5 Nov 2025 at 16:13, Suzuki K Poulose <suzuki.poulose@arm.com> wrote:
> >>
> >>
> >> On Tue, 21 Oct 2025 16:45:25 +0800, Xiaoqi Zhuang wrote:
> >>> When ETR is enabled as CS_MODE_SYSFS, if the buffer size is changed
> >>> and enabled again, currently sysfs_buf will point to the newly
> >>> allocated memory(buf_new) and free the old memory(buf_old). But the
> >>> etr_buf that is being used by the ETR remains pointed to buf_old, not
> >>> updated to buf_new. In this case, it will result in a memory
> >>> use-after-free issue.
> >>>
> >>> [...]
> >>
> >> Applied, thanks!
> >>
> >> [1/1] coresight: ETR: Fix ETR buffer use-after-free issue
> >>        https://git.kernel.org/coresight/c/35501ac3c7d4
> >>
> >> Best regards,
> >> --
> >> Suzuki K Poulose <suzuki.poulose@arm.com>
> >
> >
> >
>


-- 
Mike Leach
Principal Engineer, ARM Ltd.
Manchester Design Centre. UK