| Message ID | 20200427201247.2995622-1-yhs@fb.com |
|---|---|
| State | Changes Requested |
| Delegated to: | BPF Maintainers |
| Headers | show |
| Series | bpf: implement bpf iterator for kernel data | expand |
On Mon, Apr 27, 2020 at 1:17 PM Yonghong Song <yhs@fb.com> wrote: > > Only the tasks belonging to "current" pid namespace > are enumerated. > > For task/file target, the bpf program will have access to > struct task_struct *task > u32 fd > struct file *file > where fd/file is an open file for the task. > > Signed-off-by: Yonghong Song <yhs@fb.com> > --- > kernel/bpf/Makefile | 2 +- > kernel/bpf/task_iter.c | 319 +++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 320 insertions(+), 1 deletion(-) > create mode 100644 kernel/bpf/task_iter.c > [...] > +static void *task_seq_start(struct seq_file *seq, loff_t *pos) > +{ > + struct bpf_iter_seq_task_info *info = seq->private; > + struct task_struct *task; > + u32 id = info->id; > + > + if (*pos == 0) > + info->ns = task_active_pid_ns(current); I wonder why pid namespace is set in start() callback each time, while net_ns was set once when seq_file is created. I think it should be consistent, no? Either pid_ns is another feature and is set consistently just once using the context of the process that creates seq_file, or net_ns could be set using the same method without bpf_iter infra knowing about this feature? Or there are some non-obvious aspects which make pid_ns easier to work with? Either way, process read()'ing seq_file might be different than process open()'ing seq_file, so they might have different namespaces. We need to decide explicitly which context should be used and do it consistently. > + > + task = task_seq_get_next(info->ns, &id); > + if (!task) > + return NULL; > + > + ++*pos; > + info->task = task; > + info->id = id; > + > + return task; > +} > + > +static void *task_seq_next(struct seq_file *seq, void *v, loff_t *pos) > +{ > + struct bpf_iter_seq_task_info *info = seq->private; > + struct task_struct *task; > + > + ++*pos; > + ++info->id; this would make iterator skip pid 0? Is that by design? > + task = task_seq_get_next(info->ns, &info->id); > + if (!task) > + return NULL; > + > + put_task_struct(info->task); on very first iteration info->task might be NULL, right? > + info->task = task; > + return task; > +} > + > +struct bpf_iter__task { > + __bpf_md_ptr(struct bpf_iter_meta *, meta); > + __bpf_md_ptr(struct task_struct *, task); > +}; > + > +int __init __bpf_iter__task(struct bpf_iter_meta *meta, struct task_struct *task) > +{ > + return 0; > +} > + > +static int task_seq_show(struct seq_file *seq, void *v) > +{ > + struct bpf_iter_meta meta; > + struct bpf_iter__task ctx; > + struct bpf_prog *prog; > + int ret = 0; > + > + prog = bpf_iter_get_prog(seq, sizeof(struct bpf_iter_seq_task_info), > + &meta.session_id, &meta.seq_num, > + v == (void *)0); > + if (prog) { can it happen that prog is NULL? > + meta.seq = seq; > + ctx.meta = &meta; > + ctx.task = v; > + ret = bpf_iter_run_prog(prog, &ctx); > + } > + > + return ret == 0 ? 0 : -EINVAL; > +} > + > +static void task_seq_stop(struct seq_file *seq, void *v) > +{ > + struct bpf_iter_seq_task_info *info = seq->private; > + > + if (!v) > + task_seq_show(seq, v); hmm... show() called from stop()? what's the case where this is necessary? > + > + if (info->task) { > + put_task_struct(info->task); > + info->task = NULL; > + } > +} > + [...]
On 4/29/20 7:08 PM, Andrii Nakryiko wrote: > On Mon, Apr 27, 2020 at 1:17 PM Yonghong Song <yhs@fb.com> wrote: >> >> Only the tasks belonging to "current" pid namespace >> are enumerated. >> >> For task/file target, the bpf program will have access to >> struct task_struct *task >> u32 fd >> struct file *file >> where fd/file is an open file for the task. >> >> Signed-off-by: Yonghong Song <yhs@fb.com> >> --- >> kernel/bpf/Makefile | 2 +- >> kernel/bpf/task_iter.c | 319 +++++++++++++++++++++++++++++++++++++++++ >> 2 files changed, 320 insertions(+), 1 deletion(-) >> create mode 100644 kernel/bpf/task_iter.c >> > > [...] > >> +static void *task_seq_start(struct seq_file *seq, loff_t *pos) >> +{ >> + struct bpf_iter_seq_task_info *info = seq->private; >> + struct task_struct *task; >> + u32 id = info->id; >> + >> + if (*pos == 0) >> + info->ns = task_active_pid_ns(current); > > I wonder why pid namespace is set in start() callback each time, while > net_ns was set once when seq_file is created. I think it should be > consistent, no? Either pid_ns is another feature and is set > consistently just once using the context of the process that creates > seq_file, or net_ns could be set using the same method without > bpf_iter infra knowing about this feature? Or there are some > non-obvious aspects which make pid_ns easier to work with? > > Either way, process read()'ing seq_file might be different than > process open()'ing seq_file, so they might have different namespaces. > We need to decide explicitly which context should be used and do it > consistently. Good point. for networking case, the `net` namespace is locked at seq_file open stage and later on it is used for seq_read(). I think I should do the same thing, locking down pid namespace at open. > >> + >> + task = task_seq_get_next(info->ns, &id); >> + if (!task) >> + return NULL; >> + >> + ++*pos; >> + info->task = task; >> + info->id = id; >> + >> + return task; >> +} >> + >> +static void *task_seq_next(struct seq_file *seq, void *v, loff_t *pos) >> +{ >> + struct bpf_iter_seq_task_info *info = seq->private; >> + struct task_struct *task; >> + >> + ++*pos; >> + ++info->id; > > this would make iterator skip pid 0? Is that by design? The start will try to find pid 0. That means start will never return SEQ_START_TOKEN since the bpf program won't be called any way. > >> + task = task_seq_get_next(info->ns, &info->id); >> + if (!task) >> + return NULL; >> + >> + put_task_struct(info->task); > > on very first iteration info->task might be NULL, right? Even the first iteration info->task is not NULL. The start() will forcefully try to find the first real task from idr number 0. > >> + info->task = task; >> + return task; >> +} >> + >> +struct bpf_iter__task { >> + __bpf_md_ptr(struct bpf_iter_meta *, meta); >> + __bpf_md_ptr(struct task_struct *, task); >> +}; >> + >> +int __init __bpf_iter__task(struct bpf_iter_meta *meta, struct task_struct *task) >> +{ >> + return 0; >> +} >> + >> +static int task_seq_show(struct seq_file *seq, void *v) >> +{ >> + struct bpf_iter_meta meta; >> + struct bpf_iter__task ctx; >> + struct bpf_prog *prog; >> + int ret = 0; >> + >> + prog = bpf_iter_get_prog(seq, sizeof(struct bpf_iter_seq_task_info), >> + &meta.session_id, &meta.seq_num, >> + v == (void *)0); >> + if (prog) { > > can it happen that prog is NULL? Yes, this function is shared between show() and stop(). The stop() function might be called multiple times since user can repeatedly try read() although there is nothing there, in which case, the seq_ops will be just start() and stop(). > > >> + meta.seq = seq; >> + ctx.meta = &meta; >> + ctx.task = v; >> + ret = bpf_iter_run_prog(prog, &ctx); >> + } >> + >> + return ret == 0 ? 0 : -EINVAL; >> +} >> + >> +static void task_seq_stop(struct seq_file *seq, void *v) >> +{ >> + struct bpf_iter_seq_task_info *info = seq->private; >> + >> + if (!v) >> + task_seq_show(seq, v); > > hmm... show() called from stop()? what's the case where this is necessary? I will refactor it better. This is to invoke bpf program in stop() with NULL object to signal the end of iteration. >> + >> + if (info->task) { >> + put_task_struct(info->task); >> + info->task = NULL; >> + } >> +} >> + > > [...] >
On Fri, May 1, 2020 at 10:23 AM Yonghong Song <yhs@fb.com> wrote: > > > > On 4/29/20 7:08 PM, Andrii Nakryiko wrote: > > On Mon, Apr 27, 2020 at 1:17 PM Yonghong Song <yhs@fb.com> wrote: > >> > >> Only the tasks belonging to "current" pid namespace > >> are enumerated. > >> > >> For task/file target, the bpf program will have access to > >> struct task_struct *task > >> u32 fd > >> struct file *file > >> where fd/file is an open file for the task. > >> > >> Signed-off-by: Yonghong Song <yhs@fb.com> > >> --- > >> kernel/bpf/Makefile | 2 +- > >> kernel/bpf/task_iter.c | 319 +++++++++++++++++++++++++++++++++++++++++ > >> 2 files changed, 320 insertions(+), 1 deletion(-) > >> create mode 100644 kernel/bpf/task_iter.c > >> > > > > [...] > > > >> +static void *task_seq_start(struct seq_file *seq, loff_t *pos) > >> +{ > >> + struct bpf_iter_seq_task_info *info = seq->private; > >> + struct task_struct *task; > >> + u32 id = info->id; > >> + > >> + if (*pos == 0) > >> + info->ns = task_active_pid_ns(current); > > > > I wonder why pid namespace is set in start() callback each time, while > > net_ns was set once when seq_file is created. I think it should be > > consistent, no? Either pid_ns is another feature and is set > > consistently just once using the context of the process that creates > > seq_file, or net_ns could be set using the same method without > > bpf_iter infra knowing about this feature? Or there are some > > non-obvious aspects which make pid_ns easier to work with? > > > > Either way, process read()'ing seq_file might be different than > > process open()'ing seq_file, so they might have different namespaces. > > We need to decide explicitly which context should be used and do it > > consistently. > > Good point. for networking case, the `net` namespace is locked > at seq_file open stage and later on it is used for seq_read(). > > I think I should do the same thing, locking down pid namespace > at open. Yeah, I think it's a good idea. > > > > >> + > >> + task = task_seq_get_next(info->ns, &id); > >> + if (!task) > >> + return NULL; > >> + > >> + ++*pos; > >> + info->task = task; > >> + info->id = id; > >> + > >> + return task; > >> +} > >> + > >> +static void *task_seq_next(struct seq_file *seq, void *v, loff_t *pos) > >> +{ > >> + struct bpf_iter_seq_task_info *info = seq->private; > >> + struct task_struct *task; > >> + > >> + ++*pos; > >> + ++info->id; > > > > this would make iterator skip pid 0? Is that by design? > > The start will try to find pid 0. That means start will never > return SEQ_START_TOKEN since the bpf program won't be called any way. Never mind, I confused task_seq_next() and task_seq_get_next() :) > > > > >> + task = task_seq_get_next(info->ns, &info->id); > >> + if (!task) > >> + return NULL; > >> + > >> + put_task_struct(info->task); > > > > on very first iteration info->task might be NULL, right? > > Even the first iteration info->task is not NULL. The start() > will forcefully try to find the first real task from idr number 0. > Right, goes to same confusion as above, sorry. > > > >> + info->task = task; > >> + return task; > >> +} > >> + > >> +struct bpf_iter__task { > >> + __bpf_md_ptr(struct bpf_iter_meta *, meta); > >> + __bpf_md_ptr(struct task_struct *, task); > >> +}; > >> + > >> +int __init __bpf_iter__task(struct bpf_iter_meta *meta, struct task_struct *task) > >> +{ > >> + return 0; > >> +} > >> + > >> +static int task_seq_show(struct seq_file *seq, void *v) > >> +{ > >> + struct bpf_iter_meta meta; > >> + struct bpf_iter__task ctx; > >> + struct bpf_prog *prog; > >> + int ret = 0; > >> + > >> + prog = bpf_iter_get_prog(seq, sizeof(struct bpf_iter_seq_task_info), > >> + &meta.session_id, &meta.seq_num, > >> + v == (void *)0); > >> + if (prog) { > > > > can it happen that prog is NULL? > > Yes, this function is shared between show() and stop(). > The stop() function might be called multiple times since > user can repeatedly try read() although there is nothing > there, in which case, the seq_ops will be just > start() and stop(). Ah, right, NULL case after end of iteration, got it. > > > > > > >> + meta.seq = seq; > >> + ctx.meta = &meta; > >> + ctx.task = v; > >> + ret = bpf_iter_run_prog(prog, &ctx); > >> + } > >> + > >> + return ret == 0 ? 0 : -EINVAL; > >> +} > >> + > >> +static void task_seq_stop(struct seq_file *seq, void *v) > >> +{ > >> + struct bpf_iter_seq_task_info *info = seq->private; > >> + > >> + if (!v) > >> + task_seq_show(seq, v); > > > > hmm... show() called from stop()? what's the case where this is necessary? > > I will refactor it better. This is to invoke bpf program > in stop() with NULL object to signal the end of > iteration. > > >> + > >> + if (info->task) { > >> + put_task_struct(info->task); > >> + info->task = NULL; > >> + } > >> +} > >> + > > > > [...] > >
diff --git a/kernel/bpf/Makefile b/kernel/bpf/Makefile index b2b5eefc5254..37b2d8620153 100644 --- a/kernel/bpf/Makefile +++ b/kernel/bpf/Makefile @@ -2,7 +2,7 @@ obj-y := core.o CFLAGS_core.o += $(call cc-disable-warning, override-init) -obj-$(CONFIG_BPF_SYSCALL) += syscall.o verifier.o inode.o helpers.o tnum.o bpf_iter.o map_iter.o +obj-$(CONFIG_BPF_SYSCALL) += syscall.o verifier.o inode.o helpers.o tnum.o bpf_iter.o map_iter.o task_iter.o obj-$(CONFIG_BPF_SYSCALL) += hashtab.o arraymap.o percpu_freelist.o bpf_lru_list.o lpm_trie.o map_in_map.o obj-$(CONFIG_BPF_SYSCALL) += local_storage.o queue_stack_maps.o obj-$(CONFIG_BPF_SYSCALL) += disasm.o diff --git a/kernel/bpf/task_iter.c b/kernel/bpf/task_iter.c new file mode 100644 index 000000000000..ee29574e427d --- /dev/null +++ b/kernel/bpf/task_iter.c @@ -0,0 +1,319 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* Copyright (c) 2020 Facebook */ + +#include <linux/init.h> +#include <linux/namei.h> +#include <linux/pid_namespace.h> +#include <linux/fs.h> +#include <linux/fdtable.h> +#include <linux/filter.h> + +struct bpf_iter_seq_task_info { + struct pid_namespace *ns; + struct task_struct *task; + u32 id; +}; + +static struct task_struct *task_seq_get_next(struct pid_namespace *ns, u32 *id) +{ + struct task_struct *task = NULL; + struct pid *pid; + + rcu_read_lock(); + pid = idr_get_next(&ns->idr, id); + if (pid) + task = get_pid_task(pid, PIDTYPE_PID); + rcu_read_unlock(); + + return task; +} + +static void *task_seq_start(struct seq_file *seq, loff_t *pos) +{ + struct bpf_iter_seq_task_info *info = seq->private; + struct task_struct *task; + u32 id = info->id; + + if (*pos == 0) + info->ns = task_active_pid_ns(current); + + task = task_seq_get_next(info->ns, &id); + if (!task) + return NULL; + + ++*pos; + info->task = task; + info->id = id; + + return task; +} + +static void *task_seq_next(struct seq_file *seq, void *v, loff_t *pos) +{ + struct bpf_iter_seq_task_info *info = seq->private; + struct task_struct *task; + + ++*pos; + ++info->id; + task = task_seq_get_next(info->ns, &info->id); + if (!task) + return NULL; + + put_task_struct(info->task); + info->task = task; + return task; +} + +struct bpf_iter__task { + __bpf_md_ptr(struct bpf_iter_meta *, meta); + __bpf_md_ptr(struct task_struct *, task); +}; + +int __init __bpf_iter__task(struct bpf_iter_meta *meta, struct task_struct *task) +{ + return 0; +} + +static int task_seq_show(struct seq_file *seq, void *v) +{ + struct bpf_iter_meta meta; + struct bpf_iter__task ctx; + struct bpf_prog *prog; + int ret = 0; + + prog = bpf_iter_get_prog(seq, sizeof(struct bpf_iter_seq_task_info), + &meta.session_id, &meta.seq_num, + v == (void *)0); + if (prog) { + meta.seq = seq; + ctx.meta = &meta; + ctx.task = v; + ret = bpf_iter_run_prog(prog, &ctx); + } + + return ret == 0 ? 0 : -EINVAL; +} + +static void task_seq_stop(struct seq_file *seq, void *v) +{ + struct bpf_iter_seq_task_info *info = seq->private; + + if (!v) + task_seq_show(seq, v); + + if (info->task) { + put_task_struct(info->task); + info->task = NULL; + } +} + +static const struct seq_operations task_seq_ops = { + .start = task_seq_start, + .next = task_seq_next, + .stop = task_seq_stop, + .show = task_seq_show, +}; + +struct bpf_iter_seq_task_file_info { + struct pid_namespace *ns; + struct task_struct *task; + struct files_struct *files; + u32 id; + u32 fd; +}; + +static struct file *task_file_seq_get_next(struct pid_namespace *ns, u32 *id, + int *fd, struct task_struct **task, + struct files_struct **fstruct) +{ + struct files_struct *files; + struct task_struct *tk; + u32 sid = *id; + int sfd; + + /* If this function returns a non-NULL file object, + * it held a reference to the files_struct and file. + * Otherwise, it does not hold any reference. + */ +again: + if (*fstruct) { + files = *fstruct; + sfd = *fd; + } else { + tk = task_seq_get_next(ns, &sid); + if (!tk) + return NULL; + + files = get_files_struct(tk); + put_task_struct(tk); + if (!files) { + sid = ++(*id); + *fd = 0; + goto again; + } + *fstruct = files; + *task = tk; + if (sid == *id) { + sfd = *fd; + } else { + *id = sid; + sfd = 0; + } + } + + rcu_read_lock(); + for (; sfd < files_fdtable(files)->max_fds; sfd++) { + struct file *f; + + f = fcheck_files(files, sfd); + if (!f) + continue; + *fd = sfd; + get_file(f); + rcu_read_unlock(); + return f; + } + + /* the current task is done, go to the next task */ + rcu_read_unlock(); + put_files_struct(files); + *fstruct = NULL; + sid = ++(*id); + *fd = 0; + goto again; +} + +static void *task_file_seq_start(struct seq_file *seq, loff_t *pos) +{ + struct bpf_iter_seq_task_file_info *info = seq->private; + struct files_struct *files = NULL; + struct task_struct *task = NULL; + struct file *file; + u32 id = info->id; + int fd = info->fd; + + if (*pos == 0) + info->ns = task_active_pid_ns(current); + + file = task_file_seq_get_next(info->ns, &id, &fd, &task, &files); + if (!file) { + info->files = NULL; + return NULL; + } + + ++*pos; + info->id = id; + info->fd = fd; + info->task = task; + info->files = files; + + return file; +} + +static void *task_file_seq_next(struct seq_file *seq, void *v, loff_t *pos) +{ + struct bpf_iter_seq_task_file_info *info = seq->private; + struct files_struct *files = info->files; + struct task_struct *task = info->task; + struct file *file; + u32 id = info->id; + + ++*pos; + ++info->fd; + fput((struct file *)v); + file = task_file_seq_get_next(info->ns, &id, &info->fd, &task, &files); + if (!file) { + info->files = NULL; + return NULL; + } + + info->id = id; + info->task = task; + info->files = files; + + return file; +} + +struct bpf_iter__task_file { + __bpf_md_ptr(struct bpf_iter_meta *, meta); + __bpf_md_ptr(struct task_struct *, task); + u32 fd; + __bpf_md_ptr(struct file *, file); +}; + +int __init __bpf_iter__task_file(struct bpf_iter_meta *meta, + struct task_struct *task, u32 fd, + struct file *file) +{ + return 0; +} + +static int task_file_seq_show(struct seq_file *seq, void *v) +{ + struct bpf_iter_seq_task_file_info *info = seq->private; + struct bpf_iter__task_file ctx; + struct bpf_iter_meta meta; + struct bpf_prog *prog; + int ret = 0; + + prog = bpf_iter_get_prog(seq, sizeof(struct bpf_iter_seq_task_file_info), + &meta.session_id, &meta.seq_num, v == (void *)0); + if (prog) { + meta.seq = seq; + ctx.meta = &meta; + ctx.task = info->task; + ctx.fd = info->fd; + ctx.file = v; + ret = bpf_iter_run_prog(prog, &ctx); + } + + return ret == 0 ? 0 : -EINVAL; +} + +static void task_file_seq_stop(struct seq_file *seq, void *v) +{ + struct bpf_iter_seq_task_file_info *info = seq->private; + + if (v) + fput((struct file *)v); + else + task_file_seq_show(seq, v); + + if (info->files) { + put_files_struct(info->files); + info->files = NULL; + } +} + +static const struct seq_operations task_file_seq_ops = { + .start = task_file_seq_start, + .next = task_file_seq_next, + .stop = task_file_seq_stop, + .show = task_file_seq_show, +}; + +static int __init task_iter_init(void) +{ + struct bpf_iter_reg task_file_reg_info = { + .target = "task_file", + .target_func_name = "__bpf_iter__task_file", + .seq_ops = &task_file_seq_ops, + .seq_priv_size = sizeof(struct bpf_iter_seq_task_file_info), + .target_feature = 0, + }; + struct bpf_iter_reg task_reg_info = { + .target = "task", + .target_func_name = "__bpf_iter__task", + .seq_ops = &task_seq_ops, + .seq_priv_size = sizeof(struct bpf_iter_seq_task_info), + .target_feature = 0, + }; + int ret; + + ret = bpf_iter_reg_target(&task_reg_info); + if (ret) + return ret; + + return bpf_iter_reg_target(&task_file_reg_info); +} +late_initcall(task_iter_init);
Only the tasks belonging to "current" pid namespace are enumerated. For task/file target, the bpf program will have access to struct task_struct *task u32 fd struct file *file where fd/file is an open file for the task. Signed-off-by: Yonghong Song <yhs@fb.com> --- kernel/bpf/Makefile | 2 +- kernel/bpf/task_iter.c | 319 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 320 insertions(+), 1 deletion(-) create mode 100644 kernel/bpf/task_iter.c