Re: [PATCH 3/3] io_uring: refactor io_sq_offload_create()

[Al Viro <viro@zeniv.linux.org.uk>]

On Thu, Jul 22, 2021 at 05:06:24PM -0600, Jens Axboe wrote:

> But yes, that is not great and obviously a bug, and we'll of course get
> it fixed up asap.

Another fun question: in do_exit() you have
        io_uring_files_cancel(tsk->files);

with

static inline void io_uring_files_cancel(struct files_struct *files)
{
        if (current->io_uring)
		__io_uring_cancel(files);
}

and

void __io_uring_cancel(struct files_struct *files)
{
        io_uring_cancel_generic(!files, NULL);
}

What the hell is that about?  What are you trying to check there?

All assignments to ->files:
init/init_task.c:116:   .files          = &init_files,
	Not NULL.
fs/file.c:433:          tsk->files = NULL;
	exit_files(), sets to NULL
fs/file.c:741:          me->files = cur_fds;
	__close_range(), if the value has been changed at all, the new one
	came from if (fds) swap(cur_fds, fds), so it can't become NULL here.
kernel/fork.c:1482:     tsk->files = newf;
	copy_files(), immediately preceded by verifying newf != NULL
kernel/fork.c:3044:                     current->files = new_fd;
	ksys_unshare(), under if (new_fd)
kernel/fork.c:3097:     task->files = copy;
	unshare_files(), with if (error || !copy) return error;
	slightly upstream.

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?