On Fri, 2017-11-03 at 21:11 +0300, Mikhail Kurinnoi wrote: > В Fri, 03 Nov 2017 13:15:31 -0400 > Mimi Zohar <zohar@xxxxxxxxxxxxxxxxxx> пишет: > > > On Fri, 2017-11-03 at 20:06 +0300, Mikhail Kurinnoi wrote: > > > В Fri, 03 Nov 2017 12:54:08 -0400 > > > Mimi Zohar <zohar@xxxxxxxxxxxxxxxxxx> пишет: > > > > > > > On Fri, 2017-11-03 at 10:26 +0300, Mikhail Kurinnoi wrote: > > > > > This patch provide changes in order to allow metadata changes > > > > > for inode without xattr support. > > > > > > > > > > > > > > > Signed-off-by: Mikhail Kurinnoi <viewizard@xxxxxxxxxxxxx> > > > > > > > > > > security/integrity/evm/evm_main.c | 21 ++++++++++++--------- > > > > > 1 file changed, 12 insertions(+), 9 deletions(-) > > > > > > > > > > diff --git a/security/integrity/evm/evm_main.c > > > > > b/security/integrity/evm/evm_main.c index > > > > > 9826c02e2db8..51151c43433d 100644 --- > > > > > a/security/integrity/evm/evm_main.c +++ > > > > > b/security/integrity/evm/evm_main.c @@ -294,8 +294,7 @@ static > > > > > int evm_protect_xattr(struct dentry *dentry, const char > > > > > *xattr_name, if (!posix_xattr_acl(xattr_name)) return 0; > > > > > evm_status = > > > > > evm_verify_current_integrity(dentry); > > > > > - if ((evm_status == INTEGRITY_PASS) || > > > > > - (evm_status == INTEGRITY_NOXATTRS)) > > > > > + if (evm_status == INTEGRITY_NOXATTRS) > > > > > return 0; > > > > > goto out; > > > > > } > > > > > @@ -319,12 +318,15 @@ static int evm_protect_xattr(struct dentry > > > > > *dentry, const char *xattr_name, -EPERM, 0); > > > > > } > > > > > out: > > > > > - if (evm_status != INTEGRITY_PASS) > > > > > - integrity_audit_msg(AUDIT_INTEGRITY_METADATA, > > > > > d_backing_inode(dentry), > > > > > - dentry->d_name.name, > > > > > "appraise_metadata", > > > > > - > > > > > integrity_status_msg[evm_status], > > > > > - -EPERM, 0); > > > > > - return evm_status == INTEGRITY_PASS ? 0 : -EPERM; > > > > > + if ((evm_status == INTEGRITY_PASS) || > > > > > + (evm_status == INTEGRITY_UNKNOWN)) > > > > > + return 0; > > > > > + > > > > > + integrity_audit_msg(AUDIT_INTEGRITY_METADATA, > > > > > d_backing_inode(dentry), > > > > > + dentry->d_name.name, > > > > > "appraise_metadata", > > > > > + integrity_status_msg[evm_status], > > > > > + -EPERM, 0); > > > > > + return -EPERM; > > > > > } > > > > > > > > > > /** > > > > > @@ -435,7 +437,8 @@ int evm_inode_setattr(struct dentry *dentry, > > > > > struct iattr *attr) return 0; > > > > > evm_status = evm_verify_current_integrity(dentry); > > > > > if ((evm_status == INTEGRITY_PASS) || > > > > > - (evm_status == INTEGRITY_NOXATTRS)) > > > > > + (evm_status == INTEGRITY_NOXATTRS) || > > > > > + (evm_status == INTEGRITY_UNKNOWN)) > > > > > return 0; > > > > > integrity_audit_msg(AUDIT_INTEGRITY_METADATA, > > > > > d_backing_inode(dentry), dentry->d_name.name, > > > > > "appraise_metadata", > > > > > > > > Since this change is limited to setattr, perhaps it would be > > > > simpler to test the i_opflags directly, without modifying > > > > evm_protect_xattr(). > > > > > > In case of set/remove xattr (evm_inode_setxattr(), > > > evm_inode_removexattr()), evm should not interact fs module work, > > > that will provide proper error code. > > > As I see in __vfs_setxattr_noperm(), error code could be -EOPNOTSUPP > > > or -EIO, but evm will override it by error code -EPERM. I think, > > > this is wrong. If we don't have xattr support, let fs module handle > > > the error code. > > > > The patch description described a specific reason for the change. If > > there is another reason for the change, then either include it in the > > patch description or provide a separate patch. > > You are right, this is really poor description that don't describe > evm_protect_xattr() changes. I will provide patch v2 with extended > patch description. > > Mimi, is it appropriate changes for evm_inode_setattr(), or should I > correct patch to test the i_opflags directly instead? I could be wrong, but I think these are two separate issues and should be addressed separately. From your explanation, it sounds like you want to return the real setxattr/removexattr failure status. Perhaps after making this change, I'll think differently. Mimi
