| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <806bee31-d740-49c9-abe0-06820cfa7395@linuxfoundation.org>
Date: Tue, 15 Oct 2024 09:31:11 -0600
From: Shuah Khan <skhan@...uxfoundation.org>
To: zhouyuhang <zhouyuhang1010@....com>, brauner@...nel.org, shuah@...nel.org
Cc: linux-kernel@...r.kernel.org, linux-kselftest@...r.kernel.org,
zhouyuhang <zhouyuhang@...inos.cn>, Shuah Khan <skhan@...uxfoundation.org>
Subject: Re: [PATCH v2] selftests: clone3: Use the capget and capset syscall
directly
On 10/15/24 04:59, zhouyuhang wrote:
> From: zhouyuhang <zhouyuhang@...inos.cn>
>
> 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@...inos.cn>
> ---
> 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.
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
Powered by blists - more mailing lists