evm: allow metadata changes for inode without xattr support

PATCH In review Mikhail Kurinnoi 2 participants 7 messages Nov 3, 2017

Subsystems (4)

Security (LSM) Keys-trusted Integrity measurement architecture (IMA) Extended Verification Module (EVM)

lore

Share & tools

lore

Mikhail Kurinnoi

author

2017-11-03 07:26 UTC #0 lore raw

PATCH 1/1 evm: allow metadata changes for inode without xattr support

This patch provide changes in order to allow metadata changes for inode without xattr support.

Signed-off-by Mikhail Kurinnoi <[email protected]>

 security/integrity/evm/evm_main.c | 21 ++++++++++++---------
 1 file changed, 12 insertions(+), 9 deletions(-)

security/integrity/evm/evm_main.c +12 -9

index 9826c02e2db8..51151c43433d 100644
@@ -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",

Mimi Zohar

2017-11-03 16:54 UTC #1 lore raw

Quoted 47 lines · Mikhail Kurinnoi

This patch provide changes in order to allow metadata changes for inode without xattr support.

Signed-off-by Mikhail Kurinnoi <[email protected]>

 security/integrity/evm/evm_main.c | 21 ++++++++++++---------
 1 file changed, 12 insertions(+), 9 deletions(-)

security/integrity/evm/evm_main.c +10 -8

index 9826c02e2db8..51151c43433d 100644
@@ -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;
 }
 
 /**

security/integrity/evm/evm_main.c int evm_inode_setattr(struct dentry *dentry, struct iattr *attr) · line 444

Quoted 9 lines

security/integrity/evm/evm_main.c +2 -1

@@ -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().

Mimi

Mikhail Kurinnoi

author

2017-11-03 17:06 UTC #2 lore raw

? Fri, 03 Nov 2017 12:54:08 -0400
Mimi Zohar <[email protected]> ?????:

Quoted 50 lines · Mikhail Kurinnoi

Quoted 49 lines
This patch provide changes in order to allow metadata changes for inode without xattr support.
Signed-off-by Mikhail Kurinnoi <[email protected]>
 security/integrity/evm/evm_main.c | 21 ++++++++++++---------
 1 file changed, 12 insertions(+), 9 deletions(-)
(quoted diff) +0 -0
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;
security/integrity/evm/evm_main.c +6 -8
 		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),
(quoted diff) +1 -0
+			    dentry->d_name.name,
"appraise_metadata",
security/integrity/evm/evm_main.c +3 -0
+			    integrity_status_msg[evm_status],
+			    -EPERM, 0);
+	return -EPERM;
 }

Quoted 15 lines

security/integrity/evm/evm_main.c int evm_inode_setattr(struct dentry *dentry, · line 444
Quoted 12 lines
/**
security/integrity/evm/evm_main.c +2 -1
@@ -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.

Signature · 2 lines

Best regards,
Mikhail Kurinnoi

Mimi Zohar

2017-11-03 17:15 UTC #3 lore raw

Quoted 69 lines · Mikhail Kurinnoi

? Fri, 03 Nov 2017 12:54:08 -0400
Mimi Zohar <[email protected]> ?????:
Quoted 50 lines · Mikhail Kurinnoi
Quoted 49 lines
This patch provide changes in order to allow metadata changes for inode without xattr support.
Signed-off-by Mikhail Kurinnoi <[email protected]>
 security/integrity/evm/evm_main.c | 21 ++++++++++++---------
 1 file changed, 12 insertions(+), 9 deletions(-)
(quoted diff) +0 -0
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;
security/integrity/evm/evm_main.c +6 -8
 		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),
(quoted diff) +1 -0
+			    dentry->d_name.name,
"appraise_metadata",
security/integrity/evm/evm_main.c +3 -0
+			    integrity_status_msg[evm_status],
+			    -EPERM, 0);
+	return -EPERM;
 }
Quoted 15 lines
security/integrity/evm/evm_main.c int evm_inode_setattr(struct dentry *dentry, · line 444
Quoted 12 lines
/**
security/integrity/evm/evm_main.c +2 -1
@@ -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().

Quoted 7 lines

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.

Mimi

Mikhail Kurinnoi

author

2017-11-03 18:11 UTC #4 lore raw

? Fri, 03 Nov 2017 13:15:31 -0400
Mimi Zohar <[email protected]> ?????:

Quoted 80 lines · Mikhail Kurinnoi

Quoted 71 lines
? Fri, 03 Nov 2017 12:54:08 -0400
Mimi Zohar <[email protected]> ?????:
Quoted 51 lines · Mikhail Kurinnoi
Quoted 50 lines
This patch provide changes in order to allow metadata changes for inode without xattr support.
Signed-off-by Mikhail Kurinnoi <[email protected]>
 security/integrity/evm/evm_main.c | 21 ++++++++++++---------
 1 file changed, 12 insertions(+), 9 deletions(-)
(quoted diff) +0 -0
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);
security/integrity/evm/evm_main.c +6 -8
-		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),
(quoted diff) +1 -0
+			    dentry->d_name.name,
"appraise_metadata",
security/integrity/evm/evm_main.c +3 -0
+			    integrity_status_msg[evm_status],
+			    -EPERM, 0);
+	return -EPERM;
 }
Quoted 16 lines
Quoted 11 lines
/**
security/integrity/evm/evm_main.c +2 -1
@@ -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,
Quoted 1 line
"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().
Quoted 7 lines
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.

Quoted 3 lines

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?

Signature · 2 lines

Best regards,
Mikhail Kurinnoi

Mimi Zohar

2017-11-03 18:26 UTC #5 lore raw

Quoted 87 lines · Mikhail Kurinnoi

? Fri, 03 Nov 2017 13:15:31 -0400
Mimi Zohar <[email protected]> ?????:
Quoted 80 lines · Mikhail Kurinnoi
Quoted 71 lines
? Fri, 03 Nov 2017 12:54:08 -0400
Mimi Zohar <[email protected]> ?????:
Quoted 51 lines · Mikhail Kurinnoi
Quoted 50 lines
This patch provide changes in order to allow metadata changes for inode without xattr support.
Signed-off-by Mikhail Kurinnoi <[email protected]>
 security/integrity/evm/evm_main.c | 21 ++++++++++++---------
 1 file changed, 12 insertions(+), 9 deletions(-)
(quoted diff) +0 -0
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);
security/integrity/evm/evm_main.c +6 -8
-		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),
(quoted diff) +1 -0
+			    dentry->d_name.name,
"appraise_metadata",
security/integrity/evm/evm_main.c +3 -0
+			    integrity_status_msg[evm_status],
+			    -EPERM, 0);
+	return -EPERM;
 }
Quoted 16 lines
Quoted 11 lines
/**
security/integrity/evm/evm_main.c +2 -1
@@ -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,
Quoted 1 line
"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().
Quoted 7 lines
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.
Quoted 3 lines
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.

Quoted 6 lines

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

Mikhail Kurinnoi

author

2017-11-03 19:00 UTC #6 lore raw

? Fri, 03 Nov 2017 14:26:18 -0400
Mimi Zohar <[email protected]> ?????:

Quoted 95 lines · Mikhail Kurinnoi

Quoted 94 lines
? Fri, 03 Nov 2017 13:15:31 -0400
Mimi Zohar <[email protected]> ?????:
Quoted 75 lines · Mikhail Kurinnoi
Quoted 74 lines
? Fri, 03 Nov 2017 12:54:08 -0400
Mimi Zohar <[email protected]> ?????:
Quoted 70 lines · Mikhail Kurinnoi
Quoted 64 lines
This patch provide changes in order to allow metadata changes for inode without xattr support.
Signed-off-by Mikhail Kurinnoi <[email protected]>
 security/integrity/evm/evm_main.c | 21
++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-)
(quoted diff) +0 -0
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);
security/integrity/evm/evm_main.c +6 -8
-		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),
(quoted diff) +1 -0
+			    dentry->d_name.name,
"appraise_metadata",
+
integrity_status_msg[evm_status],
security/integrity/evm/evm_main.c +4 -1
+			    -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,
Quoted 1 line
"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().
Quoted 11 lines
Quoted 7 lines
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
(quoted diff) +0 -2
-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.

Quoted 5 lines

Quoted 2 lines
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.

Ok.

Quoted 2 lines

From your explanation, it sounds like you
want to return the real setxattr/removexattr failure status.

Yes. That was the idea of evm_protect_xattr() changes. I think, in this way we highlight the real point of issue (why we can't set/remove xattr).

Signature · 2 lines

Best regards,
Mikhail Kurinnoi