Re: [PATCH 2/2] selftests/sched_ext: Validate dl_server attach/detach in total_bw test

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The dl_server runtime and period parameters are parsed and written as signed `long` instead of `u64` or `unsigned long long`, leading to potential integer overflow on 32-bit platforms.
--

commit 6475744500400f1c70cab62bc90b17ee9b9cbbea
Author: Andrea Righi <arighi@nvidia.com>

selftests/sched_ext: Validate dl_server attach/detach in total_bw test

Extend the total_bw selftest to validate the fair/ext dl_server
auto-attach/detach operations by doubling the fair_server's runtime
via debugfs and verifying total_bw changes.

> diff --git a/tools/testing/selftests/sched_ext/total_bw.c b/tools/testing/selftests/sched_ext/total_bw.c
> index 5b0a619bab86e..2af01cee90cc0 100644
> --- a/tools/testing/selftests/sched_ext/total_bw.c
> +++ b/tools/testing/selftests/sched_ext/total_bw.c
[ ... ]
> @@ -100,6 +100,98 @@ static int read_total_bw_values(long *bw_values, int max_cpus)
>  	return cpu_count;
>  }
>  
> +/*
> + * Read a per-CPU dl_server param (runtime or period) from debugfs.
> + * Returns the value in nanoseconds, or -1 on failure.
> + */
> +static long read_server_param(const char *server, const char *param, int cpu)
> +{
> +	char path[128];
> +	long value = -1;

[Severity: Medium]
Will parsing the dl_server runtime and period parameters as signed long
lead to integer overflows on 32-bit platforms?

In the kernel, dl_server parameters like runtime and period are represented
as u64 values (in nanoseconds). On 32-bit platforms, a signed long is 32
bits, which can only represent up to ~2.14 seconds in nanoseconds.

If a test system configures a fair_server runtime larger than ~1.07 seconds,
doubling it could cause a signed integer overflow. Would it be better to use
unsigned long long or u64 with %llu to avoid this?

> +	FILE *fp;
> +
> +	snprintf(path, sizeof(path),
> +		 "/sys/kernel/debug/sched/%s_server/cpu%d/%s",
> +		 server, cpu, param);
> +	fp = fopen(path, "r");
> +	if (!fp)
> +		return -1;
> +	if (fscanf(fp, "%ld", &value) != 1)
> +		value = -1;
> +	fclose(fp);
> +
> +	return value;
> +}
> +
> +/*
> + * Write a per-CPU dl_server param to debugfs. Returns 0 on success.
> + */
> +static int write_server_param(const char *server, const char *param,
> +			      int cpu, long value)

[Severity: Medium]
Similarly here, does taking long value as a parameter restrict the maximum
runtime that can be written on 32-bit systems?

> +{
> +	char path[128];
> +	FILE *fp;
> +	int ret = 0;
> +
> +	snprintf(path, sizeof(path),
> +		 "/sys/kernel/debug/sched/%s_server/cpu%d/%s",
> +		 server, cpu, param);
> +	fp = fopen(path, "w");
> +	if (!fp)
> +		return -1;
> +	if (fprintf(fp, "%ld", value) < 0)
> +		ret = -1;
> +	if (fclose(fp) != 0)
> +		ret = -1;
> +
> +	return ret;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260526164420.638711-1-arighi@nvidia.com?part=2