linux-kernel - Re: [PATCH v3] sched/fair: Use sched_domain_span() for topology_span

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <xhsmhldoyq547.mognet@vschneid-thinkpadt14sgen2i.remote.csb>
Date: Tue, 08 Jul 2025 13:44:24 +0200
From: Valentin Schneider <vschneid@...hat.com>
To: K Prateek Nayak <kprateek.nayak@....com>, Ingo Molnar
 <mingo@...hat.com>, Peter Zijlstra <peterz@...radead.org>, Juri Lelli
 <juri.lelli@...hat.com>, Vincent Guittot <vincent.guittot@...aro.org>,
 Leon
 Romanovsky <leon@...nel.org>, linux-kernel@...r.kernel.org
Cc: Steve Wahl <steve.wahl@....com>, Borislav Petkov <bp@...en8.de>, Dietmar
 Eggemann <dietmar.eggemann@....com>, Steven Rostedt <rostedt@...dmis.org>,
 Ben Segall <bsegall@...gle.com>, Mel Gorman <mgorman@...e.de>, K Prateek
 Nayak <kprateek.nayak@....com>
Subject: Re: [PATCH v3] sched/fair: Use sched_domain_span() for
 topology_span_sane()

On 07/07/25 10:53, K Prateek Nayak wrote:
> Changelog v2..v3:
>
> o Added a check to skip uninitialized sd that can cause dereference of
>   sdd->sd beyond the percpu boundary (reported by Boris).
>
>   Tested on the trivial case using the QEMU cmdline:
>
>     sudo qemu-system-x86_64 -enable-kvm -cpu host -m 20G \
>     -smp cpus=10,socket=1,thread=10 -machine q35 \
>     -object memory-backend-ram,size=20G,id=m0 \
>     -numa node,cpus=0-9,memdev=m0,nodeid=0 \
>     ...
>

Urgh, of course directly using @sdd is not like walking up the sd hierarchy
where we end up getting a NULL sentinel... Sorry for suggesting that and
not thinking about that "small" detail, and thanks for being on top of it.

> ---
>  kernel/sched/topology.c | 25 +++++++++++++++++++------
>  1 file changed, 19 insertions(+), 6 deletions(-)
>
> diff --git a/kernel/sched/topology.c b/kernel/sched/topology.c
> index b958fe48e020..e682bf991ce6 100644
> --- a/kernel/sched/topology.c
> +++ b/kernel/sched/topology.c
> @@ -2403,6 +2403,7 @@ static bool topology_span_sane(const struct cpumask *cpu_map)
>       id_seen = sched_domains_tmpmask2;
>
>       for_each_sd_topology(tl) {
> +		struct sd_data *sdd = &tl->data;
>
>               /* NUMA levels are allowed to overlap */
>               if (tl->flags & SDTL_OVERLAP)
> @@ -2418,22 +2419,34 @@ static bool topology_span_sane(const struct cpumask *cpu_map)
>                * breaks the linking done for an earlier span.
>                */
>               for_each_cpu(cpu, cpu_map) {
> -			const struct cpumask *tl_cpu_mask = tl->mask(cpu);
> +			struct sched_domain *sd = *per_cpu_ptr(sdd->sd, cpu);
> +			struct cpumask *sd_span = sched_domain_span(sd);
>                       int id;
>
> +			/*
> +			 * If a child level for a CPU has already covered
> +			 * the entire cpumap, build_sched_domain() for the
> +			 * domains above is skipped. Use sd->private to detect
> +			 * levels that have not been initialized in the CPU's
> +			 * hierarchy and skip them.
> +			 */
> +			if (!sd->private)
> +				continue;
> +

So this works, but how about using a cpumask_empty(sd_span) check instead?
It's IMO a bit more future proof than relying on how sd->private is used.