On Fri, Aug 19, 2022 at 3:09 PM Kui-Feng Lee <kuifeng@xxxxxx> wrote:
>
> Allow creating an iterator that loops through resources of one task/thread.
>
> People could only create iterators to loop through all resources of
> files, vma, and tasks in the system, even though they were interested
> in only the resources of a specific task or process. Passing the
> additional parameters, people can now create an iterator to go
> through all resources or only the resources of a task.
>
> Signed-off-by: Kui-Feng Lee <kuifeng@xxxxxx>
> ---
> include/linux/bpf.h | 25 +++++++
> include/uapi/linux/bpf.h | 6 ++
> kernel/bpf/task_iter.c | 116 ++++++++++++++++++++++++++-------
> tools/include/uapi/linux/bpf.h | 6 ++
> 4 files changed, 129 insertions(+), 24 deletions(-)
>
> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> index 39bd36359c1e..59712dd917d8 100644
> --- a/include/linux/bpf.h
> +++ b/include/linux/bpf.h
> @@ -1729,8 +1729,33 @@ int bpf_obj_get_user(const char __user *pathname, int flags);
> extern int bpf_iter_ ## target(args); \
> int __init bpf_iter_ ## target(args) { return 0; }
>
> +/*
> + * The task type of iterators.
> + *
> + * For BPF task iterators, they can be parameterized with various
> + * parameters to visit only some of tasks.
> + *
> + * BPF_TASK_ITER_ALL (default)
> + * Iterate over resources of every task.
> + *
> + * BPF_TASK_ITER_TID
> + * Iterate over resources of a task/tid.
> + *
> + * BPF_TASK_ITER_TGID
> + * Iterate over reosurces of evevry task of a process / task group.
typos: resources, every
> + */
> +enum bpf_iter_task_type {
> + BPF_TASK_ITER_ALL = 0,
> + BPF_TASK_ITER_TID,
> + BPF_TASK_ITER_TGID,
> +};
> +
[...]
> rcu_read_lock();
> retry:
> - pid = find_ge_pid(*tid, ns);
> + pid = find_ge_pid(*tid, common->ns);
> if (pid) {
> - *tid = pid_nr_ns(pid, ns);
> + *tid = pid_nr_ns(pid, common->ns);
> task = get_pid_task(pid, PIDTYPE_PID);
> if (!task) {
> ++*tid;
> goto retry;
> - } else if (skip_if_dup_files && !thread_group_leader(task) &&
> - task->files == task->group_leader->files) {
> + } else if ((skip_if_dup_files && !thread_group_leader(task) &&
> + task->files == task->group_leader->files) ||
> + (common->type == BPF_TASK_ITER_TGID &&
> + __task_pid_nr_ns(task, PIDTYPE_TGID, common->ns) != common->pid)) {
it gets super hard to follow this logic, would a simple helper
function to calculate this condition (and maybe some comments to
explain the logic behind these checks?) make it a bit more readable?
> put_task_struct(task);
> task = NULL;
> ++*tid;
> @@ -56,7 +73,7 @@ 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;
>
> - task = task_seq_get_next(info->common.ns, &info->tid, false);
> + task = task_seq_get_next(&info->common, &info->tid, false);
> if (!task)
> return NULL;
>
> @@ -73,7 +90,7 @@ static void *task_seq_next(struct seq_file *seq, void *v, loff_t *pos)
> ++*pos;
> ++info->tid;
> put_task_struct((struct task_struct *)v);
> - task = task_seq_get_next(info->common.ns, &info->tid, false);
> + task = task_seq_get_next(&info->common, &info->tid, false);
> if (!task)
> return NULL;
>
> @@ -117,6 +134,48 @@ static void task_seq_stop(struct seq_file *seq, void *v)
> put_task_struct((struct task_struct *)v);
> }
>
> +static int bpf_iter_attach_task(struct bpf_prog *prog,
> + union bpf_iter_link_info *linfo,
> + struct bpf_iter_aux_info *aux)
> +{
> + unsigned int flags;
> + struct pid_namespace *ns;
> + struct pid *pid;
> + pid_t tgid;
it seems it would be simpler to first check that at most one of
tid/pid/pid_fd is set instead of making sure that aux->task.type
wasn't already set.
How about
if (!!linfo->task.tid + !!linfo->task.pid + !!linfo->task.pid_fd > 1)
return -EINVAL;
?
> +
> + aux->task.type = BPF_TASK_ITER_ALL;
> + if (linfo->task.tid != 0) {
> + aux->task.type = BPF_TASK_ITER_TID;
> + aux->task.pid = linfo->task.tid;
> + }
> + if (linfo->task.pid != 0) {
> + if (aux->task.type != BPF_TASK_ITER_ALL)
> + return -EINVAL;
> +
> + aux->task.type = BPF_TASK_ITER_TGID;
> + aux->task.pid = linfo->task.pid;
> + }
> + if (linfo->task.pid_fd != 0) {
> + if (aux->task.type != BPF_TASK_ITER_ALL)
> + return -EINVAL;
> +
> + aux->task.type = BPF_TASK_ITER_TGID;
> + ns = task_active_pid_ns(current);
> + if (IS_ERR(ns))
> + return PTR_ERR(ns);
> +
> + pid = pidfd_get_pid(linfo->task.pid_fd, &flags);
> + if (IS_ERR(pid))
> + return PTR_ERR(pid);
> +
> + tgid = pid_nr_ns(pid, ns);
> + aux->task.pid = tgid;
> + put_pid(pid);
> + }
> +
> + return 0;
> +}
> +
[...]
