On Thu, Jul 22, 2021 at 11:30:35PM +0000, Al Viro wrote:

IOW, task->files can be NULL *ONLY* after exit_files().  There are two callers
of that; one is for stillborns in copy_process(), another - in do_exit(),
well past that call of io_uring_files_cancel().  And around that call we have

        if (unlikely(tsk->flags & PF_EXITING)) {
		pr_alert("Fixing recursive fault but reboot is needed!\n");
		futex_exit_recursive(tsk);
		set_current_state(TASK_UNINTERRUPTIBLE);
		schedule();
	}
        io_uring_files_cancel(tsk->files);
	exit_signals(tsk);  /* sets PF_EXITING */

So how can we possibly get there with tsk->files == NULL and what does it
have to do with files, anyway?

PS: processes with ->files == NULL can be observed; e.g. access via procfs
can very well race with exit().  If procfs acquires task_struct reference
before exit(), the object won't get freed until we do put_task_struct().
However, the process in question can get through the entire do_exit(),
become a zombie, be successfull reaped, etc., so its state can be very
thoroughly taken apart while procfs tries to access it.

There the checks for tsk->files == NULL are meaningful; doing them for
current, OTOH, is basically asking "am I rather deep into do_exit()?"

	Once upon a time we had exit_files() done kernel threads.
Not for the last 9 years since 864bdb3b6cbd ("new helper:
daemonize_descriptors()"), though (and shortly after that the entire
daemonize() thing has disappeared - kernel threads are spawned by
kthreadd, and inherit ->files from it just fine).

	Should've killed the useless checks for NULL ->files at the same
time, hadn't...  FWIW, the checks in fget_task(), task_lookup_fd_rcu(),
task_lookup_next_fd_rcu(), task_state(), fs/proc/fd.c:seq_show()
and iterate_fd() are there for good reason.  The ones in unshare_fd(),
copy_files(), fs/proc/task_nommu.c:task_mem() and in exit_files() itself
are noise.  I'll throw their removal in vfs.git#work.misc...

	Anyway, if you intended to check for some(?) kernel threads,
that place needs fixing.  If not, I'd suggest just passing a boolean
to that thing (and giving it less confusing name)...