On Fri, Jun 30, 2023 at 1:34 AM Jiri Olsa <jolsa@xxxxxxxxxx> wrote:
>
> Add extra attach_type checks from link_create under
> bpf_prog_attach_check_attach_type.
>
> Suggested-by: Andrii Nakryiko <andrii@xxxxxxxxxx>
> Signed-off-by: Jiri Olsa <jolsa@xxxxxxxxxx>
> ---
> kernel/bpf/syscall.c | 108 +++++++++++++++++++------------------------
> 1 file changed, 47 insertions(+), 61 deletions(-)
>
> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> index a2aef900519c..9046ad0f9b4e 100644
> --- a/kernel/bpf/syscall.c
> +++ b/kernel/bpf/syscall.c
> @@ -3502,34 +3502,6 @@ static int bpf_raw_tracepoint_open(const union bpf_attr *attr)
> return fd;
> }
>
> -static int bpf_prog_attach_check_attach_type(const struct bpf_prog *prog,
> - enum bpf_attach_type attach_type)
> -{
> - switch (prog->type) {
> - case BPF_PROG_TYPE_CGROUP_SOCK:
> - case BPF_PROG_TYPE_CGROUP_SOCK_ADDR:
> - case BPF_PROG_TYPE_CGROUP_SOCKOPT:
> - case BPF_PROG_TYPE_SK_LOOKUP:
> - return attach_type == prog->expected_attach_type ? 0 : -EINVAL;
> - case BPF_PROG_TYPE_CGROUP_SKB:
> - if (!capable(CAP_NET_ADMIN))
> - /* cg-skb progs can be loaded by unpriv user.
> - * check permissions at attach time.
> - */
> - return -EPERM;
> - return prog->enforce_expected_attach_type &&
> - prog->expected_attach_type != attach_type ?
> - -EINVAL : 0;
> - case BPF_PROG_TYPE_KPROBE:
> - if (prog->expected_attach_type == BPF_TRACE_KPROBE_MULTI &&
> - attach_type != BPF_TRACE_KPROBE_MULTI)
> - return -EINVAL;
> - return 0;
> - default:
> - return 0;
> - }
> -}
> -
> static enum bpf_prog_type
> attach_type_to_prog_type(enum bpf_attach_type attach_type)
> {
> @@ -3593,6 +3565,53 @@ attach_type_to_prog_type(enum bpf_attach_type attach_type)
> }
> }
>
> +static int bpf_prog_attach_check_attach_type(const struct bpf_prog *prog,
> + enum bpf_attach_type attach_type)
> +{
> + enum bpf_prog_type ptype;
> +
> + switch (prog->type) {
> + case BPF_PROG_TYPE_CGROUP_SOCK:
> + case BPF_PROG_TYPE_CGROUP_SOCK_ADDR:
> + case BPF_PROG_TYPE_CGROUP_SOCKOPT:
> + case BPF_PROG_TYPE_SK_LOOKUP:
> + return attach_type == prog->expected_attach_type ? 0 : -EINVAL;
> + case BPF_PROG_TYPE_CGROUP_SKB:
> + if (!capable(CAP_NET_ADMIN))
> + /* cg-skb progs can be loaded by unpriv user.
> + * check permissions at attach time.
> + */
> + return -EPERM;
> + return prog->enforce_expected_attach_type &&
> + prog->expected_attach_type != attach_type ?
> + -EINVAL : 0;
> + case BPF_PROG_TYPE_KPROBE:
nit, I'd keep KPROBE, TRACEPOINT and PERF_EVENT next to each other in
this switch (that will just grow larger over time), as they are pretty
closely related
otherwise lgtm:
Acked-by: Andrii Nakryiko <andrii@xxxxxxxxxx>
> + if (prog->expected_attach_type == BPF_TRACE_KPROBE_MULTI &&
> + attach_type != BPF_TRACE_KPROBE_MULTI)
> + return -EINVAL;
> + if (attach_type != BPF_PERF_EVENT &&
> + attach_type != BPF_TRACE_KPROBE_MULTI)
> + return -EINVAL;
> + return 0;
> + case BPF_PROG_TYPE_EXT:
> + return 0;
> + case BPF_PROG_TYPE_NETFILTER:
> + if (attach_type != BPF_NETFILTER)
> + return -EINVAL;
> + return 0;
> + case BPF_PROG_TYPE_PERF_EVENT:
> + case BPF_PROG_TYPE_TRACEPOINT:
> + if (attach_type != BPF_PERF_EVENT)
> + return -EINVAL;
> + return 0;
> + default:
> + ptype = attach_type_to_prog_type(attach_type);
> + if (ptype == BPF_PROG_TYPE_UNSPEC || ptype != prog->type)
> + return -EINVAL;
> + return 0;
> + }
> +}
> +
> #define BPF_PROG_ATTACH_LAST_FIELD replace_bpf_fd
>
> #define BPF_F_ATTACH_MASK \
> @@ -4658,7 +4677,6 @@ static int bpf_map_do_batch(const union bpf_attr *attr,
> #define BPF_LINK_CREATE_LAST_FIELD link_create.kprobe_multi.cookies
> static int link_create(union bpf_attr *attr, bpfptr_t uattr)
> {
> - enum bpf_prog_type ptype;
> struct bpf_prog *prog;
> int ret;
>
> @@ -4677,38 +4695,6 @@ static int link_create(union bpf_attr *attr, bpfptr_t uattr)
> if (ret)
> goto out;
>
> - switch (prog->type) {
> - case BPF_PROG_TYPE_EXT:
> - break;
> - case BPF_PROG_TYPE_NETFILTER:
> - if (attr->link_create.attach_type != BPF_NETFILTER) {
> - ret = -EINVAL;
> - goto out;
> - }
> - break;
> - case BPF_PROG_TYPE_PERF_EVENT:
> - case BPF_PROG_TYPE_TRACEPOINT:
> - if (attr->link_create.attach_type != BPF_PERF_EVENT) {
> - ret = -EINVAL;
> - goto out;
> - }
> - break;
> - case BPF_PROG_TYPE_KPROBE:
> - if (attr->link_create.attach_type != BPF_PERF_EVENT &&
> - attr->link_create.attach_type != BPF_TRACE_KPROBE_MULTI) {
> - ret = -EINVAL;
> - goto out;
> - }
> - break;
> - default:
> - ptype = attach_type_to_prog_type(attr->link_create.attach_type);
> - if (ptype == BPF_PROG_TYPE_UNSPEC || ptype != prog->type) {
> - ret = -EINVAL;
> - goto out;
> - }
> - break;
> - }
> -
> switch (prog->type) {
> case BPF_PROG_TYPE_CGROUP_SKB:
> case BPF_PROG_TYPE_CGROUP_SOCK:
> --
> 2.41.0
>
