On 10/16/24 03:18, zhouyuhang wrote:
在 2024/10/15 23:31, Shuah Khan 写道:
On 10/15/24 04:59, zhouyuhang wrote:
From: zhouyuhang <zhouyuhang@xxxxxxxxxx>
The libcap commit aca076443591 ("Make cap_t operations thread safe.")
added a __u8 mutex at the beginning of the struct _cap_struct, it changes
the offset of the members in the structure that breaks the assumption
made in the "struct libcap" definition in clone3_cap_checkpoint_restore.c.
This will make the test fail. So use the capget and capset syscall
directly and remove the libcap library dependency like the
commit 663af70aabb7 ("bpf: selftests: Add helpers to directly use
the capget and capset syscall") does.
Signed-off-by: zhouyuhang <zhouyuhang@xxxxxxxxxx>
---
tools/testing/selftests/clone3/Makefile | 1 -
.../clone3/clone3_cap_checkpoint_restore.c | 53 ++++++++-----------
.../selftests/clone3/clone3_cap_helpers.h | 23 ++++++++
3 files changed, 44 insertions(+), 33 deletions(-)
create mode 100644 tools/testing/selftests/clone3/clone3_cap_helpers.h
diff --git a/tools/testing/selftests/clone3/Makefile b/tools/testing/selftests/clone3/Makefile
index 84832c369a2e..59d26e8da8d2 100644
--- a/tools/testing/selftests/clone3/Makefile
+++ b/tools/testing/selftests/clone3/Makefile
@@ -1,6 +1,5 @@
# SPDX-License-Identifier: GPL-2.0
CFLAGS += -g -std=gnu99 $(KHDR_INCLUDES)
-LDLIBS += -lcap
TEST_GEN_PROGS := clone3 clone3_clear_sighand clone3_set_tid \
clone3_cap_checkpoint_restore
diff --git a/tools/testing/selftests/clone3/clone3_cap_checkpoint_restore.c b/tools/testing/selftests/clone3/clone3_cap_checkpoint_restore.c
index 3c196fa86c99..242088eeec88 100644
--- a/tools/testing/selftests/clone3/clone3_cap_checkpoint_restore.c
+++ b/tools/testing/selftests/clone3/clone3_cap_checkpoint_restore.c
@@ -15,7 +15,6 @@
#include <stdio.h>
#include <stdlib.h>
#include <stdbool.h>
-#include <sys/capability.h>
#include <sys/prctl.h>
#include <sys/syscall.h>
#include <sys/types.h>
@@ -26,6 +25,7 @@
#include "../kselftest_harness.h"
#include "clone3_selftests.h"
+#include "clone3_cap_helpers.h"
static void child_exit(int ret)
{
@@ -87,47 +87,36 @@ static int test_clone3_set_tid(struct __test_metadata *_metadata,
return ret;
}
-struct libcap {
- struct __user_cap_header_struct hdr;
- struct __user_cap_data_struct data[2];
-};
-
static int set_capability(void)
{
- cap_value_t cap_values[] = { CAP_SETUID, CAP_SETGID };
- struct libcap *cap;
- int ret = -1;
- cap_t caps;
-
- caps = cap_get_proc();
- if (!caps) {
- perror("cap_get_proc");
+ struct __user_cap_data_struct data[2];
+ struct __user_cap_header_struct hdr = {
+ .version = _LINUX_CAPABILITY_VERSION_3,
+ };
+ __u32 cap0 = 1 << CAP_SETUID | 1 << CAP_SETGID;
+ __u32 cap1 = 1 << (CAP_CHECKPOINT_RESTORE - 32);
+ int ret;
+
+ ret = capget(&hdr, data);
+ if (ret) {
+ perror("capget");
return -1;
}
/* Drop all capabilities */
- if (cap_clear(caps)) {
- perror("cap_clear");
- goto out;
- }
+ memset(&data, 0, sizeof(data));
- cap_set_flag(caps, CAP_EFFECTIVE, 2, cap_values, CAP_SET);
- cap_set_flag(caps, CAP_PERMITTED, 2, cap_values, CAP_SET);
+ data[0].effective |= cap0;
+ data[0].permitted |= cap0;
- cap = (struct libcap *) caps;
+ data[1].effective |= cap1;
+ data[1].permitted |= cap1;
- /* 40 -> CAP_CHECKPOINT_RESTORE */
- cap->data[1].effective |= 1 << (40 - 32);
- cap->data[1].permitted |= 1 << (40 - 32);
-
- if (cap_set_proc(caps)) {
- perror("cap_set_proc");
- goto out;
+ ret = capset(&hdr, data);
+ if (ret) {
+ perror("capset");
+ return -1;
}
- ret = 0;
-out:
- if (cap_free(caps))
- perror("cap_free");
return ret;
}
diff --git a/tools/testing/selftests/clone3/clone3_cap_helpers.h b/tools/testing/selftests/clone3/clone3_cap_helpers.h
new file mode 100644
index 000000000000..3fa59ef68fb8
--- /dev/null
+++ b/tools/testing/selftests/clone3/clone3_cap_helpers.h
@@ -0,0 +1,23 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __CLONE3_CAP_HELPERS_H
+#define __CLONE3_CAP_HELPERS_H
+
+#include <linux/capability.h>
+
+/*
+ * Compatible with older version
+ * header file without defined
+ * CAP_CHECKPOINT_RESTORE.
+ */
+#ifndef CAP_CHECKPOINT_RESTORE
+#define CAP_CHECKPOINT_RESTORE 40
+#endif
+
+/*
+ * Removed the libcap library dependency.
+ * So declare them here directly.
+ */
+int capget(cap_user_header_t header, cap_user_data_t data);
+int capset(cap_user_header_t header, const cap_user_data_t data);
Sorry you haven't addressed my comments on your v1 yet.
I repeat that this is not the right direction to define system
calls locally.
I got it. I am willing to modify the code so that syscalls are not defined in local files,
but this would require including sys/capability.h which would not remove the
dependency on the libcap library. So, should we directly use syscalls or use the
libcap library function in the "set_capability" function, or do you have a better way.
I'd like to refer to your advice.
Try this:
Run make headers in the kernel repo.
Build without making any changes.
Then add you changes and add linux/capability.h to include files
Tell me what happens.
thanks,
-- Shuah
I tried this, here are my steps.
Firstly, I ran 'make headers' in the kernel repo and it was successful.
Then I wasn't quite sure which path you were referring to as' build ',
Sorry if what I said wasn't clear:
- This test depends on libcap and yes you will have to install it.
- Run ake headers in the kernel repo.
- Build the test without your patch (changes)
- If you don't have libcap, the test build will fail
- Install libcap
- Build and run.
Looks like you have done the above. Now:
- Add your patch without the local capget() and capset()
and without removing
so I compiled and installed libcap, and also compiled test, all of which were successful.
Why do you need to compile libcap? Is it because this latest
change isn't available to install from the distro you are using?
Afterwards, I applied my patch and the test was successfully built and running.
I guess what you're trying to express may be that these system calls have already
been defined in sys/capability, and those defined in the local file are duplicated with it.
Correct. You don't need the local defines and in fact you should not
define them locally.
So I included sys/capability.h and linux/capability.h and defined the system calls in the test,
but there were no errors.
Please don't define system calls locally. What happens if you don't?
thanks,
-- Shuah
