On 7/22/21 5:30 PM, Al Viro wrote:

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

quoted

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?

It's not the clearest, but the files check is just to distinguish between
exec vs normal cancel. For exec, we pass in files == NULL. It's not
related to task->files being NULL or not, we explicitly pass NULL for
exec.

-- 
Jens Axboe