Re: [PATCH] selftests: clone3: Use the capget and capset syscall directly
On 10/15/24 03:00, zhouyuhang wrote:
[snip] for clarity.
Why is this necessary? This is defined in linux/capability.h.
Sorry for not noticing this before.
This is to be compatible with some older versions of linux/capability.h that do
not define this macro.
+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);
In general prototypes such as these should be defined in header
file. Why are we defining these here?
These are defined in sys/capability.h
I don't understand this change. You are removing sys/capability.h
which requires you to add these defines here. This doesn't
sound like a correct solution to me.
I tested it on my machine without libcap-dev installed, the
/usr/include/linux/capability.h
is on this machine by default. Successfully compiled using #include
but not with #include . This patch removes libcap library
dependencies.
And we don't use any part of sys/capability.h other than these two syscalls. So
I think that's why it's necessary.
You are changing the code to not include sys/capability.h
What happens if sys/capability.h along with linux/capability.h
Do you see problems?
I'm sorry, maybe I wasn't clear enough.
When we install the libcap library it will have the following output:
test@test:~/work/libcap$ sudo make install | grep capability
install -m 0644 include/sys/capability.h /usr/include/sys
install -m 0644 include/sys/capability.h /usr/include/sys
/usr/share/man/man5 capability.conf.5 \
It installs sys/capability.h in the correct location, but does not
install linux/capability.h, so sys/capability.h is bound to the libcap library
It won't install inux/capability.h unless you run "make headers" in
the kernel repo.
and they will either exist or disappear together. Now I want to remove
the dependency of the test on libcap library so I changed the code that it
does not contain sys/capability.h but instead linux/capability.h,
so that the test can compile successfully without libcap being installed,
these two syscalls are not declared in linux/capability.h(It is sufficient for
test use except for these two syscalls)
so we need to declare them here. I think that's why the commit 663af70aabb7
("bpf: selftests: Add helpers to directly use the capget and capset syscall") I
refered to
does the same thing. As for your question "What happens if sys/capability.h
along
with linux/capability.h", I haven't found the problem yet, I sincerely hope you
can
help me improve this code. Thank you very much.
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.
Try the above first.
The change you are making isn't correct. Because you don't want to
define system calls locally in your source file.
Thanks, I see.
Maybe I should move them to a new header file and resend a patch.
No. Please see above. I would rather not see these defined at all
locally.
thanks,
-- Shuah
Re: [PATCH] selftests: clone3: Use the capget and capset syscall directly
在 2024/10/15 06:38, Shuah Khan 写道:
On 10/12/24 02:28, zhouyuhang wrote:
On 2024/10/11 22:21, Shuah Khan wrote:
On 10/11/24 00:59, zhouyuhang wrote:
On 2024/10/10 23:50, Shuah Khan wrote:
On 10/10/24 06:16, zhouyuhang wrote:
From: zhouyuhang
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.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.
NIT: grammar and comma spacing. Please fix those for readability.
e.g: Change "struct _cap_struct,it" to "struct _cap_struct, it"
Fix others as well.
Thanks, I'll fix it in V2
Signed-off-by: zhouyuhang
---
tools/testing/selftests/clone3/Makefile | 1 -
.../clone3/clone3_cap_checkpoint_restore.c | 60
+--
2 files changed, 28 insertions(+), 33 deletions(-)
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..111912e2aead 100644
--- a/tools/testing/selftests/clone3/clone3_cap_checkpoint_restore.c
+++ b/tools/testing/selftests/clone3/clone3_cap_checkpoint_restore.c
@@ -15,7 +15,7 @@
#include
#include
#include
-#include
+#include
#include
#include
#include
@@ -27,6 +27,13 @@
#include "../kselftest_harness.h"
#include "clone3_selftests.h"
+#ifndef CAP_CHECKPOINT_RESTORE
+#define CAP_CHECKPOINT_RESTORE 40
+#endif
+
Why is this necessary? This is defined in linux/capability.h.
Sorry for not noticing this before.
This is to be compatible with some older versions of linux/capability.h
that do not define this macro.
+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);
In general prototypes such as these should be defined in header
file. Why are we defining these here?
These are defined in sys/capability.h
I don't understand this change. You are removing sys/capability.h
which requires you to add these defines here. This doesn't
sound like a correct solution to me.
I tested it on my machine without libcap-dev installed, the
/usr/include/linux/capability.h
is on this machine by default. Successfully compiled using #include
but not with #include . This patch removes libcap
library dependencies.
And we don't use any part of sys/capability.h other than these two
syscalls. So I think that's why it's necessary.
You are changing the code to not include sys/capability.h
What happens if sys/capability.h along with linux/capability.h
Do you see problems?
I'm sorry, maybe I wasn't clear enough.
When we install the libcap library it will have the following output:
test@test:~/work/libcap$ sudo make install | grep capability
install -m 0644 include/sys/capability.h /usr/include/sys
install -m 0644 include/sys/capability.h /usr/include/sys
/usr/share/man/man5 capability.conf.5 \
It installs sys/capability.h in the correct location, but does not
install linux/capability.h, so sys/capability.h is bound to the
libcap library
It won't install inux/capability.h unless you run "make headers" in
the kernel repo.
and they will either exist or disappear together. Now I want to remove
the dependency of the test on libcap library so I changed the code
that it
does not contain sys/capability.h but instead linux/capability.h,
so that the test can compile successfully without libcap being
installed,
these two syscalls are not declared in linux/capability.h(It is
sufficient for test use except for these two syscalls)
so we need to declare them here. I think that's why the commit
663af70aabb7
("bpf: selftests: Add helpers to directly use the capget and capset
syscall") I refered to
does the same thing. As for your question "What happens if
sys/capability.h along
with linux/capability.h", I haven't found the problem yet, I
sincerely hope you can
help me improve this code. Thank you very much.
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.
The change you are making isn't correct. Because you don't want to
define system calls locally in your
Re: [PATCH] selftests: clone3: Use the capget and capset syscall directly
On 10/12/24 02:28, zhouyuhang wrote:
On 2024/10/11 22:21, Shuah Khan wrote:
On 10/11/24 00:59, zhouyuhang wrote:
On 2024/10/10 23:50, Shuah Khan wrote:
On 10/10/24 06:16, zhouyuhang wrote:
From: zhouyuhang
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.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.
NIT: grammar and comma spacing. Please fix those for readability.
e.g: Change "struct _cap_struct,it" to "struct _cap_struct, it"
Fix others as well.
Thanks, I'll fix it in V2
Signed-off-by: zhouyuhang
---
tools/testing/selftests/clone3/Makefile | 1 -
.../clone3/clone3_cap_checkpoint_restore.c | 60 +--
2 files changed, 28 insertions(+), 33 deletions(-)
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..111912e2aead 100644
--- a/tools/testing/selftests/clone3/clone3_cap_checkpoint_restore.c
+++ b/tools/testing/selftests/clone3/clone3_cap_checkpoint_restore.c
@@ -15,7 +15,7 @@
#include
#include
#include
-#include
+#include
#include
#include
#include
@@ -27,6 +27,13 @@
#include "../kselftest_harness.h"
#include "clone3_selftests.h"
+#ifndef CAP_CHECKPOINT_RESTORE
+#define CAP_CHECKPOINT_RESTORE 40
+#endif
+
Why is this necessary? This is defined in linux/capability.h.
+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);
In general prototypes such as these should be defined in header
file. Why are we defining these here?
These are defined in sys/capability.h
I don't understand this change. You are removing sys/capability.h
which requires you to add these defines here. This doesn't
sound like a correct solution to me.
I tested it on my machine without libcap-dev installed, the
/usr/include/linux/capability.h
is on this machine by default. Successfully compiled using #include
but not with #include . This patch removes libcap library
dependencies.
And we don't use any part of sys/capability.h other than these two syscalls. So
I think that's why it's necessary.
You are changing the code to not include sys/capability.h
What happens if sys/capability.h along with linux/capability.h
Do you see problems?
I'm sorry, maybe I wasn't clear enough.
When we install the libcap library it will have the following output:
test@test:~/work/libcap$ sudo make install | grep capability
install -m 0644 include/sys/capability.h /usr/include/sys
install -m 0644 include/sys/capability.h /usr/include/sys
/usr/share/man/man5 capability.conf.5 \
It installs sys/capability.h in the correct location, but does not
install linux/capability.h, so sys/capability.h is bound to the libcap library
It won't install inux/capability.h unless you run "make headers" in
the kernel repo.
and they will either exist or disappear together. Now I want to remove
the dependency of the test on libcap library so I changed the code that it
does not contain sys/capability.h but instead linux/capability.h,
so that the test can compile successfully without libcap being installed,
these two syscalls are not declared in linux/capability.h(It is sufficient for
test use except for these two syscalls)
so we need to declare them here. I think that's why the commit 663af70aabb7
("bpf: selftests: Add helpers to directly use the capget and capset syscall") I
refered to
does the same thing. As for your question "What happens if sys/capability.h
along
with linux/capability.h", I haven't found the problem yet, I sincerely hope you
can
help me improve this code. Thank you very much.
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.
The change you are making isn't correct. Because you don't want to
define system calls locally in your source file.
thanks,
-- Shuah
Re: [PATCH] selftests: clone3: Use the capget and capset syscall directly
On 2024/10/11 22:21, Shuah Khan wrote:
On 10/11/24 00:59, zhouyuhang wrote:
On 2024/10/10 23:50, Shuah Khan wrote:
On 10/10/24 06:16, zhouyuhang wrote:
From: zhouyuhang
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.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.
NIT: grammar and comma spacing. Please fix those for readability.
e.g: Change "struct _cap_struct,it" to "struct _cap_struct, it"
Fix others as well.
Thanks, I'll fix it in V2
Signed-off-by: zhouyuhang
---
tools/testing/selftests/clone3/Makefile | 1 -
.../clone3/clone3_cap_checkpoint_restore.c | 60
+--
2 files changed, 28 insertions(+), 33 deletions(-)
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..111912e2aead 100644
--- a/tools/testing/selftests/clone3/clone3_cap_checkpoint_restore.c
+++ b/tools/testing/selftests/clone3/clone3_cap_checkpoint_restore.c
@@ -15,7 +15,7 @@
#include
#include
#include
-#include
+#include
#include
#include
#include
@@ -27,6 +27,13 @@
#include "../kselftest_harness.h"
#include "clone3_selftests.h"
+#ifndef CAP_CHECKPOINT_RESTORE
+#define CAP_CHECKPOINT_RESTORE 40
+#endif
+
Why is this necessary? This is defined in linux/capability.h.
+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);
In general prototypes such as these should be defined in header
file. Why are we defining these here?
These are defined in sys/capability.h
I don't understand this change. You are removing sys/capability.h
which requires you to add these defines here. This doesn't
sound like a correct solution to me.
I tested it on my machine without libcap-dev installed, the
/usr/include/linux/capability.h
is on this machine by default. Successfully compiled using #include
but not with #include . This patch removes libcap
library dependencies.
And we don't use any part of sys/capability.h other than these two
syscalls. So I think that's why it's necessary.
You are changing the code to not include sys/capability.h
What happens if sys/capability.h along with linux/capability.h
Do you see problems?
I'm sorry, maybe I wasn't clear enough.
When we install the libcap library it will have the following output:
test@test:~/work/libcap$ sudo make install | grep capability
install -m 0644 include/sys/capability.h /usr/include/sys
install -m 0644 include/sys/capability.h /usr/include/sys
/usr/share/man/man5 capability.conf.5 \
It installs sys/capability.h in the correct location, but does not
install linux/capability.h, so sys/capability.h is bound to the libcap
library
and they will either exist or disappear together. Now I want to remove
the dependency of the test on libcap library so I changed the code that it
does not contain sys/capability.h but instead linux/capability.h,
so that the test can compile successfully without libcap being installed,
these two syscalls are not declared in linux/capability.h(It is
sufficient for test use except for these two syscalls)
so we need to declare them here. I think that's why the commit 663af70aabb7
("bpf: selftests: Add helpers to directly use the capget and capset
syscall") I refered to
does the same thing. As for your question "What happens if
sys/capability.h along
with linux/capability.h", I haven't found the problem yet, I sincerely
hope you can
help me improve this code. Thank you very much.
thanks,
-- Shuah
Re: [PATCH] selftests: clone3: Use the capget and capset syscall directly
On 10/11/24 00:59, zhouyuhang wrote:
On 2024/10/10 23:50, Shuah Khan wrote:
On 10/10/24 06:16, zhouyuhang wrote:
From: zhouyuhang
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.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.
NIT: grammar and comma spacing. Please fix those for readability.
e.g: Change "struct _cap_struct,it" to "struct _cap_struct, it"
Fix others as well.
Thanks, I'll fix it in V2
Signed-off-by: zhouyuhang
---
tools/testing/selftests/clone3/Makefile | 1 -
.../clone3/clone3_cap_checkpoint_restore.c | 60 +--
2 files changed, 28 insertions(+), 33 deletions(-)
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..111912e2aead 100644
--- a/tools/testing/selftests/clone3/clone3_cap_checkpoint_restore.c
+++ b/tools/testing/selftests/clone3/clone3_cap_checkpoint_restore.c
@@ -15,7 +15,7 @@
#include
#include
#include
-#include
+#include
#include
#include
#include
@@ -27,6 +27,13 @@
#include "../kselftest_harness.h"
#include "clone3_selftests.h"
+#ifndef CAP_CHECKPOINT_RESTORE
+#define CAP_CHECKPOINT_RESTORE 40
+#endif
+
Why is this necessary? This is defined in linux/capability.h.
+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);
In general prototypes such as these should be defined in header
file. Why are we defining these here?
These are defined in sys/capability.h
I don't understand this change. You are removing sys/capability.h
which requires you to add these defines here. This doesn't
sound like a correct solution to me.
I tested it on my machine without libcap-dev installed, the
/usr/include/linux/capability.h
is on this machine by default. Successfully compiled using #include
but not with #include . This patch removes libcap library
dependencies.
And we don't use any part of sys/capability.h other than these two syscalls. So
I think that's why it's necessary.
You are changing the code to not include sys/capability.h
What happens if sys/capability.h along with linux/capability.h
Do you see problems?
thanks,
-- Shuah
Re: [PATCH] selftests: clone3: Use the capget and capset syscall directly
On 2024/10/10 23:50, Shuah Khan wrote:
On 10/10/24 06:16, zhouyuhang wrote:
From: zhouyuhang
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.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.
NIT: grammar and comma spacing. Please fix those for readability.
e.g: Change "struct _cap_struct,it" to "struct _cap_struct, it"
Fix others as well.
Thanks, I'll fix it in V2
Signed-off-by: zhouyuhang
---
tools/testing/selftests/clone3/Makefile | 1 -
.../clone3/clone3_cap_checkpoint_restore.c | 60 +--
2 files changed, 28 insertions(+), 33 deletions(-)
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..111912e2aead 100644
--- a/tools/testing/selftests/clone3/clone3_cap_checkpoint_restore.c
+++ b/tools/testing/selftests/clone3/clone3_cap_checkpoint_restore.c
@@ -15,7 +15,7 @@
#include
#include
#include
-#include
+#include
#include
#include
#include
@@ -27,6 +27,13 @@
#include "../kselftest_harness.h"
#include "clone3_selftests.h"
+#ifndef CAP_CHECKPOINT_RESTORE
+#define CAP_CHECKPOINT_RESTORE 40
+#endif
+
Why is this necessary? This is defined in linux/capability.h.
+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);
In general prototypes such as these should be defined in header
file. Why are we defining these here?
These are defined in sys/capability.h
I don't understand this change. You are removing sys/capability.h
which requires you to add these defines here. This doesn't
sound like a correct solution to me.
I tested it on my machine without libcap-dev installed, the
/usr/include/linux/capability.h
is on this machine by default. Successfully compiled using #include
but not with #include . This patch removes libcap
library dependencies.
And we don't use any part of sys/capability.h other than these two
syscalls. So I think that's why it's necessary.
+
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,
+ };
+ __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;
}
thanks,
-- Shuah
Re: [PATCH] selftests: clone3: Use the capget and capset syscall directly
On 10/10/24 06:16, zhouyuhang wrote:
From: zhouyuhang
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.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.
NIT: grammar and comma spacing. Please fix those for readability.
e.g: Change "struct _cap_struct,it" to "struct _cap_struct, it"
Fix others as well.
Signed-off-by: zhouyuhang
---
tools/testing/selftests/clone3/Makefile | 1 -
.../clone3/clone3_cap_checkpoint_restore.c| 60 +--
2 files changed, 28 insertions(+), 33 deletions(-)
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..111912e2aead 100644
--- a/tools/testing/selftests/clone3/clone3_cap_checkpoint_restore.c
+++ b/tools/testing/selftests/clone3/clone3_cap_checkpoint_restore.c
@@ -15,7 +15,7 @@
#include
#include
#include
-#include
+#include
#include
#include
#include
@@ -27,6 +27,13 @@
#include "../kselftest_harness.h"
#include "clone3_selftests.h"
+#ifndef CAP_CHECKPOINT_RESTORE
+#define CAP_CHECKPOINT_RESTORE 40
+#endif
+
Why is this necessary? This is defined in linux/capability.h.
+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);
In general prototypes such as these should be defined in header
file. Why are we defining these here?
These are defined in sys/capability.h
I don't understand this change. You are removing sys/capability.h
which requires you to add these defines here. This doesn't
sound like a correct solution to me.
+
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,
+ };
+ __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;
}
thanks,
-- Shuah
