* [PATCH bpf v5 1/3] bpf, cgroup: Reject prog_attach_flags array when effective query
2022-09-21 10:46 [PATCH bpf v5 0/3] Fix wrong cgroup attach flags being assigned to effective progs Pu Lehui
@ 2022-09-21 10:46 ` Pu Lehui
2022-09-21 10:46 ` [PATCH bpf v5 2/3] bpftool: Fix wrong cgroup attach flags being assigned to effective progs Pu Lehui
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Pu Lehui @ 2022-09-21 10:46 UTC (permalink / raw)
To: Martin KaFai Lau, Stanislav Fomichev, bpf, linux-kernel
Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Quentin Monnet, Song Liu, Yonghong Song, John Fastabend, KP Singh,
Hao Luo, Jiri Olsa, Pu Lehui, Pu Lehui
From: Pu Lehui <pulehui@huawei.com>
Attach flags is only valid for attached progs of this layer cgroup,
but not for effective progs. For querying with EFFECTIVE flags,
exporting attach flags does not make sense. So when effective query,
we reject prog_attach_flags array and don't need to populate it.
Also we limit attach_flags to output 0 during effective query.
Fixes: b79c9fc9551b ("bpf: implement BPF_PROG_QUERY for BPF_LSM_CGROUP")
Signed-off-by: Pu Lehui <pulehui@huawei.com>
---
include/uapi/linux/bpf.h | 7 +++++--
kernel/bpf/cgroup.c | 28 ++++++++++++++++++----------
tools/include/uapi/linux/bpf.h | 7 +++++--
3 files changed, 28 insertions(+), 14 deletions(-)
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 59a217ca2dfd..4eff7fc7ae58 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -1233,7 +1233,7 @@ enum {
/* Query effective (directly attached + inherited from ancestor cgroups)
* programs that will be executed for events within a cgroup.
- * attach_flags with this flag are returned only for directly attached programs.
+ * attach_flags with this flag are always returned 0.
*/
#define BPF_F_QUERY_EFFECTIVE (1U << 0)
@@ -1432,7 +1432,10 @@ union bpf_attr {
__u32 attach_flags;
__aligned_u64 prog_ids;
__u32 prog_cnt;
- __aligned_u64 prog_attach_flags; /* output: per-program attach_flags */
+ /* output: per-program attach_flags.
+ * not allowed to be set during effective query.
+ */
+ __aligned_u64 prog_attach_flags;
} query;
struct { /* anonymous struct used by BPF_RAW_TRACEPOINT_OPEN command */
diff --git a/kernel/bpf/cgroup.c b/kernel/bpf/cgroup.c
index 4a400cd63731..22888aaa68b6 100644
--- a/kernel/bpf/cgroup.c
+++ b/kernel/bpf/cgroup.c
@@ -1020,6 +1020,7 @@ static int __cgroup_bpf_query(struct cgroup *cgrp, const union bpf_attr *attr,
union bpf_attr __user *uattr)
{
__u32 __user *prog_attach_flags = u64_to_user_ptr(attr->query.prog_attach_flags);
+ bool effective_query = attr->query.query_flags & BPF_F_QUERY_EFFECTIVE;
__u32 __user *prog_ids = u64_to_user_ptr(attr->query.prog_ids);
enum bpf_attach_type type = attr->query.attach_type;
enum cgroup_bpf_attach_type from_atype, to_atype;
@@ -1029,8 +1030,12 @@ static int __cgroup_bpf_query(struct cgroup *cgrp, const union bpf_attr *attr,
int total_cnt = 0;
u32 flags;
+ if (effective_query && prog_attach_flags)
+ return -EINVAL;
+
if (type == BPF_LSM_CGROUP) {
- if (attr->query.prog_cnt && prog_ids && !prog_attach_flags)
+ if (!effective_query && attr->query.prog_cnt &&
+ prog_ids && !prog_attach_flags)
return -EINVAL;
from_atype = CGROUP_LSM_START;
@@ -1045,7 +1050,7 @@ static int __cgroup_bpf_query(struct cgroup *cgrp, const union bpf_attr *attr,
}
for (atype = from_atype; atype <= to_atype; atype++) {
- if (attr->query.query_flags & BPF_F_QUERY_EFFECTIVE) {
+ if (effective_query) {
effective = rcu_dereference_protected(cgrp->bpf.effective[atype],
lockdep_is_held(&cgroup_mutex));
total_cnt += bpf_prog_array_length(effective);
@@ -1054,6 +1059,8 @@ static int __cgroup_bpf_query(struct cgroup *cgrp, const union bpf_attr *attr,
}
}
+ /* always output uattr->query.attach_flags as 0 during effective query */
+ flags = effective_query ? 0 : flags;
if (copy_to_user(&uattr->query.attach_flags, &flags, sizeof(flags)))
return -EFAULT;
if (copy_to_user(&uattr->query.prog_cnt, &total_cnt, sizeof(total_cnt)))
@@ -1068,7 +1075,7 @@ static int __cgroup_bpf_query(struct cgroup *cgrp, const union bpf_attr *attr,
}
for (atype = from_atype; atype <= to_atype && total_cnt; atype++) {
- if (attr->query.query_flags & BPF_F_QUERY_EFFECTIVE) {
+ if (effective_query) {
effective = rcu_dereference_protected(cgrp->bpf.effective[atype],
lockdep_is_held(&cgroup_mutex));
cnt = min_t(int, bpf_prog_array_length(effective), total_cnt);
@@ -1090,15 +1097,16 @@ static int __cgroup_bpf_query(struct cgroup *cgrp, const union bpf_attr *attr,
if (++i == cnt)
break;
}
- }
- if (prog_attach_flags) {
- flags = cgrp->bpf.flags[atype];
+ if (prog_attach_flags) {
+ flags = cgrp->bpf.flags[atype];
- for (i = 0; i < cnt; i++)
- if (copy_to_user(prog_attach_flags + i, &flags, sizeof(flags)))
- return -EFAULT;
- prog_attach_flags += cnt;
+ for (i = 0; i < cnt; i++)
+ if (copy_to_user(prog_attach_flags + i,
+ &flags, sizeof(flags)))
+ return -EFAULT;
+ prog_attach_flags += cnt;
+ }
}
prog_ids += cnt;
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index 59a217ca2dfd..4eff7fc7ae58 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -1233,7 +1233,7 @@ enum {
/* Query effective (directly attached + inherited from ancestor cgroups)
* programs that will be executed for events within a cgroup.
- * attach_flags with this flag are returned only for directly attached programs.
+ * attach_flags with this flag are always returned 0.
*/
#define BPF_F_QUERY_EFFECTIVE (1U << 0)
@@ -1432,7 +1432,10 @@ union bpf_attr {
__u32 attach_flags;
__aligned_u64 prog_ids;
__u32 prog_cnt;
- __aligned_u64 prog_attach_flags; /* output: per-program attach_flags */
+ /* output: per-program attach_flags.
+ * not allowed to be set during effective query.
+ */
+ __aligned_u64 prog_attach_flags;
} query;
struct { /* anonymous struct used by BPF_RAW_TRACEPOINT_OPEN command */
--
2.25.1
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH bpf v5 2/3] bpftool: Fix wrong cgroup attach flags being assigned to effective progs
2022-09-21 10:46 [PATCH bpf v5 0/3] Fix wrong cgroup attach flags being assigned to effective progs Pu Lehui
2022-09-21 10:46 ` [PATCH bpf v5 1/3] bpf, cgroup: Reject prog_attach_flags array when effective query Pu Lehui
@ 2022-09-21 10:46 ` Pu Lehui
2022-09-21 10:46 ` [PATCH bpf v5 3/3] selftests/bpf: Adapt cgroup effective query uapi change Pu Lehui
2022-09-21 18:30 ` [PATCH bpf v5 0/3] Fix wrong cgroup attach flags being assigned to effective progs patchwork-bot+netdevbpf
3 siblings, 0 replies; 5+ messages in thread
From: Pu Lehui @ 2022-09-21 10:46 UTC (permalink / raw)
To: Martin KaFai Lau, Stanislav Fomichev, bpf, linux-kernel
Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Quentin Monnet, Song Liu, Yonghong Song, John Fastabend, KP Singh,
Hao Luo, Jiri Olsa, Pu Lehui, Pu Lehui
From: Pu Lehui <pulehui@huawei.com>
When root-cgroup attach multi progs and sub-cgroup attach a override prog,
bpftool will display incorrectly for the attach flags of the sub-cgroup’s
effective progs:
$ bpftool cgroup tree /sys/fs/cgroup effective
CgroupPath
ID AttachType AttachFlags Name
/sys/fs/cgroup
6 cgroup_sysctl multi sysctl_tcp_mem
13 cgroup_sysctl multi sysctl_tcp_mem
/sys/fs/cgroup/cg1
20 cgroup_sysctl override sysctl_tcp_mem
6 cgroup_sysctl override sysctl_tcp_mem <- wrong
13 cgroup_sysctl override sysctl_tcp_mem <- wrong
/sys/fs/cgroup/cg1/cg2
20 cgroup_sysctl sysctl_tcp_mem
6 cgroup_sysctl sysctl_tcp_mem
13 cgroup_sysctl sysctl_tcp_mem
Attach flags is only valid for attached progs of this layer cgroup,
but not for effective progs. For querying with EFFECTIVE flags,
exporting attach flags does not make sense. So let's remove the
AttachFlags field and the associated logic. After this patch, the
above effective cgroup tree will show as bellow:
$ bpftool cgroup tree /sys/fs/cgroup effective
CgroupPath
ID AttachType Name
/sys/fs/cgroup
6 cgroup_sysctl sysctl_tcp_mem
13 cgroup_sysctl sysctl_tcp_mem
/sys/fs/cgroup/cg1
20 cgroup_sysctl sysctl_tcp_mem
6 cgroup_sysctl sysctl_tcp_mem
13 cgroup_sysctl sysctl_tcp_mem
/sys/fs/cgroup/cg1/cg2
20 cgroup_sysctl sysctl_tcp_mem
6 cgroup_sysctl sysctl_tcp_mem
13 cgroup_sysctl sysctl_tcp_mem
Fixes: b79c9fc9551b ("bpf: implement BPF_PROG_QUERY for BPF_LSM_CGROUP")
Fixes: a98bf57391a2 ("tools: bpftool: add support for reporting the effective cgroup progs")
Signed-off-by: Pu Lehui <pulehui@huawei.com>
---
tools/bpf/bpftool/cgroup.c | 54 ++++++++++++++++++++++++++++++++++----
1 file changed, 49 insertions(+), 5 deletions(-)
diff --git a/tools/bpf/bpftool/cgroup.c b/tools/bpf/bpftool/cgroup.c
index cced668fb2a3..b46a998d8f8d 100644
--- a/tools/bpf/bpftool/cgroup.c
+++ b/tools/bpf/bpftool/cgroup.c
@@ -136,8 +136,8 @@ static int show_bpf_prog(int id, enum bpf_attach_type attach_type,
jsonw_string_field(json_wtr, "attach_type", attach_type_str);
else
jsonw_uint_field(json_wtr, "attach_type", attach_type);
- jsonw_string_field(json_wtr, "attach_flags",
- attach_flags_str);
+ if (!(query_flags & BPF_F_QUERY_EFFECTIVE))
+ jsonw_string_field(json_wtr, "attach_flags", attach_flags_str);
jsonw_string_field(json_wtr, "name", prog_name);
if (attach_btf_name)
jsonw_string_field(json_wtr, "attach_btf_name", attach_btf_name);
@@ -150,7 +150,10 @@ static int show_bpf_prog(int id, enum bpf_attach_type attach_type,
printf("%-15s", attach_type_str);
else
printf("type %-10u", attach_type);
- printf(" %-15s %-15s", attach_flags_str, prog_name);
+ if (query_flags & BPF_F_QUERY_EFFECTIVE)
+ printf(" %-15s", prog_name);
+ else
+ printf(" %-15s %-15s", attach_flags_str, prog_name);
if (attach_btf_name)
printf(" %-15s", attach_btf_name);
else if (info.attach_btf_id)
@@ -195,6 +198,32 @@ static int cgroup_has_attached_progs(int cgroup_fd)
return no_prog ? 0 : 1;
}
+
+static int show_effective_bpf_progs(int cgroup_fd, enum bpf_attach_type type,
+ int level)
+{
+ LIBBPF_OPTS(bpf_prog_query_opts, p);
+ __u32 prog_ids[1024] = {0};
+ __u32 iter;
+ int ret;
+
+ p.query_flags = query_flags;
+ p.prog_cnt = ARRAY_SIZE(prog_ids);
+ p.prog_ids = prog_ids;
+
+ ret = bpf_prog_query_opts(cgroup_fd, type, &p);
+ if (ret)
+ return ret;
+
+ if (p.prog_cnt == 0)
+ return 0;
+
+ for (iter = 0; iter < p.prog_cnt; iter++)
+ show_bpf_prog(prog_ids[iter], type, NULL, level);
+
+ return 0;
+}
+
static int show_attached_bpf_progs(int cgroup_fd, enum bpf_attach_type type,
int level)
{
@@ -245,6 +274,14 @@ static int show_attached_bpf_progs(int cgroup_fd, enum bpf_attach_type type,
return 0;
}
+static int show_bpf_progs(int cgroup_fd, enum bpf_attach_type type,
+ int level)
+{
+ return query_flags & BPF_F_QUERY_EFFECTIVE ?
+ show_effective_bpf_progs(cgroup_fd, type, level) :
+ show_attached_bpf_progs(cgroup_fd, type, level);
+}
+
static int do_show(int argc, char **argv)
{
enum bpf_attach_type type;
@@ -292,6 +329,8 @@ static int do_show(int argc, char **argv)
if (json_output)
jsonw_start_array(json_wtr);
+ else if (query_flags & BPF_F_QUERY_EFFECTIVE)
+ printf("%-8s %-15s %-15s\n", "ID", "AttachType", "Name");
else
printf("%-8s %-15s %-15s %-15s\n", "ID", "AttachType",
"AttachFlags", "Name");
@@ -304,7 +343,7 @@ static int do_show(int argc, char **argv)
* If we were able to get the show for at least one
* attach type, let's return 0.
*/
- if (show_attached_bpf_progs(cgroup_fd, type, 0) == 0)
+ if (show_bpf_progs(cgroup_fd, type, 0) == 0)
ret = 0;
}
@@ -362,7 +401,7 @@ static int do_show_tree_fn(const char *fpath, const struct stat *sb,
btf_vmlinux = libbpf_find_kernel_btf();
for (type = 0; type < __MAX_BPF_ATTACH_TYPE; type++)
- show_attached_bpf_progs(cgroup_fd, type, ftw->level);
+ show_bpf_progs(cgroup_fd, type, ftw->level);
if (errno == EINVAL)
/* Last attach type does not support query.
@@ -436,6 +475,11 @@ static int do_show_tree(int argc, char **argv)
if (json_output)
jsonw_start_array(json_wtr);
+ else if (query_flags & BPF_F_QUERY_EFFECTIVE)
+ printf("%s\n"
+ "%-8s %-15s %-15s\n",
+ "CgroupPath",
+ "ID", "AttachType", "Name");
else
printf("%s\n"
"%-8s %-15s %-15s %-15s\n",
--
2.25.1
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH bpf v5 3/3] selftests/bpf: Adapt cgroup effective query uapi change
2022-09-21 10:46 [PATCH bpf v5 0/3] Fix wrong cgroup attach flags being assigned to effective progs Pu Lehui
2022-09-21 10:46 ` [PATCH bpf v5 1/3] bpf, cgroup: Reject prog_attach_flags array when effective query Pu Lehui
2022-09-21 10:46 ` [PATCH bpf v5 2/3] bpftool: Fix wrong cgroup attach flags being assigned to effective progs Pu Lehui
@ 2022-09-21 10:46 ` Pu Lehui
2022-09-21 18:30 ` [PATCH bpf v5 0/3] Fix wrong cgroup attach flags being assigned to effective progs patchwork-bot+netdevbpf
3 siblings, 0 replies; 5+ messages in thread
From: Pu Lehui @ 2022-09-21 10:46 UTC (permalink / raw)
To: Martin KaFai Lau, Stanislav Fomichev, bpf, linux-kernel
Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Quentin Monnet, Song Liu, Yonghong Song, John Fastabend, KP Singh,
Hao Luo, Jiri Olsa, Pu Lehui, Pu Lehui
From: Pu Lehui <pulehui@huawei.com>
The attach flags is meaningless for effective query and
its value will always be set as 0 during effective query.
Root cg's effective progs is always its attached progs,
so we use non-effective query to get its progs count and
attach flags. And we don't need the remain attach flags
check.
Fixes: b79c9fc9551b ("bpf: implement BPF_PROG_QUERY for BPF_LSM_CGROUP")
Signed-off-by: Pu Lehui <pulehui@huawei.com>
---
tools/testing/selftests/bpf/prog_tests/cgroup_link.c | 11 ++++-------
1 file changed, 4 insertions(+), 7 deletions(-)
diff --git a/tools/testing/selftests/bpf/prog_tests/cgroup_link.c b/tools/testing/selftests/bpf/prog_tests/cgroup_link.c
index 9e6e6aad347c..15093a69510e 100644
--- a/tools/testing/selftests/bpf/prog_tests/cgroup_link.c
+++ b/tools/testing/selftests/bpf/prog_tests/cgroup_link.c
@@ -71,10 +71,9 @@ void serial_test_cgroup_link(void)
ping_and_check(cg_nr, 0);
- /* query the number of effective progs and attach flags in root cg */
+ /* query the number of attached progs and attach flags in root cg */
err = bpf_prog_query(cgs[0].fd, BPF_CGROUP_INET_EGRESS,
- BPF_F_QUERY_EFFECTIVE, &attach_flags, NULL,
- &prog_cnt);
+ 0, &attach_flags, NULL, &prog_cnt);
CHECK_FAIL(err);
CHECK_FAIL(attach_flags != BPF_F_ALLOW_MULTI);
if (CHECK(prog_cnt != 1, "effect_cnt", "exp %d, got %d\n", 1, prog_cnt))
@@ -85,17 +84,15 @@ void serial_test_cgroup_link(void)
BPF_F_QUERY_EFFECTIVE, NULL, NULL,
&prog_cnt);
CHECK_FAIL(err);
- CHECK_FAIL(attach_flags != BPF_F_ALLOW_MULTI);
if (CHECK(prog_cnt != cg_nr, "effect_cnt", "exp %d, got %d\n",
cg_nr, prog_cnt))
goto cleanup;
/* query the effective prog IDs in last cg */
err = bpf_prog_query(cgs[last_cg].fd, BPF_CGROUP_INET_EGRESS,
- BPF_F_QUERY_EFFECTIVE, &attach_flags,
- prog_ids, &prog_cnt);
+ BPF_F_QUERY_EFFECTIVE, NULL, prog_ids,
+ &prog_cnt);
CHECK_FAIL(err);
- CHECK_FAIL(attach_flags != BPF_F_ALLOW_MULTI);
if (CHECK(prog_cnt != cg_nr, "effect_cnt", "exp %d, got %d\n",
cg_nr, prog_cnt))
goto cleanup;
--
2.25.1
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH bpf v5 0/3] Fix wrong cgroup attach flags being assigned to effective progs
2022-09-21 10:46 [PATCH bpf v5 0/3] Fix wrong cgroup attach flags being assigned to effective progs Pu Lehui
` (2 preceding siblings ...)
2022-09-21 10:46 ` [PATCH bpf v5 3/3] selftests/bpf: Adapt cgroup effective query uapi change Pu Lehui
@ 2022-09-21 18:30 ` patchwork-bot+netdevbpf
3 siblings, 0 replies; 5+ messages in thread
From: patchwork-bot+netdevbpf @ 2022-09-21 18:30 UTC (permalink / raw)
To: Pu Lehui
Cc: martin.lau, sdf, bpf, linux-kernel, ast, daniel, andrii, quentin,
song, yhs, john.fastabend, kpsingh, haoluo, jolsa, pulehui
Hello:
This series was applied to bpf/bpf.git (master)
by Martin KaFai Lau <martin.lau@kernel.org>:
On Wed, 21 Sep 2022 10:46:01 +0000 you wrote:
> From: Pu Lehui <pulehui@huawei.com>
>
> When root-cgroup attach multi progs and sub-cgroup attach a
> override prog, bpftool will display incorrectly for the attach
> flags of the sub-cgroup’s effective progs:
>
> $ bpftool cgroup tree /sys/fs/cgroup effective
> CgroupPath
> ID AttachType AttachFlags Name
> /sys/fs/cgroup
> 6 cgroup_sysctl multi sysctl_tcp_mem
> 13 cgroup_sysctl multi sysctl_tcp_mem
> /sys/fs/cgroup/cg1
> 20 cgroup_sysctl override sysctl_tcp_mem
> 6 cgroup_sysctl override sysctl_tcp_mem <- wrong
> 13 cgroup_sysctl override sysctl_tcp_mem <- wrong
> /sys/fs/cgroup/cg1/cg2
> 20 cgroup_sysctl sysctl_tcp_mem
> 6 cgroup_sysctl sysctl_tcp_mem
> 13 cgroup_sysctl sysctl_tcp_mem
>
> [...]
Here is the summary with links:
- [bpf,v5,1/3] bpf, cgroup: Reject prog_attach_flags array when effective query
https://git.kernel.org/bpf/bpf/c/0e426a3ae030
- [bpf,v5,2/3] bpftool: Fix wrong cgroup attach flags being assigned to effective progs
https://git.kernel.org/bpf/bpf/c/bdcee1b0b083
- [bpf,v5,3/3] selftests/bpf: Adapt cgroup effective query uapi change
https://git.kernel.org/bpf/bpf/c/d2aa993b7d9d
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 5+ messages in thread