All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jonathan Cameron <jic23@kernel.org>
To: Dave Jiang <dave.jiang@intel.com>
Cc: linux-cxl@vger.kernel.org, linux-perf-users@vger.kernel.org,
	will@kernel.org, mark.rutland@arm.com, dave@stgolabs.net,
	sashiko-bot@kernel.org, Robin Murphy <robin.murphy@arm.com>
Subject: Re: [PATCH v2 9/9] perf/cxl: Avoid cpumask_of(-1) when no CPU is assigned
Date: Wed, 29 Jul 2026 20:14:46 +0100	[thread overview]
Message-ID: <20260729201446.62732044@jic23-huawei> (raw)
In-Reply-To: <20260729145555.3919550-10-dave.jiang@intel.com>

On Wed, 29 Jul 2026 07:55:55 -0700
Dave Jiang <dave.jiang@intel.com> wrote:

> cpumask_show() feeds info->on_cpu straight into cpumask_of() for the
> world-readable cpumask sysfs attribute. on_cpu is -1 before the first
> hotplug online callback and transiently in cxl_pmu_offline_cpu() before a
> new target is chosen. cpumask_of(-1) treats the CPU number as unsigned and
> does out-of-bounds pointer arithmetic in get_cpu_mask(), so a concurrent
> read of the attribute dereferences a wild pointer and can fault -- a local
> denial of service.
> 

I'm not keen on the solution here. 

The transient state is ugly anyway. We can just move setting it to -1 into
the dummy code that deals with that well known case of you have CPUs online
and code is still running.

The init case looks like a false positive to me.  But maybe I'm missing stuff.
The perf registration that surfaces the sysfs happens after hotplug handler is
added and I believe that synchronously runs it for CPUs that are already up.
Given we are running code (and CXL stuff isn't super early) something will
be up so it won't remain -1 by the time of use.

Can we just use the generic stuff?  Maybe need Robin's stuff to add init / exit
per driver calls. 
https://lore.kernel.org/linux-arm-kernel/cover.1784911757.git.robin.murphy@arm.com/

In general, I'd like Robin to take a quick look at the more generic perf
parts of this series given he has clearly been deep in this stuff a lot
more recently than me :)

Jonathan

> Read on_cpu once and emit an empty mask when it is negative.
> 
> Fixes: 5d7107c72796 ("perf: CXL Performance Monitoring Unit driver")
> Reported-by: sashiko-bot@kernel.org
> Closes: https://sashiko.dev/#/patchset/20260715191454.459673-1-dave@stgolabs.net?part=1
> Assisted-by: Claude:claude-opus-4-8
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> ---
>  drivers/perf/cxl_pmu.c | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/perf/cxl_pmu.c b/drivers/perf/cxl_pmu.c
> index f42238b2b6b0..6aad381c0376 100644
> --- a/drivers/perf/cxl_pmu.c
> +++ b/drivers/perf/cxl_pmu.c
> @@ -501,8 +501,17 @@ static ssize_t cpumask_show(struct device *dev, struct device_attribute *attr,
>  			    char *buf)
>  {
>  	struct cxl_pmu_info *info = dev_get_drvdata(dev);
> +	int cpu = READ_ONCE(info->on_cpu);
>  
> -	return cpumap_print_to_pagebuf(true, buf, cpumask_of(info->on_cpu));
> +	/*
> +	 * on_cpu is -1 before the first online callback and transiently during
> +	 * cxl_pmu_offline_cpu(). cpumask_of(-1) computes an out-of-bounds
> +	 * pointer, so report an empty mask instead.
> +	 */
> +	if (cpu < 0)
> +		return sysfs_emit(buf, "\n");
> +
> +	return cpumap_print_to_pagebuf(true, buf, cpumask_of(cpu));
>  }
>  static DEVICE_ATTR_RO(cpumask);
>  


      parent reply	other threads:[~2026-07-29 19:14 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-29 14:55 [PATCH v2 0/9] perf/cxlpmu: Misc sashiko raised issues fixes Dave Jiang
2026-07-29 14:55 ` [PATCH v2 1/9] perf/cxl: Program the requested event group on configurable counters Dave Jiang
2026-07-29 15:06   ` sashiko-bot
2026-07-29 14:55 ` [PATCH v2 2/9] perf/cxl: Clear stale event fields before reprogramming a counter Dave Jiang
2026-07-29 15:08   ` sashiko-bot
2026-07-29 14:55 ` [PATCH v2 3/9] perf/cxl: Fix the counter overflow delta fixup Dave Jiang
2026-07-29 15:13   ` sashiko-bot
2026-07-29 14:55 ` [PATCH v2 4/9] perf/cxl: Accept an overflow interrupt on MSI message number 0 Dave Jiang
2026-07-29 19:28   ` Jonathan Cameron
2026-07-29 19:59   ` Davidlohr Bueso
2026-07-29 14:55 ` [PATCH v2 5/9] perf/cxl: Keep the overflow interrupt pinned to the managed CPU Dave Jiang
2026-07-29 15:23   ` sashiko-bot
2026-07-29 19:25   ` Jonathan Cameron
2026-07-29 20:27   ` Davidlohr Bueso
2026-07-29 14:55 ` [PATCH v2 6/9] perf/cxl: Unfreeze counters after handling an overflow interrupt Dave Jiang
2026-07-29 19:24   ` Jonathan Cameron
2026-07-29 14:55 ` [PATCH v2 7/9] perf/cxl: Validate the hardware-reported counter width Dave Jiang
2026-07-29 15:11   ` sashiko-bot
2026-07-29 19:21   ` Jonathan Cameron
2026-07-29 14:55 ` [PATCH v2 8/9] perf/cxl: Don't use pmu.dev in IRQ and hotplug callbacks after unregister Dave Jiang
2026-07-29 15:34   ` sashiko-bot
2026-07-29 19:17   ` Jonathan Cameron
2026-07-29 14:55 ` [PATCH v2 9/9] perf/cxl: Avoid cpumask_of(-1) when no CPU is assigned Dave Jiang
2026-07-29 15:19   ` sashiko-bot
2026-07-29 19:14   ` Jonathan Cameron [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=20260729201446.62732044@jic23-huawei \
    --to=jic23@kernel.org \
    --cc=dave.jiang@intel.com \
    --cc=dave@stgolabs.net \
    --cc=linux-cxl@vger.kernel.org \
    --cc=linux-perf-users@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=robin.murphy@arm.com \
    --cc=sashiko-bot@kernel.org \
    --cc=will@kernel.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.