[PATCH 1/2] selftests/bpf: Fix read_iter buffer termination in test_bpffs

[PATCH 1/2] selftests/bpf: Fix read_iter buffer termination in test_bpffs

[Sun Jian]

read_iter() always NUL-terminated at the end of the buffer, so strstr()
could scan uninitialized stack bytes on short reads. Terminate at len and
use O_RDONLY.

Signed-off-by: Sun Jian <sun.jian.kdev@gmail.com>
---
 tools/testing/selftests/bpf/prog_tests/test_bpffs.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/test_bpffs.c b/tools/testing/selftests/bpf/prog_tests/test_bpffs.c
index ea933fd151c3..e8021ff0581c 100644
--- a/tools/testing/selftests/bpf/prog_tests/test_bpffs.c
+++ b/tools/testing/selftests/bpf/prog_tests/test_bpffs.c
@@ -17,11 +17,11 @@ static int read_iter(char *file)
 	char buf[1024];
 	int fd, len;
 
-	fd = open(file, 0);
+	fd = open(file, O_RDONLY);
 	if (fd < 0)
 		return -1;
-	while ((len = read(fd, buf, sizeof(buf))) > 0) {
-		buf[sizeof(buf) - 1] = '\0';
+	while ((len = read(fd, buf, sizeof(buf) - 1)) > 0) {
+		buf[len] = '\0';
 		if (strstr(buf, "iter")) {
 			close(fd);
 			return 0;

base-commit: 1f318b96cc84d7c2ab792fcc0bfd42a7ca890681
-- 
2.43.0

[PATCH 2/2] selftests/bpf: Skip bpffs debug iter checks when unavailable in test_bpffs

[Sun Jian]

maps.debug/progs.debug may be inaccessible or absent (EPERM/EACCES/ENOENT)
in some environments, which used to abort test_bpffs before it exercised
the pin and rename checks. Treat these errors as "iter unavailable", log
an INFO message, and continue with the core bpffs tests.

Signed-off-by: Sun Jian <sun.jian.kdev@gmail.com>
---
 .../selftests/bpf/prog_tests/test_bpffs.c     | 43 ++++++++++++++++---
 1 file changed, 36 insertions(+), 7 deletions(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/test_bpffs.c b/tools/testing/selftests/bpf/prog_tests/test_bpffs.c
index e8021ff0581c..95a7d2582ba7 100644
--- a/tools/testing/selftests/bpf/prog_tests/test_bpffs.c
+++ b/tools/testing/selftests/bpf/prog_tests/test_bpffs.c
@@ -11,15 +11,18 @@
 /* TDIR must be in a location we can create a directory in. */
 #define TDIR "/tmp/test_bpffs_testdir"
 
-static int read_iter(char *file)
+static int read_iter(const char *file, int *save_errno)
 {
 	/* 1024 should be enough to get contiguous 4 "iter" letters at some point */
 	char buf[1024];
 	int fd, len;
 
 	fd = open(file, O_RDONLY);
-	if (fd < 0)
+	if (fd < 0) {
+		if (save_errno)
+			*save_errno = errno;
 		return -1;
+	}
 	while ((len = read(fd, buf, sizeof(buf) - 1)) > 0) {
 		buf[len] = '\0';
 		if (strstr(buf, "iter")) {
@@ -27,10 +30,33 @@ static int read_iter(char *file)
 			return 0;
 		}
 	}
+	if (save_errno)
+		*save_errno = (len < 0) ? errno : 0;
 	close(fd);
 	return -1;
 }
 
+static bool is_iter_skip_err(int err, int serrno)
+{
+	return err && (serrno == EPERM || serrno == EACCES || serrno == ENOENT);
+}
+
+static int read_iter_or_skip(const char *file)
+{
+	int serrno = 0;
+	int err = read_iter(file, &serrno);
+
+	if (is_iter_skip_err(err, serrno)) {
+		fprintf(stderr,
+			"INFO: %s unavailable (%d), skipping iter check\n",
+			file,
+			serrno);
+		return 0;
+	}
+
+	return err;
+}
+
 static int fn(void)
 {
 	struct stat a, b, c;
@@ -69,13 +95,16 @@ static int fn(void)
 	if (!ASSERT_OK(err, "mount bpffs " TDIR "/fs2"))
 		goto out;
 
-	err = read_iter(TDIR "/fs1/maps.debug");
-	if (!ASSERT_OK(err, "reading " TDIR "/fs1/maps.debug"))
+	err = read_iter_or_skip(TDIR "/fs1/maps.debug");
+	if (err) {
+		ASSERT_OK(err, "reading " TDIR "/fs1/maps.debug");
 		goto out;
-	err = read_iter(TDIR "/fs2/progs.debug");
-	if (!ASSERT_OK(err, "reading " TDIR "/fs2/progs.debug"))
+	}
+	err = read_iter_or_skip(TDIR "/fs2/progs.debug");
+	if (err) {
+		ASSERT_OK(err, "reading " TDIR "/fs2/progs.debug");
 		goto out;
-
+	}
 	err = mkdir(TDIR "/fs1/a", 0777);
 	if (!ASSERT_OK(err, "creating " TDIR "/fs1/a"))
 		goto out;
-- 
2.43.0

Re: [PATCH 1/2] selftests/bpf: Fix read_iter buffer termination in test_bpffs

[Viktor Malik]

On 3/10/26 11:21, Sun Jian wrote:
> read_iter() always NUL-terminated at the end of the buffer, so strstr()
> could scan uninitialized stack bytes on short reads. Terminate at len and
> use O_RDONLY.
> 
> Signed-off-by: Sun Jian <sun.jian.kdev@gmail.com>
> ---
>  tools/testing/selftests/bpf/prog_tests/test_bpffs.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/testing/selftests/bpf/prog_tests/test_bpffs.c b/tools/testing/selftests/bpf/prog_tests/test_bpffs.c
> index ea933fd151c3..e8021ff0581c 100644
> --- a/tools/testing/selftests/bpf/prog_tests/test_bpffs.c
> +++ b/tools/testing/selftests/bpf/prog_tests/test_bpffs.c
> @@ -17,11 +17,11 @@ static int read_iter(char *file)
>  	char buf[1024];
>  	int fd, len;
>  
> -	fd = open(file, 0);
> +	fd = open(file, O_RDONLY);
>  	if (fd < 0)
>  		return -1;
> -	while ((len = read(fd, buf, sizeof(buf))) > 0) {
> -		buf[sizeof(buf) - 1] = '\0';
> +	while ((len = read(fd, buf, sizeof(buf) - 1)) > 0) {
> +		buf[len] = '\0';

Does this fix any real issue with the test? I can see one very
hypothetical false negative when the uninitialized memory would contain
"iter" but that seems very unlikely to happen.

Viktor

>  		if (strstr(buf, "iter")) {
>  			close(fd);
>  			return 0;
> 
> base-commit: 1f318b96cc84d7c2ab792fcc0bfd42a7ca890681

Re: [PATCH 2/2] selftests/bpf: Skip bpffs debug iter checks when unavailable in test_bpffs

[Viktor Malik]

On 3/10/26 11:21, Sun Jian wrote:
> maps.debug/progs.debug may be inaccessible or absent (EPERM/EACCES/ENOENT)
> in some environments, which used to abort test_bpffs before it exercised
> the pin and rename checks. Treat these errors as "iter unavailable", log
> an INFO message, and continue with the core bpffs tests.
> 
> Signed-off-by: Sun Jian <sun.jian.kdev@gmail.com>
> ---
>  .../selftests/bpf/prog_tests/test_bpffs.c     | 43 ++++++++++++++++---
>  1 file changed, 36 insertions(+), 7 deletions(-)
> 
> diff --git a/tools/testing/selftests/bpf/prog_tests/test_bpffs.c b/tools/testing/selftests/bpf/prog_tests/test_bpffs.c
> index e8021ff0581c..95a7d2582ba7 100644
> --- a/tools/testing/selftests/bpf/prog_tests/test_bpffs.c
> +++ b/tools/testing/selftests/bpf/prog_tests/test_bpffs.c
> @@ -11,15 +11,18 @@
>  /* TDIR must be in a location we can create a directory in. */
>  #define TDIR "/tmp/test_bpffs_testdir"
>  
> -static int read_iter(char *file)
> +static int read_iter(const char *file, int *save_errno)
>  {
>  	/* 1024 should be enough to get contiguous 4 "iter" letters at some point */
>  	char buf[1024];
>  	int fd, len;
>  
>  	fd = open(file, O_RDONLY);
> -	if (fd < 0)
> +	if (fd < 0) {
> +		if (save_errno)
> +			*save_errno = errno;
>  		return -1;
> +	}
>  	while ((len = read(fd, buf, sizeof(buf) - 1)) > 0) {
>  		buf[len] = '\0';
>  		if (strstr(buf, "iter")) {
> @@ -27,10 +30,33 @@ static int read_iter(char *file)
>  			return 0;
>  		}
>  	}
> +	if (save_errno)
> +		*save_errno = (len < 0) ? errno : 0;
>  	close(fd);
>  	return -1;
>  }
>  
> +static bool is_iter_skip_err(int err, int serrno)
> +{
> +	return err && (serrno == EPERM || serrno == EACCES || serrno == ENOENT);
> +}
> +
> +static int read_iter_or_skip(const char *file)
> +{
> +	int serrno = 0;
> +	int err = read_iter(file, &serrno);
> +
> +	if (is_iter_skip_err(err, serrno)) {
> +		fprintf(stderr,
> +			"INFO: %s unavailable (%d), skipping iter check\n",
> +			file,
> +			serrno);

I don't think this is a good approach as it will mask genuine bugs that
would cause the files to be missing/inaccessible with one of the above
error codes.

If you really need to execute the rest of the test cases in the file,
how about you split the test into subtests and then just add the failing
subtest to your DENYLIST?

Viktor

> +		return 0;
> +	}
> +
> +	return err;
> +}
> +
>  static int fn(void)
>  {
>  	struct stat a, b, c;
> @@ -69,13 +95,16 @@ static int fn(void)
>  	if (!ASSERT_OK(err, "mount bpffs " TDIR "/fs2"))
>  		goto out;
>  
> -	err = read_iter(TDIR "/fs1/maps.debug");
> -	if (!ASSERT_OK(err, "reading " TDIR "/fs1/maps.debug"))
> +	err = read_iter_or_skip(TDIR "/fs1/maps.debug");
> +	if (err) {
> +		ASSERT_OK(err, "reading " TDIR "/fs1/maps.debug");
>  		goto out;
> -	err = read_iter(TDIR "/fs2/progs.debug");
> -	if (!ASSERT_OK(err, "reading " TDIR "/fs2/progs.debug"))
> +	}
> +	err = read_iter_or_skip(TDIR "/fs2/progs.debug");
> +	if (err) {
> +		ASSERT_OK(err, "reading " TDIR "/fs2/progs.debug");
>  		goto out;
> -
> +	}
>  	err = mkdir(TDIR "/fs1/a", 0777);
>  	if (!ASSERT_OK(err, "creating " TDIR "/fs1/a"))
>  		goto out;

Re: [PATCH 1/2] selftests/bpf: Fix read_iter buffer termination in test_bpffs

[sun jian]

On Tue, Mar 10, 2026 at 8:28 PM Viktor Malik <vmalik@redhat.com> wrote:
>
> Does this fix any real issue with the test? I can see one very
> hypothetical false negative when the uninitialized memory would contain
> "iter" but that seems very unlikely to happen.
>

Hi Viktor,

Thanks for the feedback.

Even if the probability of a false positive is low, the current code
is still incorrect:
on short reads it NUL-terminates only at the end of the buffer, so strstr() can
scan uninitialized stack bytes. That makes the helper potentially
non-deterministic.

Terminating at len makes it deterministic and ensures we only inspect
data actually
returned by read().

Regards,
Sun Jian

Re: [PATCH 2/2] selftests/bpf: Skip bpffs debug iter checks when unavailable in test_bpffs

[sun jian]

On Tue, Mar 10, 2026 at 8:32 PM Viktor Malik <vmalik@redhat.com> wrote:
>
> I don't think this is a good approach as it will mask genuine bugs that
> would cause the files to be missing/inaccessible with one of the above
> error codes.
>
Hi Viktor，

I don't think this change masks bugs in a silent way. It prints an explicit
INFO line when maps.debug/progs.debug are unavailable due to
EPERM/EACCES/ENOENT, and it still fails on other errors or unexpected
iterator output.

> If you really need to execute the rest of the test cases in the file,
> how about you split the test into subtests and then just add the failing
> subtest to your DENYLIST?

Proper subtests  would require reworking the current fork-based flow to
report SKIP/FAIL to the harness, which would be a larger refactor than
intended here.

My goal is to keep this as a minimal change while still achieving the main
purpose of this test: exercising the core bpffs operations (pinning and
renameat2 semantics), which are independent of the debug iterator files,
and clearly reporting when the iterator checks couldn't be performed.

Regards,
Sun Jian

Re: [PATCH 2/2] selftests/bpf: Skip bpffs debug iter checks when unavailable in test_bpffs

[Viktor Malik]

On 3/11/26 08:55, sun jian wrote:
> On Tue, Mar 10, 2026 at 8:32 PM Viktor Malik <vmalik@redhat.com> wrote:
>>
>> I don't think this is a good approach as it will mask genuine bugs that
>> would cause the files to be missing/inaccessible with one of the above
>> error codes.
>>
> Hi Viktor，
> 
> I don't think this change masks bugs in a silent way. It prints an explicit
> INFO line when maps.debug/progs.debug are unavailable due to
> EPERM/EACCES/ENOENT, and it still fails on other errors or unexpected
> iterator output.

Yeah but automatic CI doesn't care about INFO lines. It cares about
pass/fail status and in this case, the test could falsely appear as
passing, even though there's a bug in bpffs code causing the files not
to exist or have wrong permissions.

>> If you really need to execute the rest of the test cases in the file,
>> how about you split the test into subtests and then just add the failing
>> subtest to your DENYLIST?
> 
> Proper subtests  would require reworking the current fork-based flow to
> report SKIP/FAIL to the harness, which would be a larger refactor than
> intended here.

I don't think it would be such a big refactoring. You can still use
fork(), you just need to have separate `run` functions for different
subtests. True, the init/teardown phase would be somewhat duplicated but
I wouldn't see that as a big issue (and you can still factor it out).

> My goal is to keep this as a minimal change while still achieving the main
> purpose of this test: exercising the core bpffs operations (pinning and
> renameat2 semantics), which are independent of the debug iterator files,
> and clearly reporting when the iterator checks couldn't be performed.

That's exactly what subtests are for, at least in my view. You want to
check independent operations, you create a separate subtest for each.
That way, you can filter out some subtests if they don't pass in your
environment.

Re: [PATCH 1/2] selftests/bpf: Fix read_iter buffer termination in test_bpffs

[Viktor Malik]

On 3/11/26 07:47, sun jian wrote:
> On Tue, Mar 10, 2026 at 8:28 PM Viktor Malik <vmalik@redhat.com> wrote:
>>
>> Does this fix any real issue with the test? I can see one very
>> hypothetical false negative when the uninitialized memory would contain
>> "iter" but that seems very unlikely to happen.
>>
> 
> Hi Viktor,
> 
> Thanks for the feedback.
> 
> Even if the probability of a false positive is low, the current code
> is still incorrect:

It's actually a false negative, i.e. test reporting as passing because
it finds "iter" in the random bytes while it's actually missing. In an
absolute majority of cases, the random bytes wouldn't matter because
"iter" would be found early in the initialized part of the buffer.

So, since this fix only makes difference for cases when the test is
failing, I don't think it's worth patching it.

But I'll let maintainers decide, I'm not sure what the common practice
is for these cases.

Viktor

> on short reads it NUL-terminates only at the end of the buffer, so strstr() can
> scan uninitialized stack bytes. That makes the helper potentially
> non-deterministic.
> 
> Terminating at len makes it deterministic and ensures we only inspect
> data actually
> returned by read().
> 
> Regards,
> Sun Jian
> 

Re: [PATCH 1/2] selftests/bpf: Fix read_iter buffer termination in test_bpffs

[Yonghong Song]

On 3/10/26 3:21 AM, Sun Jian wrote:
> read_iter() always NUL-terminated at the end of the buffer, so strstr()
> could scan uninitialized stack bytes on short reads. Terminate at len and
> use O_RDONLY.
>
> Signed-off-by: Sun Jian <sun.jian.kdev@gmail.com>
> ---
>   tools/testing/selftests/bpf/prog_tests/test_bpffs.c | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/test_bpffs.c b/tools/testing/selftests/bpf/prog_tests/test_bpffs.c
> index ea933fd151c3..e8021ff0581c 100644
> --- a/tools/testing/selftests/bpf/prog_tests/test_bpffs.c
> +++ b/tools/testing/selftests/bpf/prog_tests/test_bpffs.c
> @@ -17,11 +17,11 @@ static int read_iter(char *file)
>   	char buf[1024];
>   	int fd, len;
>   
> -	fd = open(file, 0);
> +	fd = open(file, O_RDONLY);
>   	if (fd < 0)
>   		return -1;
> -	while ((len = read(fd, buf, sizeof(buf))) > 0) {
> -		buf[sizeof(buf) - 1] = '\0';
> +	while ((len = read(fd, buf, sizeof(buf) - 1)) > 0) {
> +		buf[len] = '\0';
>   		if (strstr(buf, "iter")) {
>   			close(fd);
>   			return 0;

The change here is unnecessary. Typically /sys/fs/bpf/ should be mounted
earlier and the 'iter' should appear with early prog/map id's. This ensure
'iter' will be discovered earlier. If you have an issue for this, fix
your setup.

>
> base-commit: 1f318b96cc84d7c2ab792fcc0bfd42a7ca890681

Re: [PATCH 2/2] selftests/bpf: Skip bpffs debug iter checks when unavailable in test_bpffs

[Yonghong Song]

On 3/10/26 3:21 AM, Sun Jian wrote:
> maps.debug/progs.debug may be inaccessible or absent (EPERM/EACCES/ENOENT)
> in some environments, which used to abort test_bpffs before it exercised

In which situation these two files have issues? This already root
access. If file operation failed, that must be due to your special
situation.

> the pin and rename checks. Treat these errors as "iter unavailable", log
> an INFO message, and continue with the core bpffs tests.
>
> Signed-off-by: Sun Jian <sun.jian.kdev@gmail.com>
> ---
>   .../selftests/bpf/prog_tests/test_bpffs.c     | 43 ++++++++++++++++---
>   1 file changed, 36 insertions(+), 7 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/test_bpffs.c b/tools/testing/selftests/bpf/prog_tests/test_bpffs.c
> index e8021ff0581c..95a7d2582ba7 100644
> --- a/tools/testing/selftests/bpf/prog_tests/test_bpffs.c
> +++ b/tools/testing/selftests/bpf/prog_tests/test_bpffs.c
> @@ -11,15 +11,18 @@
>   /* TDIR must be in a location we can create a directory in. */
>   #define TDIR "/tmp/test_bpffs_testdir"
>   
> -static int read_iter(char *file)
> +static int read_iter(const char *file, int *save_errno)
>   {
>   	/* 1024 should be enough to get contiguous 4 "iter" letters at some point */
>   	char buf[1024];
>   	int fd, len;
>   
>   	fd = open(file, O_RDONLY);
> -	if (fd < 0)
> +	if (fd < 0) {
> +		if (save_errno)
> +			*save_errno = errno;
>   		return -1;
> +	}
>   	while ((len = read(fd, buf, sizeof(buf) - 1)) > 0) {
>   		buf[len] = '\0';
>   		if (strstr(buf, "iter")) {
> @@ -27,10 +30,33 @@ static int read_iter(char *file)
>   			return 0;
>   		}
>   	}
> +	if (save_errno)
> +		*save_errno = (len < 0) ? errno : 0;
>   	close(fd);
>   	return -1;
>   }
>   
> +static bool is_iter_skip_err(int err, int serrno)
> +{
> +	return err && (serrno == EPERM || serrno == EACCES || serrno == ENOENT);
> +}
> +
> +static int read_iter_or_skip(const char *file)
> +{
> +	int serrno = 0;
> +	int err = read_iter(file, &serrno);
> +
> +	if (is_iter_skip_err(err, serrno)) {
> +		fprintf(stderr,
> +			"INFO: %s unavailable (%d), skipping iter check\n",
> +			file,
> +			serrno);
> +		return 0;
> +	}
> +
> +	return err;
> +}
> +
>   static int fn(void)
>   {
>   	struct stat a, b, c;
> @@ -69,13 +95,16 @@ static int fn(void)
>   	if (!ASSERT_OK(err, "mount bpffs " TDIR "/fs2"))
>   		goto out;
>   
> -	err = read_iter(TDIR "/fs1/maps.debug");
> -	if (!ASSERT_OK(err, "reading " TDIR "/fs1/maps.debug"))
> +	err = read_iter_or_skip(TDIR "/fs1/maps.debug");
> +	if (err) {
> +		ASSERT_OK(err, "reading " TDIR "/fs1/maps.debug");
>   		goto out;
> -	err = read_iter(TDIR "/fs2/progs.debug");
> -	if (!ASSERT_OK(err, "reading " TDIR "/fs2/progs.debug"))
> +	}
> +	err = read_iter_or_skip(TDIR "/fs2/progs.debug");
> +	if (err) {
> +		ASSERT_OK(err, "reading " TDIR "/fs2/progs.debug");
>   		goto out;
> -
> +	}
>   	err = mkdir(TDIR "/fs1/a", 0777);
>   	if (!ASSERT_OK(err, "creating " TDIR "/fs1/a"))
>   		goto out;

Re: [PATCH 2/2] selftests/bpf: Skip bpffs debug iter checks when unavailable in test_bpffs

[sun jian]

On Fri, Mar 13, 2026 at 5:44 AM Yonghong Song <yonghong.song@linux.dev> wrote:

> In which situation these two files have issues? This already root

I observed this in a virtme-ng guest running as root, where accessing
maps.debug/progs.debug returned EPERM while the later pin/rename checks
still worked. root in the guest was not sufficient for accessing those debug
 iterator files.

> access. If file operation failed, that must be due to your special
> situation.

Agreed, this looks environment-specific. I'll check it further.

Regards,
Sun Jian