On Sat, 20 Dec 2025 19:16:43 +0800 Li Wang <[email protected]> wrote:
> write_to_hugetlbfs currently parses the -s size argument with atoi()
> into an int. This silently accepts malformed input, cannot report overflow,
> and can truncate large sizes.
>
> --- Error log ---
> # uname -r
> 6.12.0-xxx.el10.aarch64+64k
>
> # ls /sys/kernel/mm/hugepages/hugepages-*
> hugepages-16777216kB/ hugepages-2048kB/ hugepages-524288kB/
>
> #./charge_reserved_hugetlb.sh -cgroup-v2
> # -----------------------------------------
> ...
> # nr hugepages = 10
> # writing cgroup limit: 5368709120
> # writing reseravation limit: 5368709120
Can we fix that typo while we're in there? "reservation".
> ...
> # Writing to this path: /mnt/huge/test
> # Writing this size: -1610612736 <--------
>
> Switch size to size_t and parse -s using strtoull() with proper validation.
> Also print the size using %zu.
>
> This makes the test utility more robust and avoids undefined/incorrect
> behavior with large or invalid -s values.
>
> ...
>
> --- a/tools/testing/selftests/mm/write_to_hugetlbfs.c
> +++ b/tools/testing/selftests/mm/write_to_hugetlbfs.c
> @@ -68,7 +68,7 @@ int main(int argc, char **argv)
> int key = 0;
> int *ptr = NULL;
> int c = 0;
> - int size = 0;
> + size_t size = 0;
> char path[256] = "";
> enum method method = MAX_METHOD;
> int want_sleep = 0, private = 0;
> @@ -86,7 +86,20 @@ int main(int argc, char **argv)
> while ((c = getopt(argc, argv, "s:p:m:owlrn")) != -1) {
> switch (c) {
> case 's':
> - size = atoi(optarg);
> + errno = 0;
> + char *end = NULL;
> + unsigned long long tmp = strtoull(optarg, &end, 10);
Coding-style nits: we do accept c99-style definitions nowadays but I do
think our eyes prefer the less surprising "definitions come before
code" style. So the above could be
char *end = NULL;
unsigned long long tmp = strtoull(optarg, &end, 10);
errno = 0;
Also, `errno' belongs to libc. It seems wrong to be altering it from
within our client code.
> + if (errno || end == optarg || *end != '\0') {
> + errno = EINVAL;
> + perror("Invalid -s size");
> + exit_usage();
> + }
> + if (tmp == 0) {
> + errno = EINVAL;
> + perror("size not found");
> + exit_usage();
> + }
> + size = (size_t)tmp;
> break;
I'm not really clear on what problems we're trying to solve here, but
this all seems like a lot of fuss. Can we just do
if (sscanf(optarg, "%zu", &size) != 1)
?
> case 'p':
> strncpy(path, optarg, sizeof(path) - 1);
> @@ -131,7 +144,7 @@ int main(int argc, char **argv)
> }
>
> if (size != 0) {
> - printf("Writing this size: %d\n", size);
> + printf("Writing this size: %zu\n", size);
> } else {
> errno = EINVAL;
> perror("size not found");
