Re: [PATCH 11/17] bpf/task_iter: In task_file_seq_get_next use fnext_task

Eric W. Biederman
2020-08-18T12:58:32Z

kernel test robot <lkp@intel.com> writes:

> Hi "Eric,
>
> Thank you for the patch! Perhaps something to improve:
>
> [auto build test WARNING on bpf/master]
> [also build test WARNING on linus/master v5.9-rc1 next-20200817]
> [cannot apply to bpf-next/master linux/master]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch]
>
> url: https://github.com/0day-ci/linux/commits/Eric-W-Biederman/exec-Move-unshare_files-to-fix-posix-file-locking-during-exec/20200818-061552
> base: https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf.git master
> config: i386-randconfig-m021-20200818 (attached as .config)
> compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
>
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <lkp@intel.com>
>
> smatch warnings:
> kernel/bpf/task_iter.c:162 task_file_seq_get_next() warn: ignoring unreachable code.

What is smatch warning about?

Perhaps I am blind but I don't see any unreachable code there.

Doh! I see it. That loop will never loop so curr_fd++ is unreachable.
Yes that should get fixed just so the code is readable.

I will change that.

Eric

> 128
> 129 static struct file *
> 130 task_file_seq_get_next(struct bpf_iter_seq_task_file_info *info,
> 131 struct task_struct **task)
> 132 {
> 133 struct pid_namespace *ns = info->common.ns;
> 134 u32 curr_tid = info->tid;
> 135 struct task_struct *curr_task;
> 136 unsigned int curr_fd = info->fd;
> 137
> 138 /* If this function returns a non-NULL file object,
> 139 * it held a reference to the task/file.
> 140 * Otherwise, it does not hold any reference.
> 141 */
> 142 again:
> 143 if (*task) {
> 144 curr_task = *task;
> 145 curr_fd = info->fd;
> 146 } else {
> 147 curr_task = task_seq_get_next(ns, &curr_tid);
> 148 if (!curr_task)
> 149 return NULL;
> 150
> 151 /* set *task and info->tid */
> 152 *task = curr_task;
> 153 if (curr_tid == info->tid) {
> 154 curr_fd = info->fd;
> 155 } else {
> 156 info->tid = curr_tid;
> 157 curr_fd = 0;
> 158 }
> 159 }
> 160
> 161 rcu_read_lock();
> > 162 for (;; curr_fd++) {
> 163 struct file *f;
> 164
> 165 f = fnext_task(curr_task, &curr_fd);
> 166 if (!f)
> 167 break;
> 168
> 169 /* set info->fd */
> 170 info->fd = curr_fd;
> 171 get_file(f);
> 172 rcu_read_unlock();
> 173 return f;
> 174 }
> 175
> 176 /* the current task is done, go to the next task */
> 177 rcu_read_unlock();
> 178 put_task_struct(curr_task);
> 179 *task = NULL;
> 180 info->fd = 0;
> 181 curr_tid = ++(info->tid);
> 182 goto again;
> 183 }
> 184
>
> ---
> 0-DAY CI Kernel Test Service, Intel Corporation
> https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org