On 7/22/21 5:30 PM, Al Viro wrote:
> 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?
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
