diff mbox series

[PATCHv3,bpf-next,01/26] bpf: Add attach_type checks under bpf_prog_attach_check_attach_type

Message ID 20230630083344.984305-2-jolsa@kernel.org (mailing list archive)
State Changes Requested
Delegated to: BPF
Headers show
Series bpf: Add multi uprobe link | expand

Checks

Context Check Description
netdev/series_format fail Series longer than 15 patches (and no cover letter)
netdev/tree_selection success Clearly marked for bpf-next, async
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 10 this patch: 10
netdev/cc_maintainers warning 3 maintainers not CCed: kpsingh@kernel.org martin.lau@linux.dev song@kernel.org
netdev/build_clang fail Errors and warnings before: 32 this patch: 18
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 10 this patch: 10
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 132 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0
bpf/vmtest-bpf-next-PR success PR summary
bpf/vmtest-bpf-next-VM_Test-1 success Logs for ${{ matrix.test }} on ${{ matrix.arch }} with ${{ matrix.toolchain_full }}
bpf/vmtest-bpf-next-VM_Test-2 success Logs for ShellCheck
bpf/vmtest-bpf-next-VM_Test-3 success Logs for build for aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-4 success Logs for build for s390x with gcc
bpf/vmtest-bpf-next-VM_Test-5 success Logs for build for x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-6 fail Logs for build for x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-7 success Logs for set-matrix
bpf/vmtest-bpf-next-VM_Test-8 success Logs for veristat

Commit Message

Jiri Olsa June 30, 2023, 8:33 a.m. UTC 
Add extra attach_type checks from link_create under
bpf_prog_attach_check_attach_type.

Suggested-by: Andrii Nakryiko <andrii@kernel.org>
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 kernel/bpf/syscall.c | 108 +++++++++++++++++++------------------------
 1 file changed, 47 insertions(+), 61 deletions(-)

Comments

Andrii Nakryiko July 6, 2023, 10:34 p.m. UTC | #1 
On Fri, Jun 30, 2023 at 1:34 AM Jiri Olsa <jolsa@kernel.org> wrote:
>
> Add extra attach_type checks from link_create under
> bpf_prog_attach_check_attach_type.
>
> Suggested-by: Andrii Nakryiko <andrii@kernel.org>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
>  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@kernel.org>


> +               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
>
diff mbox series

Patch

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:
+		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: