* [PATCH bpf-next v3 0/5] bpf: expose bpf_{g,s}et_retval to more cgroup hooks
@ 2022-08-18 23:27 Stanislav Fomichev
2022-08-18 23:27 ` [PATCH bpf-next v3 1/5] bpf: Introduce cgroup_{common,current}_func_proto Stanislav Fomichev
` (4 more replies)
0 siblings, 5 replies; 12+ messages in thread
From: Stanislav Fomichev @ 2022-08-18 23:27 UTC (permalink / raw)
To: bpf
Cc: ast, daniel, andrii, martin.lau, song, yhs, john.fastabend,
kpsingh, sdf, haoluo, jolsa
Apparently, only a small subset of cgroup hooks actually falls
back to cgroup_base_func_proto. This leads to unexpected result
where not all cgroup helpers have bpf_{g,s}et_retval.
It's getting harder and harder to manage which helpers are exported
to which hooks. We now have the following call chains:
- cg_skb_func_proto
- sk_filter_func_proto
- bpf_sk_base_func_proto
- bpf_base_func_proto
So by looking at cg_skb_func_proto it's pretty hard to understand
what's going on.
For cgroup helpers, I'm proposing we do the following instead:
func_proto = cgroup_common_func_proto();
if (func_proto) return func_proto;
/* optional, if hook has 'current' */
func_proto = cgroup_current_func_proto();
if (func_proto) return func_proto;
...
switch (func_id) {
/* hook specific helpers */
case BPF_FUNC_hook_specific_helper:
return &xyz;
default:
/* always fall back to plain bpf_base_func_proto */
bpf_base_func_proto(func_id);
}
If this turns out more workable, we can follow up with converting
the rest to the same pattern.
v3:
- expose strtol/strtoul everywhere (Martin)
- move helpers declarations from bpf.h to bpf-cgroup.h (Martin)
- revise bpf_{g,s}et_retval documentation (Martin)
- don't expose bpf_{g,s}et_retval to cg_skb hooks (Martin)
v2:
- move everything into kernel/bpf/cgroup.c instead (Martin)
- use cgroup_common_func_proto in lsm (Martin)
Stanislav Fomichev (5):
bpf: Introduce cgroup_{common,current}_func_proto
bpf: Use cgroup_{common,current}_func_proto in more hooks
bpf: expose bpf_strtol and bpf_strtoul to all program types
bpf: update bpf_{g,s}et_retval documentation
selftests/bpf: Make sure bpf_{g,s}et_retval is exposed everywhere
include/linux/bpf-cgroup.h | 17 ++
include/uapi/linux/bpf.h | 22 +-
kernel/bpf/bpf_lsm.c | 19 +-
kernel/bpf/cgroup.c | 212 ++++++++++++++++--
kernel/bpf/helpers.c | 82 +------
net/core/filter.c | 92 +++-----
tools/include/uapi/linux/bpf.h | 22 +-
tools/testing/selftests/bpf/Makefile | 1 +
.../bpf/cgroup_getset_retval_hooks.h | 25 +++
.../bpf/prog_tests/cgroup_getset_retval.c | 48 ++++
.../bpf/progs/cgroup_getset_retval_hooks.c | 16 ++
11 files changed, 380 insertions(+), 176 deletions(-)
create mode 100644 tools/testing/selftests/bpf/cgroup_getset_retval_hooks.h
create mode 100644 tools/testing/selftests/bpf/progs/cgroup_getset_retval_hooks.c
--
2.37.1.595.g718a3a8f04-goog
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH bpf-next v3 1/5] bpf: Introduce cgroup_{common,current}_func_proto
2022-08-18 23:27 [PATCH bpf-next v3 0/5] bpf: expose bpf_{g,s}et_retval to more cgroup hooks Stanislav Fomichev
@ 2022-08-18 23:27 ` Stanislav Fomichev
2022-08-19 18:17 ` Martin KaFai Lau
2022-08-18 23:27 ` [PATCH bpf-next v3 2/5] bpf: Use cgroup_{common,current}_func_proto in more hooks Stanislav Fomichev
` (3 subsequent siblings)
4 siblings, 1 reply; 12+ messages in thread
From: Stanislav Fomichev @ 2022-08-18 23:27 UTC (permalink / raw)
To: bpf
Cc: ast, daniel, andrii, martin.lau, song, yhs, john.fastabend,
kpsingh, sdf, haoluo, jolsa
Split cgroup_base_func_proto into the following:
* cgroup_common_func_proto - common helpers for all cgroup hooks
* cgroup_current_func_proto - common helpers for all cgroup hooks
running in the process context (== have meaningful 'current').
Move bpf_{g,s}et_retval and other cgroup-related helpers into
kernel/bpf/cgroup.c so they closer to where they are being used.
Signed-off-by: Stanislav Fomichev <sdf@google.com>
---
include/linux/bpf-cgroup.h | 17 ++++
kernel/bpf/cgroup.c | 172 +++++++++++++++++++++++++++++++++----
kernel/bpf/helpers.c | 77 -----------------
net/core/filter.c | 14 +--
4 files changed, 173 insertions(+), 107 deletions(-)
diff --git a/include/linux/bpf-cgroup.h b/include/linux/bpf-cgroup.h
index 2bd1b5f8de9b..57e9e109257e 100644
--- a/include/linux/bpf-cgroup.h
+++ b/include/linux/bpf-cgroup.h
@@ -414,6 +414,11 @@ int cgroup_bpf_prog_detach(const union bpf_attr *attr,
int cgroup_bpf_link_attach(const union bpf_attr *attr, struct bpf_prog *prog);
int cgroup_bpf_prog_query(const union bpf_attr *attr,
union bpf_attr __user *uattr);
+
+const struct bpf_func_proto *
+cgroup_common_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog);
+const struct bpf_func_proto *
+cgroup_current_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog);
#else
static inline int cgroup_bpf_inherit(struct cgroup *cgrp) { return 0; }
@@ -444,6 +449,18 @@ static inline int cgroup_bpf_prog_query(const union bpf_attr *attr,
return -EINVAL;
}
+static inline const struct bpf_func_proto *
+cgroup_common_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
+{
+ return NULL;
+}
+
+static inline const struct bpf_func_proto *
+cgroup_current_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
+{
+ return NULL;
+}
+
static inline int bpf_cgroup_storage_assign(struct bpf_prog_aux *aux,
struct bpf_map *map) { return 0; }
static inline struct bpf_cgroup_storage *bpf_cgroup_storage_alloc(
diff --git a/kernel/bpf/cgroup.c b/kernel/bpf/cgroup.c
index 59b7eb60d5b4..74b5cc692a36 100644
--- a/kernel/bpf/cgroup.c
+++ b/kernel/bpf/cgroup.c
@@ -18,6 +18,7 @@
#include <linux/bpf_verifier.h>
#include <net/sock.h>
#include <net/bpf_sk_storage.h>
+#include <net/cls_cgroup.h>
#include "../cgroup/cgroup-internal.h"
@@ -1527,6 +1528,78 @@ int __cgroup_bpf_check_dev_permission(short dev_type, u32 major, u32 minor,
return ret;
}
+BPF_CALL_0(bpf_get_current_cgroup_id)
+{
+ struct cgroup *cgrp;
+ u64 cgrp_id;
+
+ rcu_read_lock();
+ cgrp = task_dfl_cgroup(current);
+ cgrp_id = cgroup_id(cgrp);
+ rcu_read_unlock();
+
+ return cgrp_id;
+}
+
+const struct bpf_func_proto bpf_get_current_cgroup_id_proto = {
+ .func = bpf_get_current_cgroup_id,
+ .gpl_only = false,
+ .ret_type = RET_INTEGER,
+};
+
+BPF_CALL_1(bpf_get_current_ancestor_cgroup_id, int, ancestor_level)
+{
+ struct cgroup *cgrp;
+ struct cgroup *ancestor;
+ u64 cgrp_id;
+
+ rcu_read_lock();
+ cgrp = task_dfl_cgroup(current);
+ ancestor = cgroup_ancestor(cgrp, ancestor_level);
+ cgrp_id = ancestor ? cgroup_id(ancestor) : 0;
+ rcu_read_unlock();
+
+ return cgrp_id;
+}
+
+const struct bpf_func_proto bpf_get_current_ancestor_cgroup_id_proto = {
+ .func = bpf_get_current_ancestor_cgroup_id,
+ .gpl_only = false,
+ .ret_type = RET_INTEGER,
+ .arg1_type = ARG_ANYTHING,
+};
+
+BPF_CALL_2(bpf_get_local_storage, struct bpf_map *, map, u64, flags)
+{
+ /* flags argument is not used now,
+ * but provides an ability to extend the API.
+ * verifier checks that its value is correct.
+ */
+ enum bpf_cgroup_storage_type stype = cgroup_storage_type(map);
+ struct bpf_cgroup_storage *storage;
+ struct bpf_cg_run_ctx *ctx;
+ void *ptr;
+
+ /* get current cgroup storage from BPF run context */
+ ctx = container_of(current->bpf_ctx, struct bpf_cg_run_ctx, run_ctx);
+ storage = ctx->prog_item->cgroup_storage[stype];
+
+ if (stype == BPF_CGROUP_STORAGE_SHARED)
+ ptr = &READ_ONCE(storage->buf)->data[0];
+ else
+ ptr = this_cpu_ptr(storage->percpu_buf);
+
+ return (unsigned long)ptr;
+}
+
+const struct bpf_func_proto bpf_get_local_storage_proto = {
+ .func = bpf_get_local_storage,
+ .gpl_only = false,
+ .ret_type = RET_PTR_TO_MAP_VALUE,
+ .arg1_type = ARG_CONST_MAP_PTR,
+ .arg2_type = ARG_ANYTHING,
+};
+
BPF_CALL_0(bpf_get_retval)
{
struct bpf_cg_run_ctx *ctx =
@@ -1557,33 +1630,40 @@ const struct bpf_func_proto bpf_set_retval_proto = {
.arg1_type = ARG_ANYTHING,
};
+#ifdef CONFIG_CGROUP_NET_CLASSID
+BPF_CALL_0(bpf_get_cgroup_classid_curr)
+{
+ return __task_get_classid(current);
+}
+
+const struct bpf_func_proto bpf_get_cgroup_classid_curr_proto = {
+ .func = bpf_get_cgroup_classid_curr,
+ .gpl_only = false,
+ .ret_type = RET_INTEGER,
+};
+#endif
+
static const struct bpf_func_proto *
-cgroup_base_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
+cgroup_dev_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
{
+ const struct bpf_func_proto *func_proto;
+
+ func_proto = cgroup_common_func_proto(func_id, prog);
+ if (func_proto)
+ return func_proto;
+
+ func_proto = cgroup_current_func_proto(func_id, prog);
+ if (func_proto)
+ return func_proto;
+
switch (func_id) {
- case BPF_FUNC_get_current_uid_gid:
- return &bpf_get_current_uid_gid_proto;
- case BPF_FUNC_get_local_storage:
- return &bpf_get_local_storage_proto;
- case BPF_FUNC_get_current_cgroup_id:
- return &bpf_get_current_cgroup_id_proto;
case BPF_FUNC_perf_event_output:
return &bpf_event_output_data_proto;
- case BPF_FUNC_get_retval:
- return &bpf_get_retval_proto;
- case BPF_FUNC_set_retval:
- return &bpf_set_retval_proto;
default:
return bpf_base_func_proto(func_id);
}
}
-static const struct bpf_func_proto *
-cgroup_dev_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
-{
- return cgroup_base_func_proto(func_id, prog);
-}
-
static bool cgroup_dev_is_valid_access(int off, int size,
enum bpf_access_type type,
const struct bpf_prog *prog,
@@ -2096,6 +2176,16 @@ static const struct bpf_func_proto bpf_sysctl_set_new_value_proto = {
static const struct bpf_func_proto *
sysctl_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
{
+ const struct bpf_func_proto *func_proto;
+
+ func_proto = cgroup_common_func_proto(func_id, prog);
+ if (func_proto)
+ return func_proto;
+
+ func_proto = cgroup_current_func_proto(func_id, prog);
+ if (func_proto)
+ return func_proto;
+
switch (func_id) {
case BPF_FUNC_strtol:
return &bpf_strtol_proto;
@@ -2111,8 +2201,10 @@ sysctl_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
return &bpf_sysctl_set_new_value_proto;
case BPF_FUNC_ktime_get_coarse_ns:
return &bpf_ktime_get_coarse_ns_proto;
+ case BPF_FUNC_perf_event_output:
+ return &bpf_event_output_data_proto;
default:
- return cgroup_base_func_proto(func_id, prog);
+ return bpf_base_func_proto(func_id);
}
}
@@ -2233,6 +2325,16 @@ static const struct bpf_func_proto bpf_get_netns_cookie_sockopt_proto = {
static const struct bpf_func_proto *
cg_sockopt_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
{
+ const struct bpf_func_proto *func_proto;
+
+ func_proto = cgroup_common_func_proto(func_id, prog);
+ if (func_proto)
+ return func_proto;
+
+ func_proto = cgroup_current_func_proto(func_id, prog);
+ if (func_proto)
+ return func_proto;
+
switch (func_id) {
#ifdef CONFIG_NET
case BPF_FUNC_get_netns_cookie:
@@ -2254,8 +2356,10 @@ cg_sockopt_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
case BPF_FUNC_tcp_sock:
return &bpf_tcp_sock_proto;
#endif
+ case BPF_FUNC_perf_event_output:
+ return &bpf_event_output_data_proto;
default:
- return cgroup_base_func_proto(func_id, prog);
+ return bpf_base_func_proto(func_id);
}
}
@@ -2420,3 +2524,33 @@ const struct bpf_verifier_ops cg_sockopt_verifier_ops = {
const struct bpf_prog_ops cg_sockopt_prog_ops = {
};
+
+/* Common helpers for cgroup hooks. */
+const struct bpf_func_proto *
+cgroup_common_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
+{
+ switch (func_id) {
+ case BPF_FUNC_get_local_storage:
+ return &bpf_get_local_storage_proto;
+ case BPF_FUNC_get_retval:
+ return &bpf_get_retval_proto;
+ case BPF_FUNC_set_retval:
+ return &bpf_set_retval_proto;
+ default:
+ return NULL;
+ }
+}
+
+/* Common helpers for cgroup hooks with valid process context. */
+const struct bpf_func_proto *
+cgroup_current_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
+{
+ switch (func_id) {
+ case BPF_FUNC_get_current_uid_gid:
+ return &bpf_get_current_uid_gid_proto;
+ case BPF_FUNC_get_current_cgroup_id:
+ return &bpf_get_current_cgroup_id_proto;
+ default:
+ return NULL;
+ }
+}
diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
index 3c1b9bbcf971..71979d870646 100644
--- a/kernel/bpf/helpers.c
+++ b/kernel/bpf/helpers.c
@@ -386,82 +386,6 @@ const struct bpf_func_proto bpf_jiffies64_proto = {
.ret_type = RET_INTEGER,
};
-#ifdef CONFIG_CGROUPS
-BPF_CALL_0(bpf_get_current_cgroup_id)
-{
- struct cgroup *cgrp;
- u64 cgrp_id;
-
- rcu_read_lock();
- cgrp = task_dfl_cgroup(current);
- cgrp_id = cgroup_id(cgrp);
- rcu_read_unlock();
-
- return cgrp_id;
-}
-
-const struct bpf_func_proto bpf_get_current_cgroup_id_proto = {
- .func = bpf_get_current_cgroup_id,
- .gpl_only = false,
- .ret_type = RET_INTEGER,
-};
-
-BPF_CALL_1(bpf_get_current_ancestor_cgroup_id, int, ancestor_level)
-{
- struct cgroup *cgrp;
- struct cgroup *ancestor;
- u64 cgrp_id;
-
- rcu_read_lock();
- cgrp = task_dfl_cgroup(current);
- ancestor = cgroup_ancestor(cgrp, ancestor_level);
- cgrp_id = ancestor ? cgroup_id(ancestor) : 0;
- rcu_read_unlock();
-
- return cgrp_id;
-}
-
-const struct bpf_func_proto bpf_get_current_ancestor_cgroup_id_proto = {
- .func = bpf_get_current_ancestor_cgroup_id,
- .gpl_only = false,
- .ret_type = RET_INTEGER,
- .arg1_type = ARG_ANYTHING,
-};
-
-#ifdef CONFIG_CGROUP_BPF
-
-BPF_CALL_2(bpf_get_local_storage, struct bpf_map *, map, u64, flags)
-{
- /* flags argument is not used now,
- * but provides an ability to extend the API.
- * verifier checks that its value is correct.
- */
- enum bpf_cgroup_storage_type stype = cgroup_storage_type(map);
- struct bpf_cgroup_storage *storage;
- struct bpf_cg_run_ctx *ctx;
- void *ptr;
-
- /* get current cgroup storage from BPF run context */
- ctx = container_of(current->bpf_ctx, struct bpf_cg_run_ctx, run_ctx);
- storage = ctx->prog_item->cgroup_storage[stype];
-
- if (stype == BPF_CGROUP_STORAGE_SHARED)
- ptr = &READ_ONCE(storage->buf)->data[0];
- else
- ptr = this_cpu_ptr(storage->percpu_buf);
-
- return (unsigned long)ptr;
-}
-
-const struct bpf_func_proto bpf_get_local_storage_proto = {
- .func = bpf_get_local_storage,
- .gpl_only = false,
- .ret_type = RET_PTR_TO_MAP_VALUE,
- .arg1_type = ARG_CONST_MAP_PTR,
- .arg2_type = ARG_ANYTHING,
-};
-#endif
-
#define BPF_STRTOX_BASE_MASK 0x1F
static int __bpf_strtoull(const char *buf, size_t buf_len, u64 flags,
@@ -589,7 +513,6 @@ const struct bpf_func_proto bpf_strtoul_proto = {
.arg3_type = ARG_ANYTHING,
.arg4_type = ARG_PTR_TO_LONG,
};
-#endif
BPF_CALL_3(bpf_strncmp, const char *, s1, u32, s1_sz, const char *, s2)
{
diff --git a/net/core/filter.c b/net/core/filter.c
index e8508aaafd27..df6bae0f98a7 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -3004,17 +3004,6 @@ static const struct bpf_func_proto bpf_msg_pop_data_proto = {
};
#ifdef CONFIG_CGROUP_NET_CLASSID
-BPF_CALL_0(bpf_get_cgroup_classid_curr)
-{
- return __task_get_classid(current);
-}
-
-static const struct bpf_func_proto bpf_get_cgroup_classid_curr_proto = {
- .func = bpf_get_cgroup_classid_curr,
- .gpl_only = false,
- .ret_type = RET_INTEGER,
-};
-
BPF_CALL_1(bpf_skb_cgroup_classid, const struct sk_buff *, skb)
{
struct sock *sk = skb_to_full_sk(skb);
@@ -8104,6 +8093,9 @@ sock_ops_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
const struct bpf_func_proto bpf_msg_redirect_map_proto __weak;
const struct bpf_func_proto bpf_msg_redirect_hash_proto __weak;
+const struct bpf_func_proto bpf_get_cgroup_classid_curr_proto __weak;
+const struct bpf_func_proto bpf_get_current_cgroup_id_proto __weak;
+const struct bpf_func_proto bpf_get_current_ancestor_cgroup_id_proto __weak;
static const struct bpf_func_proto *
sk_msg_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
--
2.37.1.595.g718a3a8f04-goog
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH bpf-next v3 2/5] bpf: Use cgroup_{common,current}_func_proto in more hooks
2022-08-18 23:27 [PATCH bpf-next v3 0/5] bpf: expose bpf_{g,s}et_retval to more cgroup hooks Stanislav Fomichev
2022-08-18 23:27 ` [PATCH bpf-next v3 1/5] bpf: Introduce cgroup_{common,current}_func_proto Stanislav Fomichev
@ 2022-08-18 23:27 ` Stanislav Fomichev
2022-08-19 18:20 ` Martin KaFai Lau
2022-08-18 23:27 ` [PATCH bpf-next v3 3/5] bpf: expose bpf_strtol and bpf_strtoul to all program types Stanislav Fomichev
` (2 subsequent siblings)
4 siblings, 1 reply; 12+ messages in thread
From: Stanislav Fomichev @ 2022-08-18 23:27 UTC (permalink / raw)
To: bpf
Cc: ast, daniel, andrii, martin.lau, song, yhs, john.fastabend,
kpsingh, sdf, haoluo, jolsa
The following hooks are per-cgroup hooks but they are not
using cgroup_{common,current}_func_proto, fix it:
* BPF_PROG_TYPE_CGROUP_SKB (cg_skb)
* BPF_PROG_TYPE_CGROUP_SOCK_ADDR (cg_sock_addr)
* BPF_PROG_TYPE_CGROUP_SOCK (cg_sock)
* BPF_PROG_TYPE_LSM+BPF_LSM_CGROUP
Also:
* move common func_proto's into cgroup func_proto handlers
* make sure bpf_{g,s}et_retval are not accessible from recvmsg,
getpeername and getsockname (return/errno is ignored in these
places)
* as a side effect, expose get_current_pid_tgid, get_current_comm_proto,
get_current_ancestor_cgroup_id, get_cgroup_classid to more cgroup
hooks
Signed-off-by: Stanislav Fomichev <sdf@google.com>
---
kernel/bpf/bpf_lsm.c | 19 ++++++-----
kernel/bpf/cgroup.c | 40 +++++++++++++++++++++--
kernel/bpf/helpers.c | 1 +
net/core/filter.c | 78 ++++++++++++++++++--------------------------
4 files changed, 81 insertions(+), 57 deletions(-)
diff --git a/kernel/bpf/bpf_lsm.c b/kernel/bpf/bpf_lsm.c
index fa71d58b7ded..6eba60248e20 100644
--- a/kernel/bpf/bpf_lsm.c
+++ b/kernel/bpf/bpf_lsm.c
@@ -189,6 +189,16 @@ static const struct bpf_func_proto bpf_get_attach_cookie_proto = {
static const struct bpf_func_proto *
bpf_lsm_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
{
+#ifdef CONFIG_CGROUP_BPF
+ const struct bpf_func_proto *func_proto;
+
+ if (prog->expected_attach_type == BPF_LSM_CGROUP) {
+ func_proto = cgroup_common_func_proto(func_id, prog);
+ if (func_proto)
+ return func_proto;
+ }
+#endif
+
switch (func_id) {
case BPF_FUNC_inode_storage_get:
return &bpf_inode_storage_get_proto;
@@ -212,15 +222,6 @@ bpf_lsm_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
return prog->aux->sleepable ? &bpf_ima_file_hash_proto : NULL;
case BPF_FUNC_get_attach_cookie:
return bpf_prog_has_trampoline(prog) ? &bpf_get_attach_cookie_proto : NULL;
- case BPF_FUNC_get_local_storage:
- return prog->expected_attach_type == BPF_LSM_CGROUP ?
- &bpf_get_local_storage_proto : NULL;
- case BPF_FUNC_set_retval:
- return prog->expected_attach_type == BPF_LSM_CGROUP ?
- &bpf_set_retval_proto : NULL;
- case BPF_FUNC_get_retval:
- return prog->expected_attach_type == BPF_LSM_CGROUP ?
- &bpf_get_retval_proto : NULL;
#ifdef CONFIG_NET
case BPF_FUNC_setsockopt:
if (prog->expected_attach_type != BPF_LSM_CGROUP)
diff --git a/kernel/bpf/cgroup.c b/kernel/bpf/cgroup.c
index 74b5cc692a36..00988312279f 100644
--- a/kernel/bpf/cgroup.c
+++ b/kernel/bpf/cgroup.c
@@ -2533,9 +2533,35 @@ cgroup_common_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
case BPF_FUNC_get_local_storage:
return &bpf_get_local_storage_proto;
case BPF_FUNC_get_retval:
- return &bpf_get_retval_proto;
+ switch (prog->expected_attach_type) {
+ case BPF_CGROUP_INET_INGRESS:
+ case BPF_CGROUP_INET_EGRESS:
+ case BPF_CGROUP_SOCK_OPS:
+ case BPF_CGROUP_UDP4_RECVMSG:
+ case BPF_CGROUP_UDP6_RECVMSG:
+ case BPF_CGROUP_INET4_GETPEERNAME:
+ case BPF_CGROUP_INET6_GETPEERNAME:
+ case BPF_CGROUP_INET4_GETSOCKNAME:
+ case BPF_CGROUP_INET6_GETSOCKNAME:
+ return NULL;
+ default:
+ return &bpf_get_retval_proto;
+ }
case BPF_FUNC_set_retval:
- return &bpf_set_retval_proto;
+ switch (prog->expected_attach_type) {
+ case BPF_CGROUP_INET_INGRESS:
+ case BPF_CGROUP_INET_EGRESS:
+ case BPF_CGROUP_SOCK_OPS:
+ case BPF_CGROUP_UDP4_RECVMSG:
+ case BPF_CGROUP_UDP6_RECVMSG:
+ case BPF_CGROUP_INET4_GETPEERNAME:
+ case BPF_CGROUP_INET6_GETPEERNAME:
+ case BPF_CGROUP_INET4_GETSOCKNAME:
+ case BPF_CGROUP_INET6_GETSOCKNAME:
+ return NULL;
+ default:
+ return &bpf_set_retval_proto;
+ }
default:
return NULL;
}
@@ -2548,8 +2574,18 @@ cgroup_current_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
switch (func_id) {
case BPF_FUNC_get_current_uid_gid:
return &bpf_get_current_uid_gid_proto;
+ case BPF_FUNC_get_current_pid_tgid:
+ return &bpf_get_current_pid_tgid_proto;
+ case BPF_FUNC_get_current_comm:
+ return &bpf_get_current_comm_proto;
case BPF_FUNC_get_current_cgroup_id:
return &bpf_get_current_cgroup_id_proto;
+ case BPF_FUNC_get_current_ancestor_cgroup_id:
+ return &bpf_get_current_ancestor_cgroup_id_proto;
+#ifdef CONFIG_CGROUP_NET_CLASSID
+ case BPF_FUNC_get_cgroup_classid:
+ return &bpf_get_cgroup_classid_curr_proto;
+#endif
default:
return NULL;
}
diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
index 71979d870646..53451ea6721c 100644
--- a/kernel/bpf/helpers.c
+++ b/kernel/bpf/helpers.c
@@ -1521,6 +1521,7 @@ const struct bpf_func_proto bpf_probe_read_user_str_proto __weak;
const struct bpf_func_proto bpf_probe_read_kernel_proto __weak;
const struct bpf_func_proto bpf_probe_read_kernel_str_proto __weak;
const struct bpf_func_proto bpf_task_pt_regs_proto __weak;
+const struct bpf_func_proto bpf_get_cgroup_classid_curr_proto __weak;
const struct bpf_func_proto *
bpf_base_func_proto(enum bpf_func_id func_id)
diff --git a/net/core/filter.c b/net/core/filter.c
index df6bae0f98a7..eb8560a7c674 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -7655,34 +7655,23 @@ const struct bpf_func_proto bpf_sk_storage_get_cg_sock_proto __weak;
static const struct bpf_func_proto *
sock_filter_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
{
+ const struct bpf_func_proto *func_proto;
+
+ func_proto = cgroup_common_func_proto(func_id, prog);
+ if (func_proto)
+ return func_proto;
+
+ func_proto = cgroup_current_func_proto(func_id, prog);
+ if (func_proto)
+ return func_proto;
+
switch (func_id) {
- /* inet and inet6 sockets are created in a process
- * context so there is always a valid uid/gid
- */
- case BPF_FUNC_get_current_uid_gid:
- return &bpf_get_current_uid_gid_proto;
- case BPF_FUNC_get_local_storage:
- return &bpf_get_local_storage_proto;
case BPF_FUNC_get_socket_cookie:
return &bpf_get_socket_cookie_sock_proto;
case BPF_FUNC_get_netns_cookie:
return &bpf_get_netns_cookie_sock_proto;
case BPF_FUNC_perf_event_output:
return &bpf_event_output_data_proto;
- case BPF_FUNC_get_current_pid_tgid:
- return &bpf_get_current_pid_tgid_proto;
- case BPF_FUNC_get_current_comm:
- return &bpf_get_current_comm_proto;
-#ifdef CONFIG_CGROUPS
- case BPF_FUNC_get_current_cgroup_id:
- return &bpf_get_current_cgroup_id_proto;
- case BPF_FUNC_get_current_ancestor_cgroup_id:
- return &bpf_get_current_ancestor_cgroup_id_proto;
-#endif
-#ifdef CONFIG_CGROUP_NET_CLASSID
- case BPF_FUNC_get_cgroup_classid:
- return &bpf_get_cgroup_classid_curr_proto;
-#endif
case BPF_FUNC_sk_storage_get:
return &bpf_sk_storage_get_cg_sock_proto;
case BPF_FUNC_ktime_get_coarse_ns:
@@ -7695,12 +7684,17 @@ sock_filter_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
static const struct bpf_func_proto *
sock_addr_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
{
+ const struct bpf_func_proto *func_proto;
+
+ func_proto = cgroup_common_func_proto(func_id, prog);
+ if (func_proto)
+ return func_proto;
+
+ func_proto = cgroup_current_func_proto(func_id, prog);
+ if (func_proto)
+ return func_proto;
+
switch (func_id) {
- /* inet and inet6 sockets are created in a process
- * context so there is always a valid uid/gid
- */
- case BPF_FUNC_get_current_uid_gid:
- return &bpf_get_current_uid_gid_proto;
case BPF_FUNC_bind:
switch (prog->expected_attach_type) {
case BPF_CGROUP_INET4_CONNECT:
@@ -7713,24 +7707,8 @@ sock_addr_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
return &bpf_get_socket_cookie_sock_addr_proto;
case BPF_FUNC_get_netns_cookie:
return &bpf_get_netns_cookie_sock_addr_proto;
- case BPF_FUNC_get_local_storage:
- return &bpf_get_local_storage_proto;
case BPF_FUNC_perf_event_output:
return &bpf_event_output_data_proto;
- case BPF_FUNC_get_current_pid_tgid:
- return &bpf_get_current_pid_tgid_proto;
- case BPF_FUNC_get_current_comm:
- return &bpf_get_current_comm_proto;
-#ifdef CONFIG_CGROUPS
- case BPF_FUNC_get_current_cgroup_id:
- return &bpf_get_current_cgroup_id_proto;
- case BPF_FUNC_get_current_ancestor_cgroup_id:
- return &bpf_get_current_ancestor_cgroup_id_proto;
-#endif
-#ifdef CONFIG_CGROUP_NET_CLASSID
- case BPF_FUNC_get_cgroup_classid:
- return &bpf_get_cgroup_classid_curr_proto;
-#endif
#ifdef CONFIG_INET
case BPF_FUNC_sk_lookup_tcp:
return &bpf_sock_addr_sk_lookup_tcp_proto;
@@ -7811,9 +7789,13 @@ const struct bpf_func_proto bpf_sk_storage_delete_proto __weak;
static const struct bpf_func_proto *
cg_skb_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
{
+ const struct bpf_func_proto *func_proto;
+
+ func_proto = cgroup_common_func_proto(func_id, prog);
+ if (func_proto)
+ return func_proto;
+
switch (func_id) {
- case BPF_FUNC_get_local_storage:
- return &bpf_get_local_storage_proto;
case BPF_FUNC_sk_fullsock:
return &bpf_sk_fullsock_proto;
case BPF_FUNC_sk_storage_get:
@@ -8053,6 +8035,12 @@ const struct bpf_func_proto bpf_sock_hash_update_proto __weak;
static const struct bpf_func_proto *
sock_ops_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
{
+ const struct bpf_func_proto *func_proto;
+
+ func_proto = cgroup_common_func_proto(func_id, prog);
+ if (func_proto)
+ return func_proto;
+
switch (func_id) {
case BPF_FUNC_setsockopt:
return &bpf_sock_ops_setsockopt_proto;
@@ -8066,8 +8054,6 @@ sock_ops_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
return &bpf_sock_hash_update_proto;
case BPF_FUNC_get_socket_cookie:
return &bpf_get_socket_cookie_sock_ops_proto;
- case BPF_FUNC_get_local_storage:
- return &bpf_get_local_storage_proto;
case BPF_FUNC_perf_event_output:
return &bpf_event_output_data_proto;
case BPF_FUNC_sk_storage_get:
--
2.37.1.595.g718a3a8f04-goog
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH bpf-next v3 3/5] bpf: expose bpf_strtol and bpf_strtoul to all program types
2022-08-18 23:27 [PATCH bpf-next v3 0/5] bpf: expose bpf_{g,s}et_retval to more cgroup hooks Stanislav Fomichev
2022-08-18 23:27 ` [PATCH bpf-next v3 1/5] bpf: Introduce cgroup_{common,current}_func_proto Stanislav Fomichev
2022-08-18 23:27 ` [PATCH bpf-next v3 2/5] bpf: Use cgroup_{common,current}_func_proto in more hooks Stanislav Fomichev
@ 2022-08-18 23:27 ` Stanislav Fomichev
2022-08-19 18:22 ` Martin KaFai Lau
2022-08-18 23:27 ` [PATCH bpf-next v3 4/5] bpf: update bpf_{g,s}et_retval documentation Stanislav Fomichev
2022-08-18 23:27 ` [PATCH bpf-next v3 5/5] selftests/bpf: Make sure bpf_{g,s}et_retval is exposed everywhere Stanislav Fomichev
4 siblings, 1 reply; 12+ messages in thread
From: Stanislav Fomichev @ 2022-08-18 23:27 UTC (permalink / raw)
To: bpf
Cc: ast, daniel, andrii, martin.lau, song, yhs, john.fastabend,
kpsingh, sdf, haoluo, jolsa, Martin KaFai Lau
bpf_strncmp is already exposed everywhere. The motivation is to keep
those helpers in kernel/bpf/helpers.c. Otherwise it's tempting to move
them under kernel/bpf/cgroup.c because they are currently only used
by sysctl prog types.
Suggested-by: Martin KaFai Lau <kafai@fb.com>
Signed-off-by: Stanislav Fomichev <sdf@google.com>
---
kernel/bpf/cgroup.c | 4 ----
kernel/bpf/helpers.c | 4 ++++
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/kernel/bpf/cgroup.c b/kernel/bpf/cgroup.c
index 00988312279f..47796280232b 100644
--- a/kernel/bpf/cgroup.c
+++ b/kernel/bpf/cgroup.c
@@ -2187,10 +2187,6 @@ sysctl_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
return func_proto;
switch (func_id) {
- case BPF_FUNC_strtol:
- return &bpf_strtol_proto;
- case BPF_FUNC_strtoul:
- return &bpf_strtoul_proto;
case BPF_FUNC_sysctl_get_name:
return &bpf_sysctl_get_name_proto;
case BPF_FUNC_sysctl_get_current_value:
diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
index 53451ea6721c..8bb78acfe88d 100644
--- a/kernel/bpf/helpers.c
+++ b/kernel/bpf/helpers.c
@@ -1577,6 +1577,10 @@ bpf_base_func_proto(enum bpf_func_id func_id)
return &bpf_loop_proto;
case BPF_FUNC_strncmp:
return &bpf_strncmp_proto;
+ case BPF_FUNC_strtol:
+ return &bpf_strtol_proto;
+ case BPF_FUNC_strtoul:
+ return &bpf_strtoul_proto;
case BPF_FUNC_dynptr_from_mem:
return &bpf_dynptr_from_mem_proto;
case BPF_FUNC_dynptr_read:
--
2.37.1.595.g718a3a8f04-goog
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH bpf-next v3 4/5] bpf: update bpf_{g,s}et_retval documentation
2022-08-18 23:27 [PATCH bpf-next v3 0/5] bpf: expose bpf_{g,s}et_retval to more cgroup hooks Stanislav Fomichev
` (2 preceding siblings ...)
2022-08-18 23:27 ` [PATCH bpf-next v3 3/5] bpf: expose bpf_strtol and bpf_strtoul to all program types Stanislav Fomichev
@ 2022-08-18 23:27 ` Stanislav Fomichev
2022-08-19 18:25 ` Martin KaFai Lau
2022-08-18 23:27 ` [PATCH bpf-next v3 5/5] selftests/bpf: Make sure bpf_{g,s}et_retval is exposed everywhere Stanislav Fomichev
4 siblings, 1 reply; 12+ messages in thread
From: Stanislav Fomichev @ 2022-08-18 23:27 UTC (permalink / raw)
To: bpf
Cc: ast, daniel, andrii, martin.lau, song, yhs, john.fastabend,
kpsingh, sdf, haoluo, jolsa
* replace 'syscall' with 'upper layers', still mention that it's being
exported via syscall errno
* describe what happens in set_retval(-EPERM) + return 1
* describe what happens with bind's 'return 3'
Signed-off-by: Stanislav Fomichev <sdf@google.com>
---
include/uapi/linux/bpf.h | 22 +++++++++++++++++-----
tools/include/uapi/linux/bpf.h | 22 +++++++++++++++++-----
2 files changed, 34 insertions(+), 10 deletions(-)
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 934a2a8beb87..954b897a631e 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -5085,17 +5085,29 @@ union bpf_attr {
*
* int bpf_get_retval(void)
* Description
- * Get the syscall's return value that will be returned to userspace.
+ * Get the BPF program's return value that will be returned to the upper layers.
*
- * This helper is currently supported by cgroup programs only.
+ * This helper is currently supported by cgroup programs and only by the hooks
+ * where BPF program's return value is returned to the userspace via errno.
* Return
- * The syscall's return value.
+ * The BPF program's return value.
*
* int bpf_set_retval(int retval)
* Description
- * Set the syscall's return value that will be returned to userspace.
+ * Set the BPF program's return value that will be returned to the upper layers.
+ *
+ * This helper is currently supported by cgroup programs and only by the hooks
+ * where BPF program's return value is returned to the userspace via errno.
+ *
+ * Note that there is the following corner case where the program exports an error
+ * via bpf_set_retval but signals success via 'return 1':
+ *
+ * bpf_set_retval(-EPERM);
+ * return 1;
+ *
+ * In this case, the BPF program's return value will use helper's -EPERM. This
+ * still holds true for cgroup/bind{4,6} which supports extra 'return 3' success case.
*
- * This helper is currently supported by cgroup programs only.
* Return
* 0 on success, or a negative error in case of failure.
*
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index 1d6085e15fc8..b99ff5f34c61 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -5085,17 +5085,29 @@ union bpf_attr {
*
* int bpf_get_retval(void)
* Description
- * Get the syscall's return value that will be returned to userspace.
+ * Get the BPF program's return value that will be returned to the upper layers.
*
- * This helper is currently supported by cgroup programs only.
+ * This helper is currently supported by cgroup programs and only by the hooks
+ * where BPF program's return value is returned to the userspace via errno.
* Return
- * The syscall's return value.
+ * The BPF program's return value.
*
* int bpf_set_retval(int retval)
* Description
- * Set the syscall's return value that will be returned to userspace.
+ * Set the BPF program's return value that will be returned to the upper layers.
+ *
+ * This helper is currently supported by cgroup programs and only by the hooks
+ * where BPF program's return value is returned to the userspace via errno.
+ *
+ * Note that there is the following corner case where the program exports an error
+ * via bpf_set_retval but signals success via 'return 1':
+ *
+ * bpf_set_retval(-EPERM);
+ * return 1;
+ *
+ * In this case, the BPF program's return value will use helper's -EPERM. This
+ * still holds true for cgroup/bind{4,6} which supports extra 'return 3' success case.
*
- * This helper is currently supported by cgroup programs only.
* Return
* 0 on success, or a negative error in case of failure.
*
--
2.37.1.595.g718a3a8f04-goog
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH bpf-next v3 5/5] selftests/bpf: Make sure bpf_{g,s}et_retval is exposed everywhere
2022-08-18 23:27 [PATCH bpf-next v3 0/5] bpf: expose bpf_{g,s}et_retval to more cgroup hooks Stanislav Fomichev
` (3 preceding siblings ...)
2022-08-18 23:27 ` [PATCH bpf-next v3 4/5] bpf: update bpf_{g,s}et_retval documentation Stanislav Fomichev
@ 2022-08-18 23:27 ` Stanislav Fomichev
2022-08-19 18:33 ` Martin KaFai Lau
4 siblings, 1 reply; 12+ messages in thread
From: Stanislav Fomichev @ 2022-08-18 23:27 UTC (permalink / raw)
To: bpf
Cc: ast, daniel, andrii, martin.lau, song, yhs, john.fastabend,
kpsingh, sdf, haoluo, jolsa
For each hook, have a simple bpf_set_retval(bpf_get_retval) program
and make sure it loads for the hooks we want. The exceptions are
the hooks which don't propagate the error to the callers:
- sockops
- recvmsg
- getpeername
- getsockname
Signed-off-by: Stanislav Fomichev <sdf@google.com>
---
tools/testing/selftests/bpf/Makefile | 1 +
.../bpf/cgroup_getset_retval_hooks.h | 25 ++++++++++
.../bpf/prog_tests/cgroup_getset_retval.c | 48 +++++++++++++++++++
.../bpf/progs/cgroup_getset_retval_hooks.c | 16 +++++++
4 files changed, 90 insertions(+)
create mode 100644 tools/testing/selftests/bpf/cgroup_getset_retval_hooks.h
create mode 100644 tools/testing/selftests/bpf/progs/cgroup_getset_retval_hooks.c
diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index 8d59ec7f4c2d..eecad99f1735 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -323,6 +323,7 @@ $(OUTPUT)/test_l4lb_noinline.o: BPF_CFLAGS += -fno-inline
$(OUTPUT)/test_xdp_noinline.o: BPF_CFLAGS += -fno-inline
$(OUTPUT)/flow_dissector_load.o: flow_dissector_load.h
+$(OUTPUT)/cgroup_getset_retval_hooks.o: cgroup_getset_retval_hooks.h
# Build BPF object using Clang
# $1 - input .c file
diff --git a/tools/testing/selftests/bpf/cgroup_getset_retval_hooks.h b/tools/testing/selftests/bpf/cgroup_getset_retval_hooks.h
new file mode 100644
index 000000000000..a525d3544fd7
--- /dev/null
+++ b/tools/testing/selftests/bpf/cgroup_getset_retval_hooks.h
@@ -0,0 +1,25 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+BPF_RETVAL_HOOK(ingress, "cgroup_skb/ingress", __sk_buff, -EINVAL)
+BPF_RETVAL_HOOK(egress, "cgroup_skb/egress", __sk_buff, -EINVAL)
+BPF_RETVAL_HOOK(sock_create, "cgroup/sock_create", bpf_sock, 0)
+BPF_RETVAL_HOOK(sock_ops, "sockops", bpf_sock_ops, -EINVAL)
+BPF_RETVAL_HOOK(dev, "cgroup/dev", bpf_cgroup_dev_ctx, 0)
+BPF_RETVAL_HOOK(bind4, "cgroup/bind4", bpf_sock_addr, 0)
+BPF_RETVAL_HOOK(bind6, "cgroup/bind6", bpf_sock_addr, 0)
+BPF_RETVAL_HOOK(connect4, "cgroup/connect4", bpf_sock_addr, 0)
+BPF_RETVAL_HOOK(connect6, "cgroup/connect6", bpf_sock_addr, 0)
+BPF_RETVAL_HOOK(post_bind4, "cgroup/post_bind4", bpf_sock_addr, 0)
+BPF_RETVAL_HOOK(post_bind6, "cgroup/post_bind6", bpf_sock_addr, 0)
+BPF_RETVAL_HOOK(sendmsg4, "cgroup/sendmsg4", bpf_sock_addr, 0)
+BPF_RETVAL_HOOK(sendmsg6, "cgroup/sendmsg6", bpf_sock_addr, 0)
+BPF_RETVAL_HOOK(sysctl, "cgroup/sysctl", bpf_sysctl, 0)
+BPF_RETVAL_HOOK(recvmsg4, "cgroup/recvmsg4", bpf_sock_addr, -EINVAL)
+BPF_RETVAL_HOOK(recvmsg6, "cgroup/recvmsg6", bpf_sock_addr, -EINVAL)
+BPF_RETVAL_HOOK(getsockopt, "cgroup/getsockopt", bpf_sockopt, 0)
+BPF_RETVAL_HOOK(setsockopt, "cgroup/setsockopt", bpf_sockopt, 0)
+BPF_RETVAL_HOOK(getpeername4, "cgroup/getpeername4", bpf_sock_addr, -EINVAL)
+BPF_RETVAL_HOOK(getpeername6, "cgroup/getpeername6", bpf_sock_addr, -EINVAL)
+BPF_RETVAL_HOOK(getsockname4, "cgroup/getsockname4", bpf_sock_addr, -EINVAL)
+BPF_RETVAL_HOOK(getsockname6, "cgroup/getsockname6", bpf_sock_addr, -EINVAL)
+BPF_RETVAL_HOOK(sock_release, "cgroup/sock_release", bpf_sock, 0)
diff --git a/tools/testing/selftests/bpf/prog_tests/cgroup_getset_retval.c b/tools/testing/selftests/bpf/prog_tests/cgroup_getset_retval.c
index 0b47c3c000c7..4d2fa99273d8 100644
--- a/tools/testing/selftests/bpf/prog_tests/cgroup_getset_retval.c
+++ b/tools/testing/selftests/bpf/prog_tests/cgroup_getset_retval.c
@@ -10,6 +10,7 @@
#include "cgroup_getset_retval_setsockopt.skel.h"
#include "cgroup_getset_retval_getsockopt.skel.h"
+#include "cgroup_getset_retval_hooks.skel.h"
#define SOL_CUSTOM 0xdeadbeef
@@ -433,6 +434,50 @@ static void test_getsockopt_retval_sync(int cgroup_fd, int sock_fd)
cgroup_getset_retval_getsockopt__destroy(obj);
}
+struct exposed_hook {
+ const char *name;
+ int expected_err;
+} exposed_hooks[] = {
+
+#define BPF_RETVAL_HOOK(NAME, SECTION, CTX, EXPECTED_ERR) \
+ { \
+ .name = #NAME, \
+ .expected_err = EXPECTED_ERR, \
+ },
+
+#include "cgroup_getset_retval_hooks.h"
+
+#undef BPF_RETVAL_HOOK
+};
+
+static void test_exposed_hooks(int cgroup_fd, int sock_fd)
+{
+ struct cgroup_getset_retval_hooks *skel;
+ struct bpf_program *prog;
+ int err;
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(exposed_hooks); i++) {
+ skel = cgroup_getset_retval_hooks__open();
+ if (!ASSERT_OK_PTR(skel, "cgroup_getset_retval_hooks__open"))
+ continue;
+
+ prog = bpf_object__find_program_by_name(skel->obj, exposed_hooks[i].name);
+ if (!ASSERT_NEQ(prog, NULL, "bpf_object__find_program_by_name"))
+ goto close_skel;
+
+ err = bpf_program__set_autoload(prog, true);
+ if (!ASSERT_OK(err, "bpf_program__set_autoload"))
+ goto close_skel;
+
+ err = cgroup_getset_retval_hooks__load(skel);
+ ASSERT_EQ(err, exposed_hooks[i].expected_err, "expected_err");
+
+close_skel:
+ cgroup_getset_retval_hooks__destroy(skel);
+ }
+}
+
void test_cgroup_getset_retval(void)
{
int cgroup_fd = -1;
@@ -476,6 +521,9 @@ void test_cgroup_getset_retval(void)
if (test__start_subtest("getsockopt-retval_sync"))
test_getsockopt_retval_sync(cgroup_fd, sock_fd);
+ if (test__start_subtest("exposed_hooks"))
+ test_exposed_hooks(cgroup_fd, sock_fd);
+
close_fd:
close(cgroup_fd);
}
diff --git a/tools/testing/selftests/bpf/progs/cgroup_getset_retval_hooks.c b/tools/testing/selftests/bpf/progs/cgroup_getset_retval_hooks.c
new file mode 100644
index 000000000000..13dfb4bbfd28
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/cgroup_getset_retval_hooks.c
@@ -0,0 +1,16 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+#include <linux/bpf.h>
+#include <bpf/bpf_helpers.h>
+
+#define BPF_RETVAL_HOOK(name, section, ctx, expected_err) \
+ __attribute__((__section__("?" section))) \
+ int name(struct ctx *_ctx) \
+ { \
+ bpf_set_retval(bpf_get_retval()); \
+ return 1; \
+ }
+
+#include "cgroup_getset_retval_hooks.h"
+
+#undef BPF_RETVAL_HOOK
--
2.37.1.595.g718a3a8f04-goog
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH bpf-next v3 1/5] bpf: Introduce cgroup_{common,current}_func_proto
2022-08-18 23:27 ` [PATCH bpf-next v3 1/5] bpf: Introduce cgroup_{common,current}_func_proto Stanislav Fomichev
@ 2022-08-19 18:17 ` Martin KaFai Lau
2022-08-22 18:40 ` Stanislav Fomichev
0 siblings, 1 reply; 12+ messages in thread
From: Martin KaFai Lau @ 2022-08-19 18:17 UTC (permalink / raw)
To: Stanislav Fomichev
Cc: bpf, ast, daniel, andrii, martin.lau, song, yhs, john.fastabend,
kpsingh, haoluo, jolsa
On Thu, Aug 18, 2022 at 04:27:25PM -0700, Stanislav Fomichev wrote:
> +BPF_CALL_0(bpf_get_current_cgroup_id)
> +{
> + struct cgroup *cgrp;
> + u64 cgrp_id;
> +
> + rcu_read_lock();
> + cgrp = task_dfl_cgroup(current);
> + cgrp_id = cgroup_id(cgrp);
> + rcu_read_unlock();
> +
> + return cgrp_id;
> +}
> +
> +const struct bpf_func_proto bpf_get_current_cgroup_id_proto = {
> + .func = bpf_get_current_cgroup_id,
> + .gpl_only = false,
> + .ret_type = RET_INTEGER,
> +};
> +
> +BPF_CALL_1(bpf_get_current_ancestor_cgroup_id, int, ancestor_level)
> +{
> + struct cgroup *cgrp;
> + struct cgroup *ancestor;
> + u64 cgrp_id;
> +
> + rcu_read_lock();
> + cgrp = task_dfl_cgroup(current);
> + ancestor = cgroup_ancestor(cgrp, ancestor_level);
> + cgrp_id = ancestor ? cgroup_id(ancestor) : 0;
> + rcu_read_unlock();
> +
> + return cgrp_id;
> +}
> +
> +const struct bpf_func_proto bpf_get_current_ancestor_cgroup_id_proto = {
> + .func = bpf_get_current_ancestor_cgroup_id,
> + .gpl_only = false,
> + .ret_type = RET_INTEGER,
> + .arg1_type = ARG_ANYTHING,
> +};
The bpf_get_current_cgroup_id_proto and
bpf_get_current_ancestor_cgroup_id_proto should stay at helpers.c.
Otherwise, those non cgroup hooks will have issue (eg. bpf_trace.c)
when CONFIG_CGROUP_BPF not set.
May be in the future cgroup_current_func_proto() can be re-used for non
cgroup hooks.
> +#ifdef CONFIG_CGROUP_NET_CLASSID
> +BPF_CALL_0(bpf_get_cgroup_classid_curr)
> +{
> + return __task_get_classid(current);
> +}
> +
> +const struct bpf_func_proto bpf_get_cgroup_classid_curr_proto = {
> + .func = bpf_get_cgroup_classid_curr,
> + .gpl_only = false,
> + .ret_type = RET_INTEGER,
> +};
> +#endif
Same for this one. eg. sk_msg needs it. probably stay in filter.c as-is.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH bpf-next v3 2/5] bpf: Use cgroup_{common,current}_func_proto in more hooks
2022-08-18 23:27 ` [PATCH bpf-next v3 2/5] bpf: Use cgroup_{common,current}_func_proto in more hooks Stanislav Fomichev
@ 2022-08-19 18:20 ` Martin KaFai Lau
0 siblings, 0 replies; 12+ messages in thread
From: Martin KaFai Lau @ 2022-08-19 18:20 UTC (permalink / raw)
To: Stanislav Fomichev
Cc: bpf, ast, daniel, andrii, martin.lau, song, yhs, john.fastabend,
kpsingh, haoluo, jolsa
On Thu, Aug 18, 2022 at 04:27:26PM -0700, Stanislav Fomichev wrote:
> diff --git a/kernel/bpf/bpf_lsm.c b/kernel/bpf/bpf_lsm.c
> index fa71d58b7ded..6eba60248e20 100644
> --- a/kernel/bpf/bpf_lsm.c
> +++ b/kernel/bpf/bpf_lsm.c
> @@ -189,6 +189,16 @@ static const struct bpf_func_proto bpf_get_attach_cookie_proto = {
> static const struct bpf_func_proto *
> bpf_lsm_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
> {
> +#ifdef CONFIG_CGROUP_BPF
This probably is not needed. Others lgtm.
> + const struct bpf_func_proto *func_proto;
> +
> + if (prog->expected_attach_type == BPF_LSM_CGROUP) {
> + func_proto = cgroup_common_func_proto(func_id, prog);
> + if (func_proto)
> + return func_proto;
> + }
> +#endif
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH bpf-next v3 3/5] bpf: expose bpf_strtol and bpf_strtoul to all program types
2022-08-18 23:27 ` [PATCH bpf-next v3 3/5] bpf: expose bpf_strtol and bpf_strtoul to all program types Stanislav Fomichev
@ 2022-08-19 18:22 ` Martin KaFai Lau
0 siblings, 0 replies; 12+ messages in thread
From: Martin KaFai Lau @ 2022-08-19 18:22 UTC (permalink / raw)
To: Stanislav Fomichev
Cc: bpf, ast, daniel, andrii, martin.lau, song, yhs, john.fastabend,
kpsingh, haoluo, jolsa
On Thu, Aug 18, 2022 at 04:27:27PM -0700, Stanislav Fomichev wrote:
> bpf_strncmp is already exposed everywhere. The motivation is to keep
> those helpers in kernel/bpf/helpers.c. Otherwise it's tempting to move
> them under kernel/bpf/cgroup.c because they are currently only used
> by sysctl prog types.
Acked-by: Martin KaFai Lau <kafai@fb.com>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH bpf-next v3 4/5] bpf: update bpf_{g,s}et_retval documentation
2022-08-18 23:27 ` [PATCH bpf-next v3 4/5] bpf: update bpf_{g,s}et_retval documentation Stanislav Fomichev
@ 2022-08-19 18:25 ` Martin KaFai Lau
0 siblings, 0 replies; 12+ messages in thread
From: Martin KaFai Lau @ 2022-08-19 18:25 UTC (permalink / raw)
To: Stanislav Fomichev
Cc: bpf, ast, daniel, andrii, martin.lau, song, yhs, john.fastabend,
kpsingh, haoluo, jolsa
On Thu, Aug 18, 2022 at 04:27:28PM -0700, Stanislav Fomichev wrote:
> * replace 'syscall' with 'upper layers', still mention that it's being
> exported via syscall errno
> * describe what happens in set_retval(-EPERM) + return 1
> * describe what happens with bind's 'return 3'
Acked-by: Martin KaFai Lau <kafai@fb.com>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH bpf-next v3 5/5] selftests/bpf: Make sure bpf_{g,s}et_retval is exposed everywhere
2022-08-18 23:27 ` [PATCH bpf-next v3 5/5] selftests/bpf: Make sure bpf_{g,s}et_retval is exposed everywhere Stanislav Fomichev
@ 2022-08-19 18:33 ` Martin KaFai Lau
0 siblings, 0 replies; 12+ messages in thread
From: Martin KaFai Lau @ 2022-08-19 18:33 UTC (permalink / raw)
To: Stanislav Fomichev
Cc: bpf, ast, daniel, andrii, martin.lau, song, yhs, john.fastabend,
kpsingh, haoluo, jolsa
On Thu, Aug 18, 2022 at 04:27:29PM -0700, Stanislav Fomichev wrote:
> For each hook, have a simple bpf_set_retval(bpf_get_retval) program
> and make sure it loads for the hooks we want. The exceptions are
> the hooks which don't propagate the error to the callers:
>
> - sockops
> - recvmsg
> - getpeername
> - getsockname
and also
- cg_skb ingress and egress
Acked-by: Martin KaFai Lau <kafai@fb.com>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH bpf-next v3 1/5] bpf: Introduce cgroup_{common,current}_func_proto
2022-08-19 18:17 ` Martin KaFai Lau
@ 2022-08-22 18:40 ` Stanislav Fomichev
0 siblings, 0 replies; 12+ messages in thread
From: Stanislav Fomichev @ 2022-08-22 18:40 UTC (permalink / raw)
To: Martin KaFai Lau
Cc: bpf, ast, daniel, andrii, martin.lau, song, yhs, john.fastabend,
kpsingh, haoluo, jolsa
On Fri, Aug 19, 2022 at 11:18 AM Martin KaFai Lau <kafai@fb.com> wrote:
>
> On Thu, Aug 18, 2022 at 04:27:25PM -0700, Stanislav Fomichev wrote:
> > +BPF_CALL_0(bpf_get_current_cgroup_id)
> > +{
> > + struct cgroup *cgrp;
> > + u64 cgrp_id;
> > +
> > + rcu_read_lock();
> > + cgrp = task_dfl_cgroup(current);
> > + cgrp_id = cgroup_id(cgrp);
> > + rcu_read_unlock();
> > +
> > + return cgrp_id;
> > +}
> > +
> > +const struct bpf_func_proto bpf_get_current_cgroup_id_proto = {
> > + .func = bpf_get_current_cgroup_id,
> > + .gpl_only = false,
> > + .ret_type = RET_INTEGER,
> > +};
> > +
> > +BPF_CALL_1(bpf_get_current_ancestor_cgroup_id, int, ancestor_level)
> > +{
> > + struct cgroup *cgrp;
> > + struct cgroup *ancestor;
> > + u64 cgrp_id;
> > +
> > + rcu_read_lock();
> > + cgrp = task_dfl_cgroup(current);
> > + ancestor = cgroup_ancestor(cgrp, ancestor_level);
> > + cgrp_id = ancestor ? cgroup_id(ancestor) : 0;
> > + rcu_read_unlock();
> > +
> > + return cgrp_id;
> > +}
> > +
> > +const struct bpf_func_proto bpf_get_current_ancestor_cgroup_id_proto = {
> > + .func = bpf_get_current_ancestor_cgroup_id,
> > + .gpl_only = false,
> > + .ret_type = RET_INTEGER,
> > + .arg1_type = ARG_ANYTHING,
> > +};
> The bpf_get_current_cgroup_id_proto and
> bpf_get_current_ancestor_cgroup_id_proto should stay at helpers.c.
> Otherwise, those non cgroup hooks will have issue (eg. bpf_trace.c)
> when CONFIG_CGROUP_BPF not set.
Oh, good point on bpf_trace.c, will keep in helpers.c
I'm now a bit surprised I haven't seen a build failure :-/
Thank you for the review! Will address your other point and resend.
> May be in the future cgroup_current_func_proto() can be re-used for non
> cgroup hooks.
>
> > +#ifdef CONFIG_CGROUP_NET_CLASSID
> > +BPF_CALL_0(bpf_get_cgroup_classid_curr)
> > +{
> > + return __task_get_classid(current);
> > +}
> > +
> > +const struct bpf_func_proto bpf_get_cgroup_classid_curr_proto = {
> > + .func = bpf_get_cgroup_classid_curr,
> > + .gpl_only = false,
> > + .ret_type = RET_INTEGER,
> > +};
> > +#endif
> Same for this one. eg. sk_msg needs it. probably stay in filter.c as-is.
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2022-08-22 18:44 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-08-18 23:27 [PATCH bpf-next v3 0/5] bpf: expose bpf_{g,s}et_retval to more cgroup hooks Stanislav Fomichev
2022-08-18 23:27 ` [PATCH bpf-next v3 1/5] bpf: Introduce cgroup_{common,current}_func_proto Stanislav Fomichev
2022-08-19 18:17 ` Martin KaFai Lau
2022-08-22 18:40 ` Stanislav Fomichev
2022-08-18 23:27 ` [PATCH bpf-next v3 2/5] bpf: Use cgroup_{common,current}_func_proto in more hooks Stanislav Fomichev
2022-08-19 18:20 ` Martin KaFai Lau
2022-08-18 23:27 ` [PATCH bpf-next v3 3/5] bpf: expose bpf_strtol and bpf_strtoul to all program types Stanislav Fomichev
2022-08-19 18:22 ` Martin KaFai Lau
2022-08-18 23:27 ` [PATCH bpf-next v3 4/5] bpf: update bpf_{g,s}et_retval documentation Stanislav Fomichev
2022-08-19 18:25 ` Martin KaFai Lau
2022-08-18 23:27 ` [PATCH bpf-next v3 5/5] selftests/bpf: Make sure bpf_{g,s}et_retval is exposed everywhere Stanislav Fomichev
2022-08-19 18:33 ` Martin KaFai Lau
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.