[PATCH bpf-next] selftests/bpf: Fix va_list reuse across loop iterations in scanf_one_line()

[PATCH bpf-next] selftests/bpf: Fix va_list reuse across loop iterations in scanf_one_line()

[Adrián García Casado]

scanf_one_line() initializes 'ap' with va_start() once before the
getline() loop and passes it to vsscanf() on each iteration without
reinitializing it.

According to the C standard, once a va_list is passed to a function that
invokes va_arg() (such as vsscanf()), its value becomes indeterminate and
cannot be reused without an intervening va_end() and va_start().

Specifically, when scanning '/proc/self/mounts' with the format string
'%*s %4095s cgroup2 %s', every non-cgroup2 mount line matches the leading
'%*s %4095s' (returning 1 instead of 2) and consumes one pointer argument
from 'ap', exhausting the two passed pointers after the second line and
causing vsscanf() to write into uninitialized stack values on subsequent
lines.

Move va_start() and va_end() inside the getline() loop so 'ap' is
properly reinitialized for each line.

Signed-off-by: Adrián García Casado <adriangarciacasado42@gmail.com>
---
 tools/testing/selftests/bpf/veristat.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/bpf/veristat.c b/tools/testing/selftests/bpf/veristat.c
index 9622af98dc734..ba4da33043d00 100644
--- a/tools/testing/selftests/bpf/veristat.c
+++ b/tools/testing/selftests/bpf/veristat.c
@@ -1462,9 +1462,10 @@ static int scanf_one_line(const char *file, int fields_expected, const char *fmt
 	if (!f)
 		return -1;
 
-	va_start(ap, fmt);
 	while (getline(&line, &line_len, f) > 0) {
+		va_start(ap, fmt);
 		res = vsscanf(line, fmt, ap);
+		va_end(ap);
 		if (res == fields_expected)
 			goto out;
 	}
@@ -1474,7 +1475,6 @@ static int scanf_one_line(const char *file, int fields_expected, const char *fmt
 	}
 
 out:
-	va_end(ap);
 	free(line);
 	fclose(f);
 	errno = saved_errno;
-- 
2.39.5

Re: [PATCH bpf-next] selftests/bpf: Fix va_list reuse across loop iterations in scanf_one_line()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Low] selftests/bpf: missing Fixes trailer for scanf_one_line() fix
--

commit be7c1bdc105e46aa8eb43384f62da500ab1ddc9a
Author: Adrián García Casado <adriangarciacasado42@gmail.com>

> selftests/bpf: Fix va_list reuse across loop iterations in scanf_one_line()
>
> scanf_one_line() initializes 'ap' with va_start() once before the
> getline() loop and passes it to vsscanf() on each iteration without
> reinitializing it.
>
> According to the C standard, once a va_list is passed to a function that
> invokes va_arg() (such as vsscanf()), its value becomes indeterminate and
> cannot be reused without an intervening va_end() and va_start().
>
> Specifically, when scanning '/proc/self/mounts' with the format string
> '%*s %4095s cgroup2 %s', every non-cgroup2 mount line matches the leading
> '%*s %4095s' (returning 1 instead of 2) and consumes one pointer argument
> from 'ap', exhausting the two passed pointers after the second line and
> causing vsscanf() to write into uninitialized stack values on subsequent
> lines.
>
> Move va_start() and va_end() inside the getline() loop so 'ap' is
> properly reinitialized for each line.
>
> Signed-off-by: Adrián García Casado <adriangarciacasado42@gmail.com>

[Severity: Low]
Should this commit include a Fixes: tag for the commit that introduced
scanf_one_line()?

  Fixes: 67cdcc405b46 ("veristat: Memory accounting for bpf programs")

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261109143846.1626182-1-adriangarciacasado42@gmail.com?part=1