lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CAHC9VhRS2DrGoSb72uY61yVNXXZjOqKSkEdK7vNgPJxVdZgujA@mail.gmail.com>
Date: Thu, 31 Jul 2025 17:17:51 -0400
From: Paul Moore <paul@...l-moore.com>
To: "tianjia.zhang" <tianjia.zhang@...ux.alibaba.com>
Cc: Casey Schaufler <casey@...aufler-ca.com>, James Morris <jmorris@...ei.org>, 
	"Serge E. Hallyn" <serge@...lyn.com>, linux-security-module@...r.kernel.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] lsm: simplify security_inode_copy_up_xattr()

On Thu, Jul 31, 2025 at 7:59 AM tianjia.zhang
<tianjia.zhang@...ux.alibaba.com> wrote:
> On 7/29/25 11:09 PM, Paul Moore wrote:
> > On Tue, Jul 29, 2025 at 10:43 AM Casey Schaufler <casey@...aufler-ca.com> wrote:
> >> On 7/29/2025 2:09 AM, Tianjia Zhang wrote:
> >>> The implementation of function security_inode_copy_up_xattr can be
> >>> simplified to directly call call_int_hook().
> >>>
> >>> Signed-off-by: Tianjia Zhang <tianjia.zhang@...ux.alibaba.com>
> >>> ---
> >>>   security/security.c | 8 +-------
> >>>   1 file changed, 1 insertion(+), 7 deletions(-)
> >>>
> >>> diff --git a/security/security.c b/security/security.c
> >>> index 596d41818577..a5c2e5a8009f 100644
> >>> --- a/security/security.c
> >>> +++ b/security/security.c
> >>> @@ -2774,13 +2774,7 @@ EXPORT_SYMBOL(security_inode_copy_up);
> >>>    */
> >>>   int security_inode_copy_up_xattr(struct dentry *src, const char *name)
> >>>   {
> >>> -     int rc;
> >>> -
> >>> -     rc = call_int_hook(inode_copy_up_xattr, src, name);
> >>> -     if (rc != LSM_RET_DEFAULT(inode_copy_up_xattr))
> >>> -             return rc;
> >>> -
> >>> -     return LSM_RET_DEFAULT(inode_copy_up_xattr);
> >>> +     return call_int_hook(inode_copy_up_xattr, src, name);
> >>
> >> Both the existing code and the proposed change are incorrect.
> >> If two LSMs supply the hook, and the first does not recognize
> >> the attribute, the second, which might recognize the attribute,
> >> will not be called. As SELinux and EVM both supply this hook
> >> there may be a real problem here.
> >
> > It appears that Smack also supplies a inode_copy_up_xattr() callback
> > via smack_inode_copy_up_xattr().
> >
> > Someone should double check this logic, but looking at it very
> > quickly, it would appear that LSM framework should run the individual
> > LSM callbacks in order so long as they return -EOPNOTSUPP, if they do
> > not return -EOPNOTSUPP, the return value should be returned to the
> > caller without executing any further callbacks.  As a default return
> > value, or if all of the LSM callbacks succeed with -EOPNOTSUPP, the
> > hook should return -EOPNOTSUPP.
> >
> > Tianjia Zhang, would you be able to develop and test a patch for this?
> >
>
> I carefully checked the logic of security_inode_copy_up_xattr(). I think
> there is no problem with the current code. The default return value of
> inode_copy_up_xattr LSM is -EOPNOTSUPP. Therefore, when -EOPNOTSUPP is
> returned in the LSM callback, the next callback function will be called
> in a loop. When an LSM module recognizes the attribute name that needs
> to be ignored, it will return -ECANCELED to indicate
> security_inode_copy_up_xattr() to jump out of the loop and ignore the
> copy of this attribute in overlayfs.
>
> Currently, the SELinux, EVM, and Smack that supply inode_copy_up_xattr
> callback all return -ECANCELED after recognizing the extended attribute
> name they are concerned about, to indicate overlayfs to discard the
> copy_up operation of this attribute. I think this is in line with
> expectations.

Perfect, thanks!

-- 
paul-moore.com

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ