[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CAJ9a7VgXEQQmj6o7n-mPx0qtrCQXKmddGS6BugHb=xaxVAUTUQ@mail.gmail.com>
Date: Fri, 7 Nov 2025 16:24:19 +0000
From: Mike Leach <mike.leach@...aro.org>
To: Suzuki K Poulose <suzuki.poulose@....com>
Cc: James Clark <james.clark@...aro.org>,
Alexander Shishkin <alexander.shishkin@...ux.intel.com>, Linu Cherian <lcherian@...vell.com>,
Xiaoqi Zhuang <xiaoqi.zhuang@....qualcomm.com>, coresight@...ts.linaro.org,
linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
linux-arm-msm@...r.kernel.org
Subject: Re: [PATCH v3] coresight: ETR: Fix ETR buffer use-after-free issue
Hi Suzuki,
On Fri, 7 Nov 2025 at 13:28, Suzuki K Poulose <suzuki.poulose@....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@....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@....com>
> >
> >
> >
>
--
Mike Leach
Principal Engineer, ARM Ltd.
Manchester Design Centre. UK
Powered by blists - more mailing lists