Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
To: Leo Yan <leo.yan@arm.com>
Cc: Mohamed Ayman <mohamedaymanworkspace@gmail.com>,
	Suzuki K Poulose <suzuki.poulose@arm.com>,
	Mike Leach <mike.leach@arm.com>,
	James Clark <james.clark@linaro.org>,
	Alexander Shishkin <alexander.shishkin@linux.intel.com>,
	Clark Williams <clrkwllms@kernel.org>,
	Steven Rostedt <rostedt@goodmis.org>,
	"moderated list:ARM/CORESIGHT FRAMEWORK AND DRIVERS"
	<coresight@lists.linaro.org>,
	"moderated list:ARM/CORESIGHT FRAMEWORK AND DRIVERS"
	<linux-arm-kernel@lists.infradead.org>,
	open list <linux-kernel@vger.kernel.org>,
	"open list:Real-time Linux (PREEMPT_RT):Keyword:PREEMPT_RT"
	<linux-rt-devel@lists.linux.dev>
Subject: Re: [PATCH v3] coresight: Fix scheduling while atomic in coresight_cpu_pm_notify()
Date: Fri, 17 Jul 2026 18:12:01 +0200	[thread overview]
Message-ID: <20260717161201.3jkO5o71@linutronix.de> (raw)
In-Reply-To: <20260717155848.GB3998092@e132581.arm.com>

On 2026-07-17 16:58:48 [+0100], Leo Yan wrote:
> >  static struct coresight_path *coresight_cpu_get_active_path(enum cs_mode mode)
> >  {
> >  	struct coresight_device *source;
> > -	bool is_active = false;
> > +	struct coresight_path *path = NULL;
> >  
> > -	source = coresight_get_percpu_source_ref(smp_processor_id());
> > -	if (!source)
> > -		return NULL;
> > -
> > -	if (coresight_get_mode(source) & mode)
> > -		is_active = true;
> > +	guard(raw_spinlock_irqsave)(&coresight_dev_lock);
> >  
> > -	coresight_put_percpu_source_ref(source);
> > +	source = per_cpu(csdev_source, smp_processor_id());
> > +	if (source && (coresight_get_mode(source) & mode))
> > +		path = source->path;
> 
> I agree the get_device()/put_device() pair in
> coresight_cpu_get_active_path() is not a good fit for CPU PM notifier,
> because the put_device() can become the final put while IRQ is disabled.
> 
> However, my understanding is this patch might cause UAF issue that the
> existing code is intended to prevent.
> 
> The raw spinlock (coresight_dev_lock) serializes access to the per-CPU
> csdev_source pointer. It does not guarantee the lifetime of the source
> or its _parent_ device. coresight_unregister() is not only reached from
> module unload; it can also be called when a driver is unbind, for
> example DT overlay removal or device hotplug/unplug.

But doesn't coresight_unregister() block on the coresight_dev_lock here?

> This is why the UAF issue Sashiko mentioned in patch 03 of [1]. A built
> CoreSight path currently grabs references for the path components, which
> keeps module alive, but that is not the same as preventing the parent
> device/driver from being unbound and tearing down CoreSight device data
> while an active session still has raw pointers.
> 
> There are also similar race window before the path is built: for
> example etm_setup_aux() has to look up source/sink state before
> coresight_build_path() establishes the path, so it might access
> released source/sink data if device is unbound.
> 
> I think a proper fix needs a clearer lifetime model for an active
> session. E.g., we could consider to call device_link_add() to prevent
> device unbind / unregister, and unlink device when the session is
> finished. Once that is in place, the CPU PM notifier can safely use the
> active path without get_device()/put_device() pair anymore.

Right. I am also not sure about lifetime of coresight_device::path.

> Hope this is clear and makes sense.
> 
> Thanks,
> Leo

Sebastian


      reply	other threads:[~2026-07-17 16:12 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-12 21:04 [PATCH] coresight: Fix scheduling while atomic in coresight_device_release() Mohamed Ayman
2026-07-13  3:07 ` Jie Gan
2026-07-13 23:00 ` [PATCH v2] coresight: Fix scheduling while atomic in coresight_put_percpu_source_ref() Mohamed Ayman
2026-07-14 10:42   ` Sebastian Andrzej Siewior
2026-07-14 19:42     ` MOHAMED AYMAN
2026-07-15  6:58       ` Sebastian Andrzej Siewior
2026-07-16  3:07         ` MOHAMED AYMAN
2026-07-16 12:32           ` Sebastian Andrzej Siewior
2026-07-16 21:41 ` [PATCH v3] coresight: Fix scheduling while atomic in coresight_cpu_pm_notify() Mohamed Ayman
2026-07-17  8:14   ` Sebastian Andrzej Siewior
2026-07-17 10:49     ` Leo Yan
2026-07-17 15:58   ` Leo Yan
2026-07-17 16:12     ` Sebastian Andrzej Siewior [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260717161201.3jkO5o71@linutronix.de \
    --to=bigeasy@linutronix.de \
    --cc=alexander.shishkin@linux.intel.com \
    --cc=clrkwllms@kernel.org \
    --cc=coresight@lists.linaro.org \
    --cc=james.clark@linaro.org \
    --cc=leo.yan@arm.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-rt-devel@lists.linux.dev \
    --cc=mike.leach@arm.com \
    --cc=mohamedaymanworkspace@gmail.com \
    --cc=rostedt@goodmis.org \
    --cc=suzuki.poulose@arm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox