linux-kernel - Re: [PATCH v2 17/23] bpf: selftests: introduce read_cgroup

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 for Android: free password hash cracker in your pocket

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

Message-ID: <87ms5b3s45.fsf@linux.dev>
Date: Tue, 28 Oct 2025 09:31:38 -0700
From: Roman Gushchin <roman.gushchin@...ux.dev>
To: bot+bpf-ci@...nel.org
Cc: akpm@...ux-foundation.org,  linux-kernel@...r.kernel.org,
 ast@...nel.org, surenb@...gle.com, mhocko@...nel.org,
 shakeel.butt@...ux.dev, hannes@...xchg.org, andrii@...nel.org,
 inwardvessel@...il.com, linux-mm@...ck.org, cgroups@...r.kernel.org,
 bpf@...r.kernel.org, martin.lau@...nel.org, song@...nel.org,
 memxor@...il.com, tj@...nel.org, daniel@...earbox.net, eddyz87@...il.com,
 yonghong.song@...ux.dev, clm@...a.com, ihor.solodrai@...ux.dev
Subject: Re: [PATCH v2 17/23] bpf: selftests: introduce read_cgroup_file()
 helper

bot+bpf-ci@...nel.org writes:

> ```
> commit 712137918bba1b0a9c7c670bab9200b0f0d6a7e8
> Author: Roman Gushchin <roman.gushchin@...ux.dev>
>
> bpf: selftests: introduce read_cgroup_file() helper
>
> This patch adds a new helper function read_cgroup_file() to read from
> cgroup control files in BPF selftests. The function opens a file,
> reads into a buffer, and returns the result.
>
>> diff --git a/tools/testing/selftests/bpf/cgroup_helpers.c b/tools/testing/selftests/bpf/cgroup_helpers.c
>> index 20cede4db..8fb02fe4c 100644
>> --- a/tools/testing/selftests/bpf/cgroup_helpers.c
>> +++ b/tools/testing/selftests/bpf/cgroup_helpers.c
>> @@ -126,6 +126,45 @@ int enable_controllers(const char *relative_path, const char *controllers)
>>  	return __enable_controllers(cgroup_path, controllers);
>>  }
>>
>> +static size_t __read_cgroup_file(const char *cgroup_path, const char *file,
>> +				 char *buf, size_t size)
>> +{
>> +	char file_path[PATH_MAX + 1];
>> +	size_t ret;
>> +	int fd;
>> +
>> +	snprintf(file_path, sizeof(file_path), "%s/%s", cgroup_path, file);
>> +	fd = open(file_path, O_RDONLY);
>> +	if (fd < 0) {
>> +		log_err("Opening %s", file_path);
>> +		return -1;
>
> Can __read_cgroup_file() return -1 when its return type is size_t?
>
> The function signature declares size_t (unsigned), but returns -1 here.
> When -1 is implicitly converted to size_t, it becomes SIZE_MAX
> (18446744073709551615 on 64-bit systems). Callers checking for errors
> with "if (ret < 0)" will fail because size_t is unsigned, and callers
> checking "if (ret == -1)" will also fail after the cast.
>
>> +	}
>> +
>> +	ret = read(fd, buf, size);
>> +	close(fd);
>> +	return ret;
>
> Does this handle read() errors correctly?
>
> The read() syscall returns ssize_t which can be negative on error.
> Assigning this to size_t (line 143) converts negative error codes into
> large positive numbers. For example, read() returning -1 becomes SIZE_MAX
> when stored in ret.
>
> Should the return type be ssize_t instead of size_t to properly propagate
> both success (byte count) and error (negative values) to callers?

Correct, fixed to ssize_t. Thanks!