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 90EADCA0FE8 for ; Fri, 1 Sep 2023 12:51:14 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S244190AbjIAMvP (ORCPT ); Fri, 1 Sep 2023 08:51:15 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:50052 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S244118AbjIAMvP (ORCPT ); Fri, 1 Sep 2023 08:51:15 -0400 Received: from dfw.source.kernel.org (dfw.source.kernel.org [IPv6:2604:1380:4641:c500::1]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 479BE1726; Fri, 1 Sep 2023 05:50:51 -0700 (PDT) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id EE22260DFF; Fri, 1 Sep 2023 12:50:38 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 92AD8C433C8; Fri, 1 Sep 2023 12:50:36 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1693572638; bh=nMZ96azLHYWOYXamjxieuqFfT91YKa/NfwWGHknuPOA=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=Z8wsQTwLmYV87LF3PK4H2AQ7G1wx/tK9CDEsJ/DyuLkl21MZG6X5XYFgyIrPrP7Vq a7NP/uk99iNgEq6sSEUfRzicNdhdarQM0aI0Je56+u4rmDeO9xkiCzV561wHIXOM4q x8MaI0RWxXkHXLJCLZsaKRBOtiYGRUcJaODa6gPqTOAiAzJ26qL6DBEVsF6Ace5KjC +G4rdN1g1utaP9bkhWy6SoNufaCKGaR50a9tkSHr2HgB/HIfQg1VRU49174lyRcTe4 /0QR7t888E4uYAZFkq5a8rMhFbPbAO5ZncJbbWxn3n03+EQyOzYlm9b9CO5+jupH1L qaoteLrPPTfbw== Date: Fri, 1 Sep 2023 14:50:33 +0200 From: Christian Brauner To: Bernd Schubert Cc: Bernd Schubert , linux-fsdevel@vger.kernel.org, miklos@szeredi.hu, dsingh@ddn.com, Josef Bacik , linux-btrfs@vger.kernel.org, Alexander Viro Subject: Re: [PATCH 1/2] fs: Add and export file_needs_remove_privs Message-ID: <20230901-briefe-amtieren-0c8b555219cb@brauner> References: <20230831112431.2998368-1-bschubert@ddn.com> <20230831112431.2998368-2-bschubert@ddn.com> <20230831-letzlich-eruption-9187c3adaca6@brauner> <99bc64c2-44e2-5000-45b7-d9343bcc8fb8@fastmail.fm> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <99bc64c2-44e2-5000-45b7-d9343bcc8fb8@fastmail.fm> Precedence: bulk List-ID: X-Mailing-List: linux-btrfs@vger.kernel.org On Thu, Aug 31, 2023 at 04:17:01PM +0200, Bernd Schubert wrote: > > > On 8/31/23 15:40, Christian Brauner wrote: > > On Thu, Aug 31, 2023 at 01:24:30PM +0200, Bernd Schubert wrote: > > > File systems want to hold a shared lock for DIO writes, > > > but may need to drop file priveliges - that a requires an > > > exclusive lock. The new export function file_needs_remove_privs() > > > is added in order to first check if that is needed. > > > > > > Cc: Miklos Szeredi > > > Cc: Dharmendra Singh > > > Cc: Josef Bacik > > > Cc: linux-btrfs@vger.kernel.org > > > Cc: Alexander Viro > > > Cc: Christian Brauner > > > Cc: linux-fsdevel@vger.kernel.org > > > Signed-off-by: Bernd Schubert > > > --- > > > fs/inode.c | 8 ++++++++ > > > include/linux/fs.h | 1 + > > > 2 files changed, 9 insertions(+) > > > > > > diff --git a/fs/inode.c b/fs/inode.c > > > index 67611a360031..9b05db602e41 100644 > > > --- a/fs/inode.c > > > +++ b/fs/inode.c > > > @@ -2013,6 +2013,14 @@ int dentry_needs_remove_privs(struct mnt_idmap *idmap, > > > return mask; > > > } > > > +int file_needs_remove_privs(struct file *file) > > > +{ > > > + struct dentry *dentry = file_dentry(file); > > > + > > > + return dentry_needs_remove_privs(file_mnt_idmap(file), dentry); > > > > Ugh, I wanted to propose to get rid of this dentry dance but I propsed > > that before and remembered it's because of __vfs_getxattr() which is > > called from the capability security hook that we need it... > > Is there anything specific you are suggesting? No, it's not actionable for you here. It would require adding inode methods to set and get filesystem capabilities and then converting it in such a way that we don't need to rely on passing dentries around. That's a separate larger patchset that we would need with surgery across a bunch of filesystems and the vfs - Seth (Forshee) has been working on this. The callchains are just pointless which I remembered when I saw the patchset: file_needs_remove_privs(file) -> dentry_needs_remove_privs(dentry) -> inode = d_inode(dentry) // do inode stuff // security_needs_*(dentry) point is ideally we shouldn't need the dentry in *remove_privs() at all.