* [PATCH] evm: allow metadata changes for inode without xattr support @ 2017-11-03 7:26 Mikhail Kurinnoi 2017-11-03 16:54 ` Mimi Zohar 0 siblings, 1 reply; 7+ messages in thread From: Mikhail Kurinnoi @ 2017-11-03 7:26 UTC (permalink / raw) To: linux-integrity This patch provide changes in order to allow metadata changes for inode without xattr support. Signed-off-by: Mikhail Kurinnoi <viewizard@viewizard.com> 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", ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] evm: allow metadata changes for inode without xattr support 2017-11-03 7:26 [PATCH] evm: allow metadata changes for inode without xattr support Mikhail Kurinnoi @ 2017-11-03 16:54 ` Mimi Zohar 2017-11-03 17:06 ` Mikhail Kurinnoi 0 siblings, 1 reply; 7+ messages in thread From: Mimi Zohar @ 2017-11-03 16:54 UTC (permalink / raw) To: Mikhail Kurinnoi, linux-integrity 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@viewizard.com> > > 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(). Mimi ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] evm: allow metadata changes for inode without xattr support 2017-11-03 16:54 ` Mimi Zohar @ 2017-11-03 17:06 ` Mikhail Kurinnoi 2017-11-03 17:15 ` Mimi Zohar 0 siblings, 1 reply; 7+ messages in thread From: Mikhail Kurinnoi @ 2017-11-03 17:06 UTC (permalink / raw) To: Mimi Zohar; +Cc: linux-integrity ? Fri, 03 Nov 2017 12:54:08 -0400 Mimi Zohar <zohar@linux.vnet.ibm.com> ?????: > 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@viewizard.com> > > > > 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. -- Best regards, Mikhail Kurinnoi ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] evm: allow metadata changes for inode without xattr support 2017-11-03 17:06 ` Mikhail Kurinnoi @ 2017-11-03 17:15 ` Mimi Zohar 2017-11-03 18:11 ` Mikhail Kurinnoi 0 siblings, 1 reply; 7+ messages in thread From: Mimi Zohar @ 2017-11-03 17:15 UTC (permalink / raw) To: Mikhail Kurinnoi; +Cc: linux-integrity On Fri, 2017-11-03 at 20:06 +0300, Mikhail Kurinnoi wrote: > ? Fri, 03 Nov 2017 12:54:08 -0400 > Mimi Zohar <zohar@linux.vnet.ibm.com> ?????: > > > 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@viewizard.com> > > > > > > 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. Mimi ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] evm: allow metadata changes for inode without xattr support 2017-11-03 17:15 ` Mimi Zohar @ 2017-11-03 18:11 ` Mikhail Kurinnoi 2017-11-03 18:26 ` Mimi Zohar 0 siblings, 1 reply; 7+ messages in thread From: Mikhail Kurinnoi @ 2017-11-03 18:11 UTC (permalink / raw) To: Mimi Zohar; +Cc: linux-integrity ? Fri, 03 Nov 2017 13:15:31 -0400 Mimi Zohar <zohar@linux.vnet.ibm.com> ?????: > On Fri, 2017-11-03 at 20:06 +0300, Mikhail Kurinnoi wrote: > > ? Fri, 03 Nov 2017 12:54:08 -0400 > > Mimi Zohar <zohar@linux.vnet.ibm.com> ?????: > > > > > 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@viewizard.com> > > > > > > > > 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? -- Best regards, Mikhail Kurinnoi ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] evm: allow metadata changes for inode without xattr support 2017-11-03 18:11 ` Mikhail Kurinnoi @ 2017-11-03 18:26 ` Mimi Zohar 2017-11-03 19:00 ` Mikhail Kurinnoi 0 siblings, 1 reply; 7+ messages in thread From: Mimi Zohar @ 2017-11-03 18:26 UTC (permalink / raw) To: Mikhail Kurinnoi; +Cc: linux-integrity On Fri, 2017-11-03 at 21:11 +0300, Mikhail Kurinnoi wrote: > ? Fri, 03 Nov 2017 13:15:31 -0400 > Mimi Zohar <zohar@linux.vnet.ibm.com> ?????: > > > On Fri, 2017-11-03 at 20:06 +0300, Mikhail Kurinnoi wrote: > > > ? Fri, 03 Nov 2017 12:54:08 -0400 > > > Mimi Zohar <zohar@linux.vnet.ibm.com> ?????: > > > > > > > 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@viewizard.com> > > > > > > > > > > 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] evm: allow metadata changes for inode without xattr support 2017-11-03 18:26 ` Mimi Zohar @ 2017-11-03 19:00 ` Mikhail Kurinnoi 0 siblings, 0 replies; 7+ messages in thread From: Mikhail Kurinnoi @ 2017-11-03 19:00 UTC (permalink / raw) To: Mimi Zohar; +Cc: linux-integrity ? Fri, 03 Nov 2017 14:26:18 -0400 Mimi Zohar <zohar@linux.vnet.ibm.com> ?????: > On Fri, 2017-11-03 at 21:11 +0300, Mikhail Kurinnoi wrote: > > ? Fri, 03 Nov 2017 13:15:31 -0400 > > Mimi Zohar <zohar@linux.vnet.ibm.com> ?????: > > > > > On Fri, 2017-11-03 at 20:06 +0300, Mikhail Kurinnoi wrote: > > > > ? Fri, 03 Nov 2017 12:54:08 -0400 > > > > Mimi Zohar <zohar@linux.vnet.ibm.com> ?????: > > > > > > > > > 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@viewizard.com> > > > > > > > > > > > > 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. Ok. > 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). -- Best regards, Mikhail Kurinnoi ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2017-11-03 19:00 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-11-03 7:26 [PATCH] evm: allow metadata changes for inode without xattr support Mikhail Kurinnoi 2017-11-03 16:54 ` Mimi Zohar 2017-11-03 17:06 ` Mikhail Kurinnoi 2017-11-03 17:15 ` Mimi Zohar 2017-11-03 18:11 ` Mikhail Kurinnoi 2017-11-03 18:26 ` Mimi Zohar 2017-11-03 19:00 ` Mikhail Kurinnoi
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.