Re: [PATCH v3 1/3] selftests/mm/write_to_hugetlbfs: parse -s as size_t

[Li Wang <liwang@redhat.com>]

David Laight <david.laight.linux@gmail.com> wrote:

> > > Maybe you could use "%zu%c" and check the count is 1 - but I bet
> > > some static checker won't like that.
> > >
> >
> > Yes, that would be stronger, since it would reject trailing garbage.
> > But for a selftest this is probably sufficient: switching to size_t and
> > parsing with "%zu" already avoids the int truncation issue.
>
> Have you checked at what does sscanf() does with an overlong digit string?
> I'd guess that it just processes all the digits and then masks the result
> to fix (like the kernel one does).

It will truncate the number to the size SIZE_MAX.

From my test:

# ./write_to_hugetlbfs -m 0 -s 99999999999999999999999999 -p /mnt/huge/test -o
Writing to this path: /mnt/huge/test
Writing this size: 18446744073709551615    <---------- SIZE_MAX
Populating.
Not writing to memory.
Using method=0
Shared mapping.
RESERVE mapping.
Allocating using HUGETLBFS.
write_to_hugetlbfs: Error mapping the file: Invalid argument


> It reality scanf() is 'not the function you are lookign for'.
>
> IIRC the 'SUS' (used to) say that this was absolutely fine for command
> line parsing for 'standard utilities'.
>
> It is best to use strtoul() and check the 'end' character is '\0'.

Hmm, that sounds like we need to go back to the patch V1 [1] method.
But I am not sure, @Andrew Morton, do you think so?

--- a/tools/testing/selftests/mm/write_to_hugetlbfs.c
+++ b/tools/testing/selftests/mm/write_to_hugetlbfs.c
@@ -86,10 +86,17 @@ int main(int argc, char **argv)
        while ((c = getopt(argc, argv, "s:p:m:owlrn")) != -1) {
                switch (c) {
                case 's':
-                       if (sscanf(optarg, "%zu", &size) != 1) {
-                               perror("Invalid -s.");
+                       char *end = NULL;
+                       unsigned long tmp = strtoul(optarg, &end, 10);
+                       if (errno || end == optarg || *end != '\0') {
+                               perror("Invalid -s size");
                                exit_usage();
                        }
+                       if (tmp == 0) {
+                               perror("size not found");
+                               exit_usage();
+                       }
+                       size = (size_t)tmp;
                        break;
                case 'p':


[1] https://lore.kernel.org/linux-kselftest/20251220111645.2246009-3-liwang@redhat.com/T/#m5a5765349dedfda1ed66c54b8cab7af184bff371



--
Regards,
Li Wang