selftests/bpf: Strerror expects positive number

Commit Message

Feng Yang March 5, 2025, 2:22 a.m. UTC

From: Feng Yang <[e-mail]>

Before:
  failed to restore CAP_SYS_ADMIN: -1, Unknown error -1
  ...

After:
  failed to restore CAP_SYS_ADMIN: -3, No such process
  failed to restore CAP_SYS_ADMIN: -22, Invalid argument
  ...

Signed-off-by: Feng Yang <[e-mail]>
---
Changes in v2:
- cap_{enable,disable}_effective return -errno. Thanks, Eduard Zingerman!
- Link to v1: https://lore.kernel.org/all/[e-mail]
---
 tools/testing/selftests/bpf/cap_helpers.c         | 8 ++++----
 tools/testing/selftests/bpf/cap_helpers.h         | 1 +
 tools/testing/selftests/bpf/prog_tests/verifier.c | 4 ++--
 tools/testing/selftests/bpf/test_loader.c         | 6 +++---
 4 files changed, 10 insertions(+), 9 deletions(-)

Comments

Eduard Zingerman March 5, 2025, 9:49 p.m. UTC | #1

On Wed, 2025-03-05 at 10:22 +0800, Feng Yang wrote:
> From: Feng Yang <[e-mail]>
> 
> Before:
>   failed to restore CAP_SYS_ADMIN: -1, Unknown error -1
>   ...
> 
> After:
>   failed to restore CAP_SYS_ADMIN: -3, No such process
>   failed to restore CAP_SYS_ADMIN: -22, Invalid argument
>   ...
> 
> Signed-off-by: Feng Yang <[e-mail]>
> ---
> Changes in v2:
> - cap_{enable,disable}_effective return -errno. Thanks, Eduard Zingerman!
> - Link to v1: https://lore.kernel.org/all/[e-mail]
> ---

Thank you for fixing this.
Nit: for a v2 of the patch please use '[PATCH v2]' tag.

Acked-by: Eduard Zingerman <[e-mail]>

[...]

[e-mail] March 7, 2025, 5:20 p.m. UTC | #2

Hello:

This patch was applied to bpf/bpf-next.git (master)
by Alexei Starovoitov <[e-mail]>:

On Wed,  5 Mar 2025 10:22:34 +0800 you wrote:
> From: Feng Yang <[e-mail]>
> 
> Before:
>   failed to restore CAP_SYS_ADMIN: -1, Unknown error -1
>   ...
> 
> After:
>   failed to restore CAP_SYS_ADMIN: -3, No such process
>   failed to restore CAP_SYS_ADMIN: -22, Invalid argument
>   ...
> 
> [...]

Here is the summary with links:
  - selftests/bpf: Strerror expects positive number
    https://git.kernel.org/bpf/bpf-next/c/7e437dcd3975

You are awesome, thank you!

Patch

diff --git a/tools/testing/selftests/bpf/cap_helpers.c b/tools/testing/selftests/bpf/cap_helpers.c
index d5ac507401d7..98f840c3a38f 100644
--- a/tools/testing/selftests/bpf/cap_helpers.c
+++ b/tools/testing/selftests/bpf/cap_helpers.c
@@ -19,7 +19,7 @@  int cap_enable_effective(__u64 caps, __u64 *old_caps)
 
 	err = capget(&hdr, data);
 	if (err)
-		return err;
+		return -errno;
 
 	if (old_caps)
 		*old_caps = (__u64)(data[1].effective) << 32 | data[0].effective;
@@ -32,7 +32,7 @@  int cap_enable_effective(__u64 caps, __u64 *old_caps)
 	data[1].effective |= cap1;
 	err = capset(&hdr, data);
 	if (err)
-		return err;
+		return -errno;
 
 	return 0;
 }
@@ -49,7 +49,7 @@  int cap_disable_effective(__u64 caps, __u64 *old_caps)
 
 	err = capget(&hdr, data);
 	if (err)
-		return err;
+		return -errno;
 
 	if (old_caps)
 		*old_caps = (__u64)(data[1].effective) << 32 | data[0].effective;
@@ -61,7 +61,7 @@  int cap_disable_effective(__u64 caps, __u64 *old_caps)
 	data[1].effective &= ~cap1;
 	err = capset(&hdr, data);
 	if (err)
-		return err;
+		return -errno;
 
 	return 0;
 }
diff --git a/tools/testing/selftests/bpf/cap_helpers.h b/tools/testing/selftests/bpf/cap_helpers.h
index 6d163530cb0f..8dcb28557f76 100644
--- a/tools/testing/selftests/bpf/cap_helpers.h
+++ b/tools/testing/selftests/bpf/cap_helpers.h
@@ -4,6 +4,7 @@ 
 
 #include <linux/types.h>
 #include <linux/capability.h>
+#include <errno.h>
 
 #ifndef CAP_PERFMON
 #define CAP_PERFMON		38
diff --git a/tools/testing/selftests/bpf/prog_tests/verifier.c b/tools/testing/selftests/bpf/prog_tests/verifier.c
index 8a0e1ff8a2dc..ecc320e04551 100644
--- a/tools/testing/selftests/bpf/prog_tests/verifier.c
+++ b/tools/testing/selftests/bpf/prog_tests/verifier.c
@@ -121,7 +121,7 @@  static void run_tests_aux(const char *skel_name,
 	/* test_verifier tests are executed w/o CAP_SYS_ADMIN, do the same here */
 	err = cap_disable_effective(1ULL << CAP_SYS_ADMIN, &old_caps);
 	if (err) {
-		PRINT_FAIL("failed to drop CAP_SYS_ADMIN: %i, %s\n", err, strerror(err));
+		PRINT_FAIL("failed to drop CAP_SYS_ADMIN: %i, %s\n", err, strerror(-err));
 		return;
 	}
 
@@ -131,7 +131,7 @@  static void run_tests_aux(const char *skel_name,
 
 	err = cap_enable_effective(old_caps, NULL);
 	if (err)
-		PRINT_FAIL("failed to restore CAP_SYS_ADMIN: %i, %s\n", err, strerror(err));
+		PRINT_FAIL("failed to restore CAP_SYS_ADMIN: %i, %s\n", err, strerror(-err));
 }
 
 #define RUN(skel) run_tests_aux(#skel, skel##__elf_bytes, NULL)
diff --git a/tools/testing/selftests/bpf/test_loader.c b/tools/testing/selftests/bpf/test_loader.c
index 53b06647cf57..8a403e5aa314 100644
--- a/tools/testing/selftests/bpf/test_loader.c
+++ b/tools/testing/selftests/bpf/test_loader.c
@@ -773,7 +773,7 @@  static int drop_capabilities(struct cap_state *caps)
 
 	err = cap_disable_effective(caps_to_drop, &caps->old_caps);
 	if (err) {
-		PRINT_FAIL("failed to drop capabilities: %i, %s\n", err, strerror(err));
+		PRINT_FAIL("failed to drop capabilities: %i, %s\n", err, strerror(-err));
 		return err;
 	}
 
@@ -790,7 +790,7 @@  static int restore_capabilities(struct cap_state *caps)
 
 	err = cap_enable_effective(caps->old_caps, NULL);
 	if (err)
-		PRINT_FAIL("failed to restore capabilities: %i, %s\n", err, strerror(err));
+		PRINT_FAIL("failed to restore capabilities: %i, %s\n", err, strerror(-err));
 	caps->initialized = false;
 	return err;
 }
@@ -959,7 +959,7 @@  void run_subtest(struct test_loader *tester,
 		if (subspec->caps) {
 			err = cap_enable_effective(subspec->caps, NULL);
 			if (err) {
-				PRINT_FAIL("failed to set capabilities: %i, %s\n", err, strerror(err));
+				PRINT_FAIL("failed to set capabilities: %i, %s\n", err, strerror(-err));
 				goto subtest_cleanup;
 			}
 		}