From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-3.8 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id C865EC432BE for ; Fri, 23 Jul 2021 00:03:06 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id AD13F60EB5 for ; Fri, 23 Jul 2021 00:03:06 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232727AbhGVXWa (ORCPT ); Thu, 22 Jul 2021 19:22:30 -0400 Received: from zeniv-ca.linux.org.uk ([142.44.231.140]:35402 "EHLO zeniv-ca.linux.org.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232636AbhGVXW3 (ORCPT ); Thu, 22 Jul 2021 19:22:29 -0400 Received: from viro by zeniv-ca.linux.org.uk with local (Exim 4.94.2 #2 (Red Hat Linux)) id 1m6iea-0031GC-1b; Fri, 23 Jul 2021 00:03:00 +0000 Date: Fri, 23 Jul 2021 00:03:00 +0000 From: Al Viro To: Jens Axboe Cc: Pavel Begunkov , io-uring@vger.kernel.org, linux-fsdevel@vger.kernel.org, Linus Torvalds Subject: Re: [PATCH 3/3] io_uring: refactor io_sq_offload_create() Message-ID: References: <939776f90de8d2cdd0414e1baa29c8ec0926b561.1618916549.git.asml.silence@gmail.com> <57758edf-d064-d37e-e544-e0c72299823d@kernel.dk> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Sender: Al Viro Precedence: bulk List-ID: X-Mailing-List: linux-fsdevel@vger.kernel.org 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)...