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

[Eric W. Biederman]

kernel test robot <lkp@xxxxxxxxx> 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@xxxxxxxxx>
>
> 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@xxxxxxxxxxxx