Re: [PATCH bpf-next v3 2/2] selftests/bpf: verify that rebinding to port < 1024 from BPF works

On Tue, Jan 26, 2021 at 10:18 AM Martin KaFai Lau [off-list ref] wrote:

On Tue, Jan 26, 2021 at 08:51:04AM -0800, Stanislav Fomichev wrote:

quoted

Return 3 to indicate that permission check for port 111
should be skipped.

[ ... ]

quoted

+void cap_net_bind_service(cap_flag_value_t flag)
+{
+     const cap_value_t cap_net_bind_service = CAP_NET_BIND_SERVICE;
+     cap_t caps;
+
+     caps = cap_get_proc();
+     if (CHECK(!caps, "cap_get_proc", "errno %d", errno))
+             goto free_caps;
+
+     if (CHECK(cap_set_flag(caps, CAP_EFFECTIVE, 1, &cap_net_bind_service,
+                            flag),
+               "cap_set_flag", "errno %d", errno))
+             goto free_caps;
+
+     if (CHECK(cap_set_proc(caps), "cap_set_proc", "errno %d", errno))
+             goto free_caps;
+
+free_caps:
+     if (CHECK(cap_free(caps), "cap_free", "errno %d", errno))
+             goto free_caps;

Also mentioned in v2, there is a loop.

Oops, missed that one, sorry.

quoted

+}
+
+void test_bind_perm(void)
+{
+     struct bind_perm *skel;
+     int cgroup_fd;
+
+     cgroup_fd = test__join_cgroup("/bind_perm");
+     if (CHECK(cgroup_fd < 0, "cg-join", "errno %d", errno))
+             return;
+
+     skel = bind_perm__open_and_load();
+     if (!ASSERT_OK_PTR(skel, "skel"))
+             goto close_cgroup_fd;
+
+     skel->links.bind_v4_prog = bpf_program__attach_cgroup(skel->progs.bind_v4_prog, cgroup_fd);
+     if (!ASSERT_OK_PTR(skel, "bind_v4_prog"))
+             goto close_skeleton;
+
+     cap_net_bind_service(CAP_CLEAR);
+     try_bind(110, EACCES);
+     try_bind(111, 0);
+     cap_net_bind_service(CAP_SET);

Instead of always CAP_SET at the end of the test,
it is better to do a cap_get_flag() to save the original value
at the beginning of the test and restore it at the end
of the test.

It might be easier to change cap_net_bind_service() to return a bool
which indicates that the flag was originally set.
If it wasn't, we can bypass cap_net_bind_service(CAP_SET).
Let me know if you strongly disagree, I'll try to play with this idea
and will send a v4 if it plays out nicely.