[PATCH bpf-next 0/3] bpf: expose bpf_{g,s}et_retval to more cgroup hooks

[PATCH bpf-next 0/3] bpf: expose bpf_{g,s}et_retval to more cgroup hooks

[Stanislav Fomichev]

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.

Stanislav Fomichev (3):
  bpf: introduce cgroup_{common,current}_func_proto
  bpf: use cgroup_{common,current}_func_proto in more hooks
  selftests/bpf: make sure bpf_{g,s}et_retval is exposed everywhere

 include/linux/bpf.h                           | 16 +++
 kernel/bpf/cgroup.c                           | 82 +++++++--------
 kernel/bpf/helpers.c                          | 99 ++++++++++++++++++-
 net/core/filter.c                             | 78 ++++++---------
 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 +++
 8 files changed, 270 insertions(+), 95 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

[PATCH bpf-next 1/3] bpf: introduce cgroup_{common,current}_func_proto

[Stanislav Fomichev]

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 into kernel/bpf/helpers.c so they are more
usable from the core parts.

Signed-off-by: Stanislav Fomichev <sdf@google.com>
---
 include/linux/bpf.h  | 16 +++++++++
 kernel/bpf/cgroup.c  | 82 +++++++++++++++++++-------------------------
 kernel/bpf/helpers.c | 67 ++++++++++++++++++++++++++++++++++--
 3 files changed, 116 insertions(+), 49 deletions(-)

diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index a627a02cf8ab..cdb295c6c728 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -1948,6 +1948,10 @@ struct bpf_prog *bpf_prog_by_id(u32 id);
 struct bpf_link *bpf_link_by_id(u32 id);
 
 const struct bpf_func_proto *bpf_base_func_proto(enum bpf_func_id func_id);
+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);
 void bpf_task_storage_free(struct task_struct *task);
 bool bpf_prog_has_kfunc_call(const struct bpf_prog *prog);
 const struct btf_func_model *
@@ -2154,6 +2158,18 @@ bpf_base_func_proto(enum bpf_func_id func_id)
 	return NULL;
 }
 
+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 void bpf_task_storage_free(struct task_struct *task)
 {
 }
diff --git a/kernel/bpf/cgroup.c b/kernel/bpf/cgroup.c
index 59b7eb60d5b4..cc0b33b7d4cc 100644
--- a/kernel/bpf/cgroup.c
+++ b/kernel/bpf/cgroup.c
@@ -1527,63 +1527,27 @@ int __cgroup_bpf_check_dev_permission(short dev_type, u32 major, u32 minor,
 	return ret;
 }
 
-BPF_CALL_0(bpf_get_retval)
-{
-	struct bpf_cg_run_ctx *ctx =
-		container_of(current->bpf_ctx, struct bpf_cg_run_ctx, run_ctx);
-
-	return ctx->retval;
-}
-
-const struct bpf_func_proto bpf_get_retval_proto = {
-	.func		= bpf_get_retval,
-	.gpl_only	= false,
-	.ret_type	= RET_INTEGER,
-};
-
-BPF_CALL_1(bpf_set_retval, int, retval)
+static const struct bpf_func_proto *
+cgroup_dev_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
 {
-	struct bpf_cg_run_ctx *ctx =
-		container_of(current->bpf_ctx, struct bpf_cg_run_ctx, run_ctx);
+	const struct bpf_func_proto *func_proto;
 
-	ctx->retval = retval;
-	return 0;
-}
+	func_proto = cgroup_common_func_proto(func_id, prog);
+	if (func_proto)
+		return func_proto;
 
-const struct bpf_func_proto bpf_set_retval_proto = {
-	.func		= bpf_set_retval,
-	.gpl_only	= false,
-	.ret_type	= RET_INTEGER,
-	.arg1_type	= ARG_ANYTHING,
-};
+	func_proto = cgroup_current_func_proto(func_id, prog);
+	if (func_proto)
+		return func_proto;
 
-static const struct bpf_func_proto *
-cgroup_base_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_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 +2060,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 +2085,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 +2209,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 +2240,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);
 	}
 }
 
diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
index 3c1b9bbcf971..de7d2fabb06d 100644
--- a/kernel/bpf/helpers.c
+++ b/kernel/bpf/helpers.c
@@ -429,7 +429,6 @@ const struct bpf_func_proto bpf_get_current_ancestor_cgroup_id_proto = {
 };
 
 #ifdef CONFIG_CGROUP_BPF
-
 BPF_CALL_2(bpf_get_local_storage, struct bpf_map *, map, u64, flags)
 {
 	/* flags argument is not used now,
@@ -460,7 +459,37 @@ const struct bpf_func_proto bpf_get_local_storage_proto = {
 	.arg1_type	= ARG_CONST_MAP_PTR,
 	.arg2_type	= ARG_ANYTHING,
 };
-#endif
+
+BPF_CALL_0(bpf_get_retval)
+{
+	struct bpf_cg_run_ctx *ctx =
+		container_of(current->bpf_ctx, struct bpf_cg_run_ctx, run_ctx);
+
+	return ctx->retval;
+}
+
+const struct bpf_func_proto bpf_get_retval_proto = {
+	.func		= bpf_get_retval,
+	.gpl_only	= false,
+	.ret_type	= RET_INTEGER,
+};
+
+BPF_CALL_1(bpf_set_retval, int, retval)
+{
+	struct bpf_cg_run_ctx *ctx =
+		container_of(current->bpf_ctx, struct bpf_cg_run_ctx, run_ctx);
+
+	ctx->retval = retval;
+	return 0;
+}
+
+const struct bpf_func_proto bpf_set_retval_proto = {
+	.func		= bpf_set_retval,
+	.gpl_only	= false,
+	.ret_type	= RET_INTEGER,
+	.arg1_type	= ARG_ANYTHING,
+};
+#endif /* CONFIG_CGROUP_BPF */
 
 #define BPF_STRTOX_BASE_MASK 0x1F
 
@@ -1726,6 +1755,40 @@ bpf_base_func_proto(enum bpf_func_id func_id)
 	}
 }
 
+/* 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) {
+#ifdef CONFIG_CGROUP_BPF
+	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;
+#endif
+	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) {
+#ifdef CONFIG_CGROUPS
+	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;
+#endif
+	default:
+		return NULL;
+	}
+}
+
 BTF_SET8_START(tracing_btf_ids)
 #ifdef CONFIG_KEXEC_CORE
 BTF_ID_FLAGS(func, crash_kexec, KF_DESTRUCTIVE)
-- 
2.37.1.595.g718a3a8f04-goog

Re: [PATCH bpf-next 1/3] bpf: introduce cgroup_{common,current}_func_proto

[Martin KaFai Lau]

On Fri, Aug 12, 2022 at 12:02:39PM -0700, Stanislav Fomichev wrote:
> diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
> index 3c1b9bbcf971..de7d2fabb06d 100644
> --- a/kernel/bpf/helpers.c
> +++ b/kernel/bpf/helpers.c
> @@ -429,7 +429,6 @@ const struct bpf_func_proto bpf_get_current_ancestor_cgroup_id_proto = {
>  };
>  
>  #ifdef CONFIG_CGROUP_BPF
> -
>  BPF_CALL_2(bpf_get_local_storage, struct bpf_map *, map, u64, flags)
>  {
>  	/* flags argument is not used now,
> @@ -460,7 +459,37 @@ const struct bpf_func_proto bpf_get_local_storage_proto = {
>  	.arg1_type	= ARG_CONST_MAP_PTR,
>  	.arg2_type	= ARG_ANYTHING,
>  };
> -#endif
> +
> +BPF_CALL_0(bpf_get_retval)
> +{
> +	struct bpf_cg_run_ctx *ctx =
> +		container_of(current->bpf_ctx, struct bpf_cg_run_ctx, run_ctx);
> +
> +	return ctx->retval;
> +}
> +
> +const struct bpf_func_proto bpf_get_retval_proto = {
> +	.func		= bpf_get_retval,
> +	.gpl_only	= false,
> +	.ret_type	= RET_INTEGER,
> +};
> +
> +BPF_CALL_1(bpf_set_retval, int, retval)
> +{
> +	struct bpf_cg_run_ctx *ctx =
> +		container_of(current->bpf_ctx, struct bpf_cg_run_ctx, run_ctx);
> +
> +	ctx->retval = retval;
> +	return 0;
> +}
> +
> +const struct bpf_func_proto bpf_set_retval_proto = {
> +	.func		= bpf_set_retval,
> +	.gpl_only	= false,
> +	.ret_type	= RET_INTEGER,
> +	.arg1_type	= ARG_ANYTHING,
> +};
> +#endif /* CONFIG_CGROUP_BPF */
>  
>  #define BPF_STRTOX_BASE_MASK 0x1F
>  
> @@ -1726,6 +1755,40 @@ bpf_base_func_proto(enum bpf_func_id func_id)
>  	}
>  }
>  
> +/* 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) {
> +#ifdef CONFIG_CGROUP_BPF
> +	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;
> +#endif
> +	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) {
> +#ifdef CONFIG_CGROUPS
> +	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;
> +#endif
> +	default:
> +		return NULL;
> +	}
> +}
Does it make sense to move all these changes to kernel/bpf/cgroup.c
instead such that there is no need to do 'ifdef CONFIG_CGROUPS*'.
bpf_get_local_storage probably needs to move to kernel/bpf/cgroup.c
also.

Re: [PATCH bpf-next 1/3] bpf: introduce cgroup_{common,current}_func_proto

[Stanislav Fomichev]

On Mon, Aug 15, 2022 at 2:27 PM Martin KaFai Lau <kafai@fb.com> wrote:
>
> On Fri, Aug 12, 2022 at 12:02:39PM -0700, Stanislav Fomichev wrote:
> > diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
> > index 3c1b9bbcf971..de7d2fabb06d 100644
> > --- a/kernel/bpf/helpers.c
> > +++ b/kernel/bpf/helpers.c
> > @@ -429,7 +429,6 @@ const struct bpf_func_proto bpf_get_current_ancestor_cgroup_id_proto = {
> >  };
> >
> >  #ifdef CONFIG_CGROUP_BPF
> > -
> >  BPF_CALL_2(bpf_get_local_storage, struct bpf_map *, map, u64, flags)
> >  {
> >       /* flags argument is not used now,
> > @@ -460,7 +459,37 @@ const struct bpf_func_proto bpf_get_local_storage_proto = {
> >       .arg1_type      = ARG_CONST_MAP_PTR,
> >       .arg2_type      = ARG_ANYTHING,
> >  };
> > -#endif
> > +
> > +BPF_CALL_0(bpf_get_retval)
> > +{
> > +     struct bpf_cg_run_ctx *ctx =
> > +             container_of(current->bpf_ctx, struct bpf_cg_run_ctx, run_ctx);
> > +
> > +     return ctx->retval;
> > +}
> > +
> > +const struct bpf_func_proto bpf_get_retval_proto = {
> > +     .func           = bpf_get_retval,
> > +     .gpl_only       = false,
> > +     .ret_type       = RET_INTEGER,
> > +};
> > +
> > +BPF_CALL_1(bpf_set_retval, int, retval)
> > +{
> > +     struct bpf_cg_run_ctx *ctx =
> > +             container_of(current->bpf_ctx, struct bpf_cg_run_ctx, run_ctx);
> > +
> > +     ctx->retval = retval;
> > +     return 0;
> > +}
> > +
> > +const struct bpf_func_proto bpf_set_retval_proto = {
> > +     .func           = bpf_set_retval,
> > +     .gpl_only       = false,
> > +     .ret_type       = RET_INTEGER,
> > +     .arg1_type      = ARG_ANYTHING,
> > +};
> > +#endif /* CONFIG_CGROUP_BPF */
> >
> >  #define BPF_STRTOX_BASE_MASK 0x1F
> >
> > @@ -1726,6 +1755,40 @@ bpf_base_func_proto(enum bpf_func_id func_id)
> >       }
> >  }
> >
> > +/* 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) {
> > +#ifdef CONFIG_CGROUP_BPF
> > +     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;
> > +#endif
> > +     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) {
> > +#ifdef CONFIG_CGROUPS
> > +     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;
> > +#endif
> > +     default:
> > +             return NULL;
> > +     }
> > +}
> Does it make sense to move all these changes to kernel/bpf/cgroup.c
> instead such that there is no need to do 'ifdef CONFIG_CGROUPS*'.
> bpf_get_local_storage probably needs to move to kernel/bpf/cgroup.c
> also.

Hm, didn't think about it, let me try moving everything under
bpf/cgroup.c instead..

(saw the build failures but decided not to spam with a v2)

[PATCH bpf-next 2/3] bpf: use cgroup_{common,current}_func_proto in more hooks

[Stanislav Fomichev]

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)

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 ignore 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/helpers.c | 36 ++++++++++++++++++--
 net/core/filter.c    | 78 ++++++++++++++++++--------------------------
 2 files changed, 66 insertions(+), 48 deletions(-)

diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
index de7d2fabb06d..87ce47b13b22 100644
--- a/kernel/bpf/helpers.c
+++ b/kernel/bpf/helpers.c
@@ -1764,9 +1764,31 @@ 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_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_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;
+		}
 #endif
 	default:
 		return NULL;
@@ -1781,8 +1803,18 @@ cgroup_current_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
 #ifdef CONFIG_CGROUPS
 	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;
+#endif
+#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/net/core/filter.c b/net/core/filter.c
index 5669248aff25..6731e60621e9 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -7663,34 +7663,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:
@@ -7703,12 +7692,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:
@@ -7721,24 +7715,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;
@@ -7819,9 +7797,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:
@@ -8061,6 +8043,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;
@@ -8074,8 +8062,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

Re: [PATCH bpf-next 2/3] bpf: use cgroup_{common,current}_func_proto in more hooks

[kernel test robot]

Hi Stanislav,

I love your patch! Yet something to improve:

[auto build test ERROR on bpf-next/master]

url:    https://github.com/intel-lab-lkp/linux/commits/Stanislav-Fomichev/bpf-expose-bpf_-g-s-et_retval-to-more-cgroup-hooks/20220813-030615
base:   https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git master
config: x86_64-randconfig-a013 (https://download.01.org/0day-ci/archive/20220813/202208131415.CkdRZBrY-lkp@intel.com/config)
compiler: gcc-11 (Debian 11.3.0-3) 11.3.0
reproduce (this is a W=1 build):
        # https://github.com/intel-lab-lkp/linux/commit/0429824054f7a843ee976d48432e825e493a0a7e
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Stanislav-Fomichev/bpf-expose-bpf_-g-s-et_retval-to-more-cgroup-hooks/20220813-030615
        git checkout 0429824054f7a843ee976d48432e825e493a0a7e
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        make W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   kernel/bpf/helpers.c: In function 'cgroup_current_func_proto':
>> kernel/bpf/helpers.c:1817:25: error: 'bpf_get_cgroup_classid_curr_proto' undeclared (first use in this function)
    1817 |                 return &bpf_get_cgroup_classid_curr_proto;
         |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   kernel/bpf/helpers.c:1817:25: note: each undeclared identifier is reported only once for each function it appears in


vim +/bpf_get_cgroup_classid_curr_proto +1817 kernel/bpf/helpers.c

  1797	
  1798	/* Common helpers for cgroup hooks with valid process context. */
  1799	const struct bpf_func_proto *
  1800	cgroup_current_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
  1801	{
  1802		switch (func_id) {
  1803	#ifdef CONFIG_CGROUPS
  1804		case BPF_FUNC_get_current_uid_gid:
  1805			return &bpf_get_current_uid_gid_proto;
  1806		case BPF_FUNC_get_current_pid_tgid:
  1807			return &bpf_get_current_pid_tgid_proto;
  1808		case BPF_FUNC_get_current_comm:
  1809			return &bpf_get_current_comm_proto;
  1810		case BPF_FUNC_get_current_cgroup_id:
  1811			return &bpf_get_current_cgroup_id_proto;
  1812		case BPF_FUNC_get_current_ancestor_cgroup_id:
  1813			return &bpf_get_current_ancestor_cgroup_id_proto;
  1814	#endif
  1815	#ifdef CONFIG_CGROUP_NET_CLASSID
  1816		case BPF_FUNC_get_cgroup_classid:
> 1817			return &bpf_get_cgroup_classid_curr_proto;
  1818	#endif
  1819		default:
  1820			return NULL;
  1821		}
  1822	}
  1823	

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp

Re: [PATCH bpf-next 2/3] bpf: use cgroup_{common,current}_func_proto in more hooks

[kernel test robot]

Hi Stanislav,

I love your patch! Yet something to improve:

[auto build test ERROR on bpf-next/master]

url:    https://github.com/intel-lab-lkp/linux/commits/Stanislav-Fomichev/bpf-expose-bpf_-g-s-et_retval-to-more-cgroup-hooks/20220813-030615
base:   https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git master
config: i386-randconfig-a004 (https://download.01.org/0day-ci/archive/20220814/202208140239.yjzPuMmv-lkp@intel.com/config)
compiler: clang version 16.0.0 (https://github.com/llvm/llvm-project 3329cec2f79185bafd678f310fafadba2a8c76d2)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/0429824054f7a843ee976d48432e825e493a0a7e
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Stanislav-Fomichev/bpf-expose-bpf_-g-s-et_retval-to-more-cgroup-hooks/20220813-030615
        git checkout 0429824054f7a843ee976d48432e825e493a0a7e
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=i386 SHELL=/bin/bash

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> kernel/bpf/helpers.c:1817:11: error: use of undeclared identifier 'bpf_get_cgroup_classid_curr_proto'
                   return &bpf_get_cgroup_classid_curr_proto;
                           ^
   1 error generated.


vim +/bpf_get_cgroup_classid_curr_proto +1817 kernel/bpf/helpers.c

  1797	
  1798	/* Common helpers for cgroup hooks with valid process context. */
  1799	const struct bpf_func_proto *
  1800	cgroup_current_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
  1801	{
  1802		switch (func_id) {
  1803	#ifdef CONFIG_CGROUPS
  1804		case BPF_FUNC_get_current_uid_gid:
  1805			return &bpf_get_current_uid_gid_proto;
  1806		case BPF_FUNC_get_current_pid_tgid:
  1807			return &bpf_get_current_pid_tgid_proto;
  1808		case BPF_FUNC_get_current_comm:
  1809			return &bpf_get_current_comm_proto;
  1810		case BPF_FUNC_get_current_cgroup_id:
  1811			return &bpf_get_current_cgroup_id_proto;
  1812		case BPF_FUNC_get_current_ancestor_cgroup_id:
  1813			return &bpf_get_current_ancestor_cgroup_id_proto;
  1814	#endif
  1815	#ifdef CONFIG_CGROUP_NET_CLASSID
  1816		case BPF_FUNC_get_cgroup_classid:
> 1817			return &bpf_get_cgroup_classid_curr_proto;
  1818	#endif
  1819		default:
  1820			return NULL;
  1821		}
  1822	}
  1823	

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp

Re: [PATCH bpf-next 2/3] bpf: use cgroup_{common,current}_func_proto in more hooks

[Martin KaFai Lau]

On Fri, Aug 12, 2022 at 12:02:40PM -0700, Stanislav Fomichev wrote:
> diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
> index de7d2fabb06d..87ce47b13b22 100644
> --- a/kernel/bpf/helpers.c
> +++ b/kernel/bpf/helpers.c
> @@ -1764,9 +1764,31 @@ 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_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_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;
> +		}
Does it make sense to have bpf_lsm_func_proto() calling
cgroup_common_func_proto() also?

Re: [PATCH bpf-next 2/3] bpf: use cgroup_{common,current}_func_proto in more hooks

[Stanislav Fomichev]

On Mon, Aug 15, 2022 at 2:58 PM Martin KaFai Lau <kafai@fb.com> wrote:
>
> On Fri, Aug 12, 2022 at 12:02:40PM -0700, Stanislav Fomichev wrote:
> > diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
> > index de7d2fabb06d..87ce47b13b22 100644
> > --- a/kernel/bpf/helpers.c
> > +++ b/kernel/bpf/helpers.c
> > @@ -1764,9 +1764,31 @@ 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_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_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;
> > +             }
> Does it make sense to have bpf_lsm_func_proto() calling
> cgroup_common_func_proto() also?

Oh, sure, will do, thanks!

[PATCH bpf-next 3/3] selftests/bpf: make sure bpf_{g,s}et_retval is exposed everywhere

[Stanislav Fomichev]

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..4a8d2063163d
--- /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, 0)
+BPF_RETVAL_HOOK(egress, "cgroup_skb/egress", __sk_buff, 0)
+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