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 Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 2BA09CA0FF6 for ; Sat, 2 Sep 2023 04:44:21 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1351509AbjIBEoW (ORCPT ); Sat, 2 Sep 2023 00:44:22 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:51150 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1351477AbjIBEoV (ORCPT ); Sat, 2 Sep 2023 00:44:21 -0400 Received: from zeniv.linux.org.uk (zeniv.linux.org.uk [IPv6:2a03:a000:7:0:5054:ff:fe1c:15ff]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 41E0C10F8; Fri, 1 Sep 2023 21:44:18 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=linux.org.uk; s=zeniv-20220401; h=Sender:In-Reply-To:Content-Type: MIME-Version:References:Message-ID:Subject:Cc:To:From:Date:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description; bh=jZEmoopfjFAadS37fL5Ohwd3FkpsxLZbrexmwjaT+jI=; b=T+Ak+8mknlf10plUIf8cqsZHYT Hg/jAlgEkRr5Sz7ydWcfTVC1vmPXaNFtla9tmSsHcPbSnJ9Jw6nfEzIwSfj05ridaggvrDJvMPZ/r Qrk1HiNzp+/cOUnUIk1RjKU0Do84Os+H7z4LvGUSIY9O+7QNZgDO1jSBN8ffJ7wgaCEPWw1hMCDae btCN/lhV1hmg9QJjiXwQZE+9gDRViUuWHPodrOlFX1kXdtL2GkyQNuETbziCmksUXyHEBJqtE7FGh mGQ1eU0yZvqgxdyX1TNZEG9PqTcK4uE7jsPLgaigyR4qKzzWgL3oPLVjr5mctPGk/3nEln0guwAel QiB7hSkw==; Received: from viro by zeniv.linux.org.uk with local (Exim 4.96 #2 (Red Hat Linux)) id 1qcIUV-002kR1-1A; Sat, 02 Sep 2023 04:44:11 +0000 Date: Sat, 2 Sep 2023 05:44:11 +0100 From: Al Viro To: Linus Torvalds Cc: Christian Brauner , Jens Axboe , Christoph Hellwig , Aleksa Sarai , Seth Forshee , linux-fsdevel@vger.kernel.org, stable@vger.kernel.org Subject: Re: [PATCH] file: always lock position Message-ID: <20230902044411.GI3390869@ZenIV> References: <20230724-vfs-fdget_pos-v1-1-a4abfd7103f3@kernel.org> <20230724-scheren-absegnen-8c807c760ba1@brauner> 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 Mon, Jul 24, 2023 at 09:51:05AM -0700, Linus Torvalds wrote: > On Mon, 24 Jul 2023 at 09:36, Linus Torvalds > wrote: > > > > There are magic rules with "total_refs == inflight_refs", and that > > total_refs thing is very much the file count, ie > > > > total_refs = file_count(u->sk.sk_socket->file); > > > > where we had some nasty bugs with files coming back to life. > > Ok, I don't think this is an issue here. It really is that "only > in-flight refs remaining" that is a special case, and even > pidfd_getfd() shouldn't be able to change that. > > But the magic code is all in fget_task(), and those need to be checked. > > You can see how proc does things properly: it does do "fget_task()", > but then it only uses it to copy the path part, and just does fput() > afterwards. > > The bpf code does something like that too, and seems ok (ie it gets > the file in order to copy data from it, not to install it). Aside of fget_task() use, it has this: rcu_read_lock(); for (;; curr_fd++) { struct file *f; f = task_lookup_next_fd_rcu(curr_task, &curr_fd); if (!f) break; if (!get_file_rcu(f)) continue; /* set info->fd */ info->fd = curr_fd; rcu_read_unlock(); return f; } curr_task is not cached current here - it can be an arbitrary thread. And what we do to the file reference we get here is ctx.meta = &meta; ctx.task = info->task; ctx.fd = info->fd; ctx.file = file; return bpf_iter_run_prog(prog, &ctx); I think it can't be used to shove it into any descriptor table, but then there's forming an SCM_RIGHTS datagram and sending it, etc. - it might be worth looking into.