Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1574300 > unrolled thread

[PATCH net-next v1 7/7] bpf: Always test unprivileged programs

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH net-next v1 7/7] bpf: Always test unprivileged programs Mickaël Salaün <mic@digikod.net> - 2017-02-06 00:20 +0100
    Re: [PATCH net-next v1 7/7] bpf: Always test unprivileged programs Daniel Borkmann <daniel@iogearbox.net> - 2017-02-06 16:50 +0100
    Re: [PATCH net-next v1 7/7] bpf: Always test unprivileged programs Mickaël Salaün <mic@digikod.net> - 2017-02-06 20:30 +0100

#1574300 — [PATCH net-next v1 7/7] bpf: Always test unprivileged programs

If selftests are run as root, then execute the unprivileged checks as
well. This switch from 240 to 364 tests.

The test numbers are suffixed with "/u" when executed as unprivileged or
with "/p" when executed as privileged.

The geteuid() check is replaced with a capability check.

Handling capabilities require the libcap dependency.

Signed-off-by: Mickaël Salaün <mic@digikod.net>
Cc: Alexei Starovoitov <ast@fb.com>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: Shuah Khan <shuah@kernel.org>
---
 tools/testing/selftests/bpf/Makefile        |  2 +-
 tools/testing/selftests/bpf/test_verifier.c | 68 ++++++++++++++++++++++++++---
 2 files changed, 64 insertions(+), 6 deletions(-)

diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index 712861492278..30bb40261692 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -1,4 +1,4 @@
-CFLAGS += -Wall -O2 -I../../../include/uapi -I../../../lib
+CFLAGS += -Wall -O2 -lcap -I../../../include/uapi -I../../../lib
 
 test_objs = test_verifier test_tag test_maps test_lru_map test_lpm_map
 
diff --git a/tools/testing/selftests/bpf/test_verifier.c b/tools/testing/selftests/bpf/test_verifier.c
index 04a549e54f61..aa42aef22b85 100644
--- a/tools/testing/selftests/bpf/test_verifier.c
+++ b/tools/testing/selftests/bpf/test_verifier.c
@@ -16,6 +16,7 @@
 #include <stdbool.h>
 #include <sched.h>
 
+#include <sys/capability.h>
 #include <sys/resource.h>
 
 #include <linux/unistd.h>
@@ -4498,6 +4499,55 @@ static void do_test_single(struct bpf_test *test, bool unpriv,
 	goto close_fds;
 }
 
+static bool is_admin(void)
+{
+	cap_t caps;
+	cap_flag_value_t sysadmin = CAP_CLEAR;
+	const cap_value_t cap_val = CAP_SYS_ADMIN;
+
+	if (!CAP_IS_SUPPORTED(CAP_SETFCAP)) {
+		perror("cap_get_flag");
+		return false;
+	}
+	caps = cap_get_proc();
+	if (!caps) {
+		perror("cap_get_proc");
+		return false;
+	}
+	if (cap_get_flag(caps, cap_val, CAP_EFFECTIVE, &sysadmin))
+		perror("cap_get_flag");
+	if (cap_free(caps) == -1)
+		perror("cap_free");
+	return (sysadmin == CAP_SET);
+}
+
+static int set_admin(bool admin)
+{
+	cap_t caps;
+	const cap_value_t cap_val = CAP_SYS_ADMIN;
+	int ret = -1;
+
+	caps = cap_get_proc();
+	if (!caps) {
+		perror("cap_get_proc");
+		return -1;
+	}
+	if (cap_set_flag(caps, CAP_EFFECTIVE, 1, &cap_val,
+				admin ? CAP_SET : CAP_CLEAR)) {
+		perror("cap_set_flag");
+		goto out;
+	}
+	if (cap_set_proc(caps)) {
+		perror("cap_set_proc");
+		goto out;
+	}
+	ret = 0;
+out:
+	if (cap_free(caps) == -1)
+		perror("cap_free");
+	return ret;
+}
+
 static int do_test(bool unpriv, unsigned int from, unsigned int to)
 {
 	int i, passes = 0, errors = 0;
@@ -4508,11 +4558,19 @@ static int do_test(bool unpriv, unsigned int from, unsigned int to)
 		/* Program types that are not supported by non-root we
 		 * skip right away.
 		 */
-		if (unpriv && test->prog_type)
-			continue;
+		if (!test->prog_type) {
+			if (!unpriv)
+				set_admin(false);
+			printf("#%d/u %s ", i, test->descr);
+			do_test_single(test, true, &passes, &errors);
+			if (!unpriv)
+				set_admin(true);
+		}
 
-		printf("#%d %s ", i, test->descr);
-		do_test_single(test, unpriv, &passes, &errors);
+		if (!unpriv) {
+			printf("#%d/p %s ", i, test->descr);
+			do_test_single(test, false, &passes, &errors);
+		}
 	}
 
 	printf("Summary: %d PASSED, %d FAILED\n", passes, errors);
@@ -4524,7 +4582,7 @@ int main(int argc, char **argv)
 	struct rlimit rinf = { RLIM_INFINITY, RLIM_INFINITY };
 	struct rlimit rlim = { 1 << 20, 1 << 20 };
 	unsigned int from = 0, to = ARRAY_SIZE(tests);
-	bool unpriv = geteuid() != 0;
+	bool unpriv = !is_admin();
 
 	if (argc == 3) {
 		unsigned int l = atoi(argv[argc - 2]);
-- 
2.11.0

[toc] | [next] | [standalone]

#1574916

On 02/06/2017 12:14 AM, Mickaël Salaün wrote:
> If selftests are run as root, then execute the unprivileged checks as
> well. This switch from 240 to 364 tests.
>
> The test numbers are suffixed with "/u" when executed as unprivileged or
> with "/p" when executed as privileged.
>
> The geteuid() check is replaced with a capability check.
>
> Handling capabilities require the libcap dependency.
>
> Signed-off-by: Mickaël Salaün <mic@digikod.net>
> Cc: Alexei Starovoitov <ast@fb.com>
> Cc: Daniel Borkmann <daniel@iogearbox.net>
> Cc: Shuah Khan <shuah@kernel.org>

Very useful! Could probably also go as stand-alone to net-next,
but how you prefer.

Acked-by: Daniel Borkmann <daniel@iogearbox.net>

[toc] | [prev] | [next] | [standalone]

#1575089

[Multipart message — attachments visible in raw view] — view raw

On 06/02/2017 17:09, Alexei Starovoitov wrote:
> On 2/5/17 3:14 PM, Mickaël Salaün wrote:
>> -        if (unpriv && test->prog_type)
>> -            continue;
>> +        if (!test->prog_type) {
>> +            if (!unpriv)
>> +                set_admin(false);
>> +            printf("#%d/u %s ", i, test->descr);
>> +            do_test_single(test, true, &passes, &errors);
>> +            if (!unpriv)
>> +                set_admin(true);
>> +        }
>>
>> -        printf("#%d %s ", i, test->descr);
>> -        do_test_single(test, unpriv, &passes, &errors);
>> +        if (!unpriv) {
>> +            printf("#%d/p %s ", i, test->descr);
>> +            do_test_single(test, false, &passes, &errors);
>> +        }
> 
> great idea.
> Acked-by: Alexei Starovoitov <ast@kernel.org>
> 
> as far as other patches.. we need to figure out how to avoid conflicts
> between net-next and Arnaldo's tree where Joe's patches went.

A merge between this series and Arnaldo's tree works fine. The only
dependency is between patches 6 and 7.

> 
> Mickael,
> can you see some way of splitting the patch set between trees?
> Like above test_verfier.c improvement needs to go into net-next.
> The rest can go via perf
> 
> 

OK, I'll send a first series with the patches from 1 to 5 for the perf
tree and a second series with the 6th and 7th patches (touching
tools/testing/selftests/bpf only) to net-next.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web