Commit f52c8947 authored by Alexei Starovoitov's avatar Alexei Starovoitov
Browse files

Merge branch 'bpf: expose bpf_{g,s}et_retval to more cgroup hooks' 



Stanislav Fomichev says:

====================

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.

v5:
- remove net/cls_cgroup.h include from patch 1/5 (Martin)
- move endif changes from patch 1/5 to 3/5 (Martin)
- don't define __weak protos, the ones in core.c suffice (Martin)

v4:
- don't touch existing helper.c helpers (Martin)
- drop unneeded CONFIG_CGROUP_BPF in bpf_lsm_func_proto (Martin)

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)
====================

Signed-off-by: default avatarAlexei Starovoitov <ast@kernel.org>
parents 92ec1cc3 e7215f57

include/linux/bpf-cgroup.h

+17 −0
Original line number Diff line number Diff line
@@ -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(

include/linux/bpf.h

+1 −0
Original line number Diff line number Diff line
@@ -2375,6 +2375,7 @@ extern const struct bpf_func_proto bpf_sock_map_update_proto; 
extern const struct bpf_func_proto bpf_sock_hash_update_proto;

extern const struct bpf_func_proto bpf_get_current_cgroup_id_proto;

extern const struct bpf_func_proto bpf_get_current_ancestor_cgroup_id_proto;

extern const struct bpf_func_proto bpf_get_cgroup_classid_curr_proto;

extern const struct bpf_func_proto bpf_msg_redirect_hash_proto;

extern const struct bpf_func_proto bpf_msg_redirect_map_proto;

extern const struct bpf_func_proto bpf_sk_redirect_hash_proto;

include/uapi/linux/bpf.h

+17 −5
Original line number Diff line number Diff line
@@ -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.

 *

kernel/bpf/bpf_lsm.c

+8 −9
Original line number Diff line number Diff line
@@ -189,6 +189,14 @@ 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)

{

	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;

	}



	switch (func_id) {

	case BPF_FUNC_inode_storage_get:

		return &bpf_inode_storage_get_proto;
@@ -212,15 +220,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)

kernel/bpf/cgroup.c

+134 −23
Original line number Diff line number Diff line
@@ -1527,6 +1527,37 @@ int __cgroup_bpf_check_dev_permission(short dev_type, u32 major, u32 minor, 
	return ret;

}



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 =
@@ -1558,32 +1589,26 @@ const struct bpf_func_proto bpf_set_retval_proto = { 
};



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,11 +2121,17 @@ 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;

	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:
@@ -2111,8 +2142,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 +2266,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 +2297,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 +2465,69 @@ 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:

		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:

		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;

	}

}



/* 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_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;

	}

}