On Wed, Aug 10, 2022 at 05:16:52PM -0700, Kui-Feng Lee wrote:
SNIP
> +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;
> +
> + if (linfo->task.tid != 0) {
> + aux->task.type = BPF_TASK_ITER_TID;
> + aux->task.tid = linfo->task.tid;
> + } else if (linfo->task.tgid != 0) {
> + aux->task.type = BPF_TASK_ITER_TGID;
> + aux->task.tgid = linfo->task.tgid;
> + } else if (linfo->task.pid_fd != 0) {
> + aux->task.type = BPF_TASK_ITER_TGID;
> + pid = pidfd_get_pid(linfo->task.pid_fd, &flags);
> + if (IS_ERR(pid))
> + return PTR_ERR(pid);
> +
> + ns = task_active_pid_ns(current);
> + if (IS_ERR(ns))
> + return PTR_ERR(ns);
> +
> + tgid = pid_nr_ns(pid, ns);
> + if (tgid <= 0)
> + return -EINVAL;
> +
> + aux->task.tgid = tgid;
> + } else {
> + aux->task.type = BPF_TASK_ITER_ALL;
> + }
> +
> + return 0;
> +}
> +
> static const struct seq_operations task_seq_ops = {
> .start = task_seq_start,
> .next = task_seq_next,
> @@ -137,8 +198,7 @@ struct bpf_iter_seq_task_file_info {
> static struct file *
> task_file_seq_get_next(struct bpf_iter_seq_task_file_info *info)
> {
> - struct pid_namespace *ns = info->common.ns;
> - u32 curr_tid = info->tid;
> + u32 saved_tid = info->tid;
we use 'curr_' prefix for other stuff in the function, like
curr_task, curr_fd .. I think we should either change all of
them or actually keep curr_tid, which seem better to me
jirka
> struct task_struct *curr_task;
> unsigned int curr_fd = info->fd;
>
> @@ -151,21 +211,18 @@ task_file_seq_get_next(struct bpf_iter_seq_task_file_info *info)
> curr_task = info->task;
> curr_fd = info->fd;
> } else {
> - curr_task = task_seq_get_next(ns, &curr_tid, true);
> + curr_task = task_seq_get_next(&info->common, &info->tid, true);
> if (!curr_task) {
> info->task = NULL;
> - info->tid = curr_tid;
> return NULL;
> }
>
> - /* set info->task and info->tid */
> + /* set info->task */
> info->task = curr_task;
> - if (curr_tid == info->tid) {
> + if (saved_tid == info->tid)
> curr_fd = info->fd;
> - } else {
> - info->tid = curr_tid;
> + else
> curr_fd = 0;
> - }
> }
>
SNIP
