From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp-190b.mail.infomaniak.ch (smtp-190b.mail.infomaniak.ch [185.125.25.11]) (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 9CCC62E3381 for ; Mon, 24 Mar 2025 19:27:12 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=185.125.25.11 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1742844436; cv=none; b=aaYersHewMSXhc9/pTaURkffgWgk07HMqI9IYgX40q3no0jGaZz4ANYP8mFP6HdlvQwVZaalvkGkvx4JHSQaUK1VFw4XKZ70oEdbcHTakB64PXSk8JuB72QcR0m377Z4fv/J0XgJHXXNX7LG5VOlHIv3Q9sZcMuR48KA27Haivo= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1742844436; c=relaxed/simple; bh=P4P2z1eF/YNnOfLW8fBoOKbdAwyqWpJFO/rcNQNfqQo=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=KDkChI6jncu2zvmGkm8NEHE1qacfNW9Te+D/XibTAw2XAhrbYPTHhqJhgVKd14RdIcAZmDRx03ZCLZuNfzJSPeJNAh4f1JoOnnM40IrudtjMYWN7hh8dRCNWKbwen0RT8B8Ysea6M8HwQdqyMx4H888c16ezICAPw2ebwTwltvI= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=digikod.net; spf=pass smtp.mailfrom=digikod.net; dkim=pass (1024-bit key) header.d=digikod.net header.i=@digikod.net header.b=ZHOurTVq; arc=none smtp.client-ip=185.125.25.11 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=digikod.net Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=digikod.net Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=digikod.net header.i=@digikod.net header.b="ZHOurTVq" Received: from smtp-4-0000.mail.infomaniak.ch (smtp-4-0000.mail.infomaniak.ch [10.7.10.107]) by smtp-4-3000.mail.infomaniak.ch (Postfix) with ESMTPS id 4ZM32x2TBHzdNB; Mon, 24 Mar 2025 20:27:05 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=digikod.net; s=20191114; t=1742844425; bh=QBR/kecod01J1h193BKpllSnIdD5DJtmTOWxdGopk5I=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=ZHOurTVqBYQHfSi3F6pdPowbpieWYs2igBgvPmtLeNtKh6bOX6LndXdRYsKY6jh9C KH3Sauy/1pIzJ/oIq5tcqOK2Grf+Vopzl7YTCM6Ps/dI4TzMQV42PBtmh1lSd4a4Ot uxKt6oeGXt0Fw5OGk8If/a4DunNXPqyIjb/UWnHQ= Received: from unknown by smtp-4-0000.mail.infomaniak.ch (Postfix) with ESMTPA id 4ZM32v3B5vz9hp; Mon, 24 Mar 2025 20:27:03 +0100 (CET) Date: Mon, 24 Mar 2025 20:27:02 +0100 From: =?utf-8?Q?Micka=C3=ABl_Sala=C3=BCn?= To: Paul Moore Cc: Andrey Albershteyn , Richard Henderson , Matt Turner , Russell King , Catalin Marinas , Will Deacon , Geert Uytterhoeven , Michal Simek , Thomas Bogendoerfer , "James E.J. Bottomley" , Helge Deller , Madhavan Srinivasan , Michael Ellerman , Nicholas Piggin , Christophe Leroy , Naveen N Rao , Heiko Carstens , Vasily Gorbik , Alexander Gordeev , Christian Borntraeger , Sven Schnelle , Yoshinori Sato , Rich Felker , John Paul Adrian Glaubitz , "David S. Miller" , Andreas Larsson , Andy Lutomirski , Thomas Gleixner , Ingo Molnar , Borislav Petkov , Dave Hansen , x86@kernel.org, "H. Peter Anvin" , Chris Zankel , Max Filippov , Alexander Viro , Christian Brauner , Jan Kara , =?utf-8?Q?G=C3=BCnther?= Noack , Arnd Bergmann , Pali =?utf-8?B?Um9ow6Fy?= , James Morris , "Serge E. Hallyn" , linux-alpha@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-m68k@lists.linux-m68k.org, linux-mips@vger.kernel.org, linux-parisc@vger.kernel.org, linuxppc-dev@lists.ozlabs.org, linux-s390@vger.kernel.org, linux-sh@vger.kernel.org, sparclinux@vger.kernel.org, linux-fsdevel@vger.kernel.org, linux-security-module@vger.kernel.org, linux-api@vger.kernel.org, linux-arch@vger.kernel.org, selinux@vger.kernel.org, Andrey Albershteyn Subject: Re: [PATCH v4 1/3] lsm: introduce new hooks for setting/getting inode fsxattr Message-ID: <20250324.aThi9ioghiex@digikod.net> References: <20250321-xattrat-syscall-v4-1-3e82e6fb3264@kernel.org> Precedence: bulk X-Mailing-List: linux-api@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: X-Infomaniak-Routing: alpha On Fri, Mar 21, 2025 at 05:32:25PM -0400, Paul Moore wrote: > On Mar 21, 2025 Andrey Albershteyn wrote: > > > > Introduce new hooks for setting and getting filesystem extended > > attributes on inode (FS_IOC_FSGETXATTR). > > > > Cc: selinux@vger.kernel.org > > Cc: Paul Moore > > > > Signed-off-by: Andrey Albershteyn > > --- > > fs/ioctl.c | 7 ++++++- > > include/linux/lsm_hook_defs.h | 4 ++++ > > include/linux/security.h | 16 ++++++++++++++++ > > security/security.c | 32 ++++++++++++++++++++++++++++++++ > > 4 files changed, 58 insertions(+), 1 deletion(-) > > Thanks Andrey, one small change below, but otherwise this looks pretty > good. If you feel like trying to work up the SELinux implementation but > need some assitance please let me know, I'll be happy to help :) > > > diff --git a/fs/ioctl.c b/fs/ioctl.c > > index 638a36be31c14afc66a7fd6eb237d9545e8ad997..4434c97bc5dff5a3e8635e28745cd99404ff353e 100644 > > --- a/fs/ioctl.c > > +++ b/fs/ioctl.c > > @@ -525,10 +525,15 @@ EXPORT_SYMBOL(fileattr_fill_flags); > > int vfs_fileattr_get(struct dentry *dentry, struct fileattr *fa) > > { > > struct inode *inode = d_inode(dentry); > > + int error; > > > > if (!inode->i_op->fileattr_get) > > return -ENOIOCTLCMD; > > > > + error = security_inode_getfsxattr(inode, fa); > > + if (error) > > + return error; > > + > > return inode->i_op->fileattr_get(dentry, fa); > > } > > EXPORT_SYMBOL(vfs_fileattr_get); > > @@ -692,7 +697,7 @@ int vfs_fileattr_set(struct mnt_idmap *idmap, struct dentry *dentry, > > fa->flags |= old_ma.flags & ~FS_COMMON_FL; > > } > > err = fileattr_set_prepare(inode, &old_ma, fa); > > - if (!err) > > + if (!err && !security_inode_setfsxattr(inode, fa)) > > err = inode->i_op->fileattr_set(idmap, dentry, fa); > > } > > inode_unlock(inode); > > I don't believe we want to hide or otherwise drop the LSM return code as > that could lead to odd behavior, e.g. returning 0/success despite not > having executed the fileattr_set operation. Yes, this should look something like this: err = fileattr_set_prepare(inode, &old_ma, fa); if (err) goto out; err = security_inode_setfsxattr(dentry, fa); if (err) goto out; err = inode->i_op->fileattr_set(idmap, dentry, fa); if (err) goto out; > > -- > paul-moore.com >