From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 3204D1CA82; Wed, 3 Jan 2024 18:49:59 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 16542C433C8; Wed, 3 Jan 2024 18:49:58 +0000 (UTC) Date: Wed, 3 Jan 2024 13:51:02 -0500 From: Steven Rostedt To: Linus Torvalds Cc: LKML , Linux Trace Kernel , Masami Hiramatsu , Mathieu Desnoyers Subject: Re: [PATCH] eventfs: Stop using dcache_readdir() for getdents() Message-ID: <20240103135102.38352956@gandalf.local.home> In-Reply-To: References: <20240103102553.17a19cea@gandalf.local.home> X-Mailer: Claws Mail 3.19.1 (GTK+ 2.24.33; x86_64-pc-linux-gnu) Precedence: bulk X-Mailing-List: linux-trace-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit On Wed, 3 Jan 2024 10:38:09 -0800 Linus Torvalds wrote: > On Wed, 3 Jan 2024 at 10:12, Linus Torvalds > wrote: > > > > Much better. Now eventfs looks more like a real filesystem, and less > > like an eldritch horror monster that is parts of dcache tackled onto a > > pseudo-filesystem. > > Oh, except I think you still need to just remove the 'set_gid()' mess. > > It's disgusting and it's wrong, and it's not even what the 'uid' > option does (it only sets the root inode uid). > > If you remount the filesystem with different gid values, you get to > keep both broken pieces. And if it isn't a remount, then setting the > root uid is sufficient. > > I think the whole thing was triggered by commit 49d67e445742, and > maybe the fix is to just revert that commit. > > That commit makes no sense in general, since the default mounting > position for tracefs that the kernel sets up is only accessible to > root anyway. > > Alternatively, just do the ->permissions() thing, and allow access to > the group in the mount options. > > Getting rid of set_gid() would be this attached lovely patch: > > fs/tracefs/inode.c | 83 ++---------------------------------------------------- > 1 file changed, 2 insertions(+), 81 deletions(-) > > and would get rid of the final (?) piece of disgusting dcache hackery > that tracefs most definitely should not have. > I'll look at that and play with it. I understand VFS much better now that I spent so much time with eventfs. That commit had to do with allowing OTH read access, which is a security issue as the trace files expose a lot of the kernel internals. I think these changes are a bit much for -rc8, don't you? Or do you want all this in before v6.7 is released. I'd be more comfortable with adding these changes in the upcoming merge window, where I can have more time playing with them. -- Steve