diff mbox series

[v3,bpf-next,07/11] bpf: attach raw_tp program with BTF via type name

Message ID 20191016032505.2089704-8-ast@kernel.org
State Accepted
Delegated to: BPF Maintainers
Headers show
Series bpf: revolutionize bpf tracing | expand

Commit Message

Alexei Starovoitov Oct. 16, 2019, 3:25 a.m. UTC
BTF type id specified at program load time has all
necessary information to attach that program to raw tracepoint.
Use kernel type name to find raw tracepoint.

Add missing CHECK_ATTR() condition.

Signed-off-by: Alexei Starovoitov <ast@kernel.org>
---
There is a tiny chance that CHECK_ATTR() may break some user space.
In such case the CHECK_ATTR change will be reverted.
---
 kernel/bpf/syscall.c | 70 +++++++++++++++++++++++++++++---------------
 1 file changed, 47 insertions(+), 23 deletions(-)

Comments

Andrii Nakryiko Oct. 16, 2019, 8:13 p.m. UTC | #1
On Wed, Oct 16, 2019 at 4:16 AM Alexei Starovoitov <ast@kernel.org> wrote:
>
> BTF type id specified at program load time has all
> necessary information to attach that program to raw tracepoint.
> Use kernel type name to find raw tracepoint.
>
> Add missing CHECK_ATTR() condition.
>
> Signed-off-by: Alexei Starovoitov <ast@kernel.org>
> ---
> There is a tiny chance that CHECK_ATTR() may break some user space.
> In such case the CHECK_ATTR change will be reverted.

Sounds good.

Acked-by: Andrii Nakryiko <andriin@fb.com>

> ---
>  kernel/bpf/syscall.c | 70 +++++++++++++++++++++++++++++---------------
>  1 file changed, 47 insertions(+), 23 deletions(-)
>
> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> index b56c482c9760..523e3ac15a08 100644
> --- a/kernel/bpf/syscall.c
> +++ b/kernel/bpf/syscall.c
> @@ -1816,17 +1816,52 @@ static int bpf_raw_tracepoint_open(const union bpf_attr *attr)
>         struct bpf_raw_tracepoint *raw_tp;
>         struct bpf_raw_event_map *btp;
>         struct bpf_prog *prog;
> -       char tp_name[128];
> +       const char *tp_name;
> +       char buf[128];
>         int tp_fd, err;
>
> -       if (strncpy_from_user(tp_name, u64_to_user_ptr(attr->raw_tracepoint.name),
> -                             sizeof(tp_name) - 1) < 0)
> -               return -EFAULT;
> -       tp_name[sizeof(tp_name) - 1] = 0;
> +       if (CHECK_ATTR(BPF_RAW_TRACEPOINT_OPEN))
> +               return -EINVAL;
> +
> +       prog = bpf_prog_get(attr->raw_tracepoint.prog_fd);
> +       if (IS_ERR(prog))
> +               return PTR_ERR(prog);
> +
> +       if (prog->type != BPF_PROG_TYPE_RAW_TRACEPOINT &&
> +           prog->type != BPF_PROG_TYPE_RAW_TRACEPOINT_WRITABLE) {
> +               err = -EINVAL;
> +               goto out_put_prog;
> +       }
> +
> +       if (prog->type == BPF_PROG_TYPE_RAW_TRACEPOINT &&
> +           prog->aux->attach_btf_id) {
> +               if (attr->raw_tracepoint.name) {
> +                       /* raw_tp name should not be specified in raw_tp
> +                        * programs that were verified via in-kernel BTF info
> +                        */
> +                       err = -EINVAL;
> +                       goto out_put_prog;
> +               }
> +               /* raw_tp name is taken from type name instead */
> +               tp_name = kernel_type_name(prog->aux->attach_btf_id);
> +               /* skip the prefix */
> +               tp_name += sizeof("btf_trace_") - 1;
> +       } else {
> +               if (strncpy_from_user(buf,
> +                                     u64_to_user_ptr(attr->raw_tracepoint.name),
> +                                     sizeof(buf) - 1) < 0) {
> +                       err = -EFAULT;
> +                       goto out_put_prog;
> +               }
> +               buf[sizeof(buf) - 1] = 0;
> +               tp_name = buf;
> +       }
>
>         btp = bpf_get_raw_tracepoint(tp_name);
> -       if (!btp)
> -               return -ENOENT;
> +       if (!btp) {
> +               err = -ENOENT;
> +               goto out_put_prog;
> +       }
>
>         raw_tp = kzalloc(sizeof(*raw_tp), GFP_USER);
>         if (!raw_tp) {
> @@ -1834,38 +1869,27 @@ static int bpf_raw_tracepoint_open(const union bpf_attr *attr)
>                 goto out_put_btp;
>         }
>         raw_tp->btp = btp;
> -
> -       prog = bpf_prog_get(attr->raw_tracepoint.prog_fd);
> -       if (IS_ERR(prog)) {
> -               err = PTR_ERR(prog);
> -               goto out_free_tp;
> -       }
> -       if (prog->type != BPF_PROG_TYPE_RAW_TRACEPOINT &&
> -           prog->type != BPF_PROG_TYPE_RAW_TRACEPOINT_WRITABLE) {
> -               err = -EINVAL;
> -               goto out_put_prog;
> -       }
> +       raw_tp->prog = prog;
>
>         err = bpf_probe_register(raw_tp->btp, prog);
>         if (err)
> -               goto out_put_prog;
> +               goto out_free_tp;
>
> -       raw_tp->prog = prog;
>         tp_fd = anon_inode_getfd("bpf-raw-tracepoint", &bpf_raw_tp_fops, raw_tp,
>                                  O_CLOEXEC);
>         if (tp_fd < 0) {
>                 bpf_probe_unregister(raw_tp->btp, prog);
>                 err = tp_fd;
> -               goto out_put_prog;
> +               goto out_free_tp;
>         }
>         return tp_fd;
>
> -out_put_prog:
> -       bpf_prog_put(prog);
>  out_free_tp:
>         kfree(raw_tp);
>  out_put_btp:
>         bpf_put_raw_tracepoint(btp);
> +out_put_prog:
> +       bpf_prog_put(prog);
>         return err;
>  }
>
> --
> 2.17.1
>
diff mbox series

Patch

diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index b56c482c9760..523e3ac15a08 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -1816,17 +1816,52 @@  static int bpf_raw_tracepoint_open(const union bpf_attr *attr)
 	struct bpf_raw_tracepoint *raw_tp;
 	struct bpf_raw_event_map *btp;
 	struct bpf_prog *prog;
-	char tp_name[128];
+	const char *tp_name;
+	char buf[128];
 	int tp_fd, err;
 
-	if (strncpy_from_user(tp_name, u64_to_user_ptr(attr->raw_tracepoint.name),
-			      sizeof(tp_name) - 1) < 0)
-		return -EFAULT;
-	tp_name[sizeof(tp_name) - 1] = 0;
+	if (CHECK_ATTR(BPF_RAW_TRACEPOINT_OPEN))
+		return -EINVAL;
+
+	prog = bpf_prog_get(attr->raw_tracepoint.prog_fd);
+	if (IS_ERR(prog))
+		return PTR_ERR(prog);
+
+	if (prog->type != BPF_PROG_TYPE_RAW_TRACEPOINT &&
+	    prog->type != BPF_PROG_TYPE_RAW_TRACEPOINT_WRITABLE) {
+		err = -EINVAL;
+		goto out_put_prog;
+	}
+
+	if (prog->type == BPF_PROG_TYPE_RAW_TRACEPOINT &&
+	    prog->aux->attach_btf_id) {
+		if (attr->raw_tracepoint.name) {
+			/* raw_tp name should not be specified in raw_tp
+			 * programs that were verified via in-kernel BTF info
+			 */
+			err = -EINVAL;
+			goto out_put_prog;
+		}
+		/* raw_tp name is taken from type name instead */
+		tp_name = kernel_type_name(prog->aux->attach_btf_id);
+		/* skip the prefix */
+		tp_name += sizeof("btf_trace_") - 1;
+	} else {
+		if (strncpy_from_user(buf,
+				      u64_to_user_ptr(attr->raw_tracepoint.name),
+				      sizeof(buf) - 1) < 0) {
+			err = -EFAULT;
+			goto out_put_prog;
+		}
+		buf[sizeof(buf) - 1] = 0;
+		tp_name = buf;
+	}
 
 	btp = bpf_get_raw_tracepoint(tp_name);
-	if (!btp)
-		return -ENOENT;
+	if (!btp) {
+		err = -ENOENT;
+		goto out_put_prog;
+	}
 
 	raw_tp = kzalloc(sizeof(*raw_tp), GFP_USER);
 	if (!raw_tp) {
@@ -1834,38 +1869,27 @@  static int bpf_raw_tracepoint_open(const union bpf_attr *attr)
 		goto out_put_btp;
 	}
 	raw_tp->btp = btp;
-
-	prog = bpf_prog_get(attr->raw_tracepoint.prog_fd);
-	if (IS_ERR(prog)) {
-		err = PTR_ERR(prog);
-		goto out_free_tp;
-	}
-	if (prog->type != BPF_PROG_TYPE_RAW_TRACEPOINT &&
-	    prog->type != BPF_PROG_TYPE_RAW_TRACEPOINT_WRITABLE) {
-		err = -EINVAL;
-		goto out_put_prog;
-	}
+	raw_tp->prog = prog;
 
 	err = bpf_probe_register(raw_tp->btp, prog);
 	if (err)
-		goto out_put_prog;
+		goto out_free_tp;
 
-	raw_tp->prog = prog;
 	tp_fd = anon_inode_getfd("bpf-raw-tracepoint", &bpf_raw_tp_fops, raw_tp,
 				 O_CLOEXEC);
 	if (tp_fd < 0) {
 		bpf_probe_unregister(raw_tp->btp, prog);
 		err = tp_fd;
-		goto out_put_prog;
+		goto out_free_tp;
 	}
 	return tp_fd;
 
-out_put_prog:
-	bpf_prog_put(prog);
 out_free_tp:
 	kfree(raw_tp);
 out_put_btp:
 	bpf_put_raw_tracepoint(btp);
+out_put_prog:
+	bpf_prog_put(prog);
 	return err;
 }