Re: [PATCH v3] selftests: clone3: Use the capget and capset syscall directly

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 10/29/24 20:50, 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 cause the test case to fail with the following output:

  #  RUN           global.clone3_cap_checkpoint_restore ...
  # clone3() syscall supported
  # clone3_cap_checkpoint_restore.c:151:clone3_cap_checkpoint_restore:Child has PID 130508
  cap_set_proc: Operation not permitted

Sounds like EPERM is returned here. What's the error number from
cap_set_proc().
  # clone3_cap_checkpoint_restore.c:160:clone3_cap_checkpoint_restore:Expected set_capability() (-1) == 0 (0)

What's the error number here? Looks like this test simply
uses perror - it is better to use strerror() which includes
the error number.

Is this related EPERM?
  # clone3_cap_checkpoint_restore.c:161:clone3_cap_checkpoint_restore:Could not set CAP_CHECKPOINT_RESTORE
  # clone3_cap_checkpoint_restore: Test terminated by assertion
  #          FAIL  global.clone3_cap_checkpoint_restore

Changing to using capget and capset syscall directly here can fix this error,
just like what the commit 663af70aabb7 ("bpf: selftests: Add helpers to directly
use the capget and capset syscall") does.

Is this still accurate for v3 - Does this patch match the
bpf commit?

What is the output with this patch? Include it in the change log.


Signed-off-by: zhouyuhang <zhouyuhang@xxxxxxxxxx>> ---

Please mention the changes from v2 to v3 here so it makes it
easier for reviewers associating the changes to the reviewer.

I had to go look up v1 and v2.

  .../clone3/clone3_cap_checkpoint_restore.c    | 58 +++++++++----------
  1 file changed, 27 insertions(+), 31 deletions(-)

diff --git a/tools/testing/selftests/clone3/clone3_cap_checkpoint_restore.c b/tools/testing/selftests/clone3/clone3_cap_checkpoint_restore.c
index 3c196fa86c99..8b61702bf721 100644
--- a/tools/testing/selftests/clone3/clone3_cap_checkpoint_restore.c
+++ b/tools/testing/selftests/clone3/clone3_cap_checkpoint_restore.c
@@ -27,6 +27,13 @@
  #include "../kselftest_harness.h"
  #include "clone3_selftests.h"
+/*
+ * Prevent not being defined in the header file
+ */
+#ifndef CAP_CHECKPOINT_RESTORE
+#define CAP_CHECKPOINT_RESTORE 40
+#endif
+
  static void child_exit(int ret)
  {
  	fflush(stdout);
@@ -87,47 +94,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,

cap_validate_magic() handles _LINUX_CAPABILITY_VERSION_1,
_LINUX_CAPABILITY_VERSION_2, and _LINUX_CAPABILITY_VERSION_3

It would help to add a comment on why it is necessary to
set the version here.

+	};
+	__u32 cap0 = 1 << CAP_SETUID | 1 << CAP_SETGID;
+	__u32 cap1 = 1 << (CAP_CHECKPOINT_RESTORE - 32);

Explain why this is necessary - a comment will help future
maintenance of this code.

+	int ret;
+
+	ret = capget(&hdr, data);
+	if (ret) {
+		perror("capget");


  		return -1;
  	}
/* Drop all capabilities */
-	if (cap_clear(caps)) {
-		perror("cap_clear");
-		goto out;
-	}
-
-	cap_set_flag(caps, CAP_EFFECTIVE, 2, cap_values, CAP_SET);
-	cap_set_flag(caps, CAP_PERMITTED, 2, cap_values, CAP_SET);
+	memset(&data, 0, sizeof(data));
- cap = (struct libcap *) caps;
+	data[0].effective |= cap0;
+	data[0].permitted |= cap0;
- /* 40 -> CAP_CHECKPOINT_RESTORE */
-	cap->data[1].effective |= 1 << (40 - 32);
-	cap->data[1].permitted |= 1 << (40 - 32);
+	data[1].effective |= cap1;
+	data[1].permitted |= cap1;
- 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;
  }





[Index of Archives]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Share Photos]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]

  Powered by Linux