* [PATCH] src/t_attr_corruption: use security.capability instead of security.evm
@ 2019-02-25 19:10 jeffm
2019-02-25 22:47 ` Darrick J. Wong
0 siblings, 1 reply; 6+ messages in thread
From: jeffm @ 2019-02-25 19:10 UTC (permalink / raw)
To: fstests; +Cc: darrick.wong, Jeff Mahoney
From: Jeff Mahoney <jeffm@suse.com>
src/t_attr_corruption uses the security.evm extended attribute because
it sorts before security.posix_acl_access. The security.evm attribute
is a formatted structure and when passed an uninitialized buffer, it
will fail with EPERM.
We see test failures like:
--- tests/generic/529.out2019-02-21 13:22:47.583406922 -0500
+++ /opt/xfstests/results//generic/529.out.bad 2019-02-21 13:57:31.967406922 -0500
@@ -1,2 +1,2 @@
QA output created by 529
-list attr: Numerical result out of range
+set evm: Operation not permitted
This patch uses security.capability which also sorts where it needs to
do for the test and also accepts an unformatted buffer.
Signed-off-by: Jeff Mahoney <jeffm@suse.com>
---
src/t_attr_corruption.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/t_attr_corruption.c b/src/t_attr_corruption.c
index f26611f9..0c229dbc 100644
--- a/src/t_attr_corruption.c
+++ b/src/t_attr_corruption.c
@@ -76,7 +76,7 @@ int main(int argc, char *argv[])
if (ret)
die("set posix acl");
- ret = fsetxattr(fd, "security.evm", buf, 1, 1);
+ ret = fsetxattr(fd, "security.capability", buf, 1, 1);
if (ret)
die("set evm");
--
2.16.4
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] src/t_attr_corruption: use security.capability instead of security.evm 2019-02-25 19:10 [PATCH] src/t_attr_corruption: use security.capability instead of security.evm jeffm @ 2019-02-25 22:47 ` Darrick J. Wong 2019-02-25 23:26 ` Darrick J. Wong 2019-02-26 1:39 ` Jeff Mahoney 0 siblings, 2 replies; 6+ messages in thread From: Darrick J. Wong @ 2019-02-25 22:47 UTC (permalink / raw) To: jeffm; +Cc: fstests On Mon, Feb 25, 2019 at 02:10:52PM -0500, jeffm@suse.com wrote: > From: Jeff Mahoney <jeffm@suse.com> > > src/t_attr_corruption uses the security.evm extended attribute because > it sorts before security.posix_acl_access. The security.evm attribute > is a formatted structure and when passed an uninitialized buffer, it > will fail with EPERM. > > We see test failures like: > --- tests/generic/529.out2019-02-21 13:22:47.583406922 -0500 > +++ /opt/xfstests/results//generic/529.out.bad 2019-02-21 13:57:31.967406922 -0500 > @@ -1,2 +1,2 @@ > QA output created by 529 > -list attr: Numerical result out of range > +set evm: Operation not permitted > > This patch uses security.capability which also sorts where it needs to > do for the test and also accepts an unformatted buffer. > > Signed-off-by: Jeff Mahoney <jeffm@suse.com> > --- > src/t_attr_corruption.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/src/t_attr_corruption.c b/src/t_attr_corruption.c > index f26611f9..0c229dbc 100644 > --- a/src/t_attr_corruption.c > +++ b/src/t_attr_corruption.c > @@ -76,7 +76,7 @@ int main(int argc, char *argv[]) > if (ret) > die("set posix acl"); > > - ret = fsetxattr(fd, "security.evm", buf, 1, 1); > + ret = fsetxattr(fd, "security.capability", buf, 1, 1); This fails for me both with and without EVM configured into my kernel: fsetxattr(3, "security.capability", "\3", 1, XATTR_CREATE) = -1 EINVAL (Invalid argument) Judging from fs/xattr.c it looks as though security.capability also has a defined format that's parsed by security/commoncap.c... --D > if (ret) > die("set evm"); > > -- > 2.16.4 > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] src/t_attr_corruption: use security.capability instead of security.evm 2019-02-25 22:47 ` Darrick J. Wong @ 2019-02-25 23:26 ` Darrick J. Wong 2019-02-26 2:07 ` Xiao Yang 2019-02-26 3:33 ` Xiao Yang 2019-02-26 1:39 ` Jeff Mahoney 1 sibling, 2 replies; 6+ messages in thread From: Darrick J. Wong @ 2019-02-25 23:26 UTC (permalink / raw) To: jeffm; +Cc: fstests On Mon, Feb 25, 2019 at 02:47:44PM -0800, Darrick J. Wong wrote: > On Mon, Feb 25, 2019 at 02:10:52PM -0500, jeffm@suse.com wrote: > > From: Jeff Mahoney <jeffm@suse.com> > > > > src/t_attr_corruption uses the security.evm extended attribute because > > it sorts before security.posix_acl_access. The security.evm attribute > > is a formatted structure and when passed an uninitialized buffer, it > > will fail with EPERM. > > > > We see test failures like: > > --- tests/generic/529.out2019-02-21 13:22:47.583406922 -0500 > > +++ /opt/xfstests/results//generic/529.out.bad 2019-02-21 13:57:31.967406922 -0500 > > @@ -1,2 +1,2 @@ > > QA output created by 529 > > -list attr: Numerical result out of range > > +set evm: Operation not permitted > > > > This patch uses security.capability which also sorts where it needs to > > do for the test and also accepts an unformatted buffer. > > > > Signed-off-by: Jeff Mahoney <jeffm@suse.com> > > --- > > src/t_attr_corruption.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/src/t_attr_corruption.c b/src/t_attr_corruption.c > > index f26611f9..0c229dbc 100644 > > --- a/src/t_attr_corruption.c > > +++ b/src/t_attr_corruption.c > > @@ -76,7 +76,7 @@ int main(int argc, char *argv[]) > > if (ret) > > die("set posix acl"); > > > > - ret = fsetxattr(fd, "security.evm", buf, 1, 1); > > + ret = fsetxattr(fd, "security.capability", buf, 1, 1); > > This fails for me both with and without EVM configured into my kernel: > > fsetxattr(3, "security.capability", "\3", 1, XATTR_CREATE) = -1 EINVAL (Invalid argument) > > Judging from fs/xattr.c it looks as though security.capability also has > a defined format that's parsed by security/commoncap.c... And now that I've figured out how to reproduce the xfs bug without a second attribute, NAK to this and I'll send a more complete fix shortly. --D > --D > > > if (ret) > > die("set evm"); > > > > -- > > 2.16.4 > > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] src/t_attr_corruption: use security.capability instead of security.evm 2019-02-25 23:26 ` Darrick J. Wong @ 2019-02-26 2:07 ` Xiao Yang 2019-02-26 3:33 ` Xiao Yang 1 sibling, 0 replies; 6+ messages in thread From: Xiao Yang @ 2019-02-26 2:07 UTC (permalink / raw) To: Darrick J. Wong; +Cc: jeffm, fstests On 2019/02/26 7:26, Darrick J. Wong wrote: > On Mon, Feb 25, 2019 at 02:47:44PM -0800, Darrick J. Wong wrote: >> On Mon, Feb 25, 2019 at 02:10:52PM -0500, jeffm@suse.com wrote: >>> From: Jeff Mahoney<jeffm@suse.com> >>> >>> src/t_attr_corruption uses the security.evm extended attribute because >>> it sorts before security.posix_acl_access. The security.evm attribute >>> is a formatted structure and when passed an uninitialized buffer, it >>> will fail with EPERM. >>> >>> We see test failures like: >>> --- tests/generic/529.out2019-02-21 13:22:47.583406922 -0500 >>> +++ /opt/xfstests/results//generic/529.out.bad 2019-02-21 13:57:31.967406922 -0500 >>> @@ -1,2 +1,2 @@ >>> QA output created by 529 >>> -list attr: Numerical result out of range >>> +set evm: Operation not permitted >>> >>> This patch uses security.capability which also sorts where it needs to >>> do for the test and also accepts an unformatted buffer. >>> >>> Signed-off-by: Jeff Mahoney<jeffm@suse.com> >>> --- >>> src/t_attr_corruption.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/src/t_attr_corruption.c b/src/t_attr_corruption.c >>> index f26611f9..0c229dbc 100644 >>> --- a/src/t_attr_corruption.c >>> +++ b/src/t_attr_corruption.c >>> @@ -76,7 +76,7 @@ int main(int argc, char *argv[]) >>> if (ret) >>> die("set posix acl"); >>> >>> - ret = fsetxattr(fd, "security.evm", buf, 1, 1); >>> + ret = fsetxattr(fd, "security.capability", buf, 1, 1); >> This fails for me both with and without EVM configured into my kernel: >> >> fsetxattr(3, "security.capability", "\3", 1, XATTR_CREATE) = -1 EINVAL (Invalid argument) >> >> Judging from fs/xattr.c it looks as though security.capability also has >> a defined format that's parsed by security/commoncap.c... > And now that I've figured out how to reproduce the xfs bug without a > second attribute, NAK to this and I'll send a more complete fix shortly. > > --D Hi Darrick, Jeff It seems that fsetxattr() with an uninitialized buffer fell into the following xattr_data->type check at security/integrity/evm/evm_main.c in kernel: ---------------------------------------------------------------------------------------------------------------------------- int evm_inode_setxattr(struct dentry *dentry, const char *xattr_name, const void *xattr_value, size_t xattr_value_len) { ... if (strcmp(xattr_name, XATTR_NAME_EVM) == 0) { if (!xattr_value_len) return -EINVAL; if (xattr_data->type != EVM_IMA_XATTR_DIGSIG && xattr_data->type != EVM_XATTR_PORTABLE_DIGSIG) return -EPERM; } ... } ---------------------------------------------------------------------------------------------------------------------------- Perhaps, we can skip this check by setting security.evm attribute to special value(e.g. EVM_IMA_XATTR_DIGSIG or EVM_XATTR_PORTABLE_DIGSIG) . For example: --------------------------------------------------------------------------------------------------------------------- diff --git a/src/t_attr_corruption.c b/root/t_attr_corruption.c index 1fa5e41f..05ce5b46 100644 --- a/src/t_attr_corruption.c +++ b/root/t_attr_corruption.c @@ -61,6 +61,7 @@ int main(int argc, char *argv[]) ssize_t sz; int fd; int ret; + int eval = 0x03; if (argc > 1) { ret = chdir(argv[1]); @@ -76,7 +77,7 @@ int main(int argc, char *argv[]) if (ret) die("set posix acl"); - ret = fsetxattr(fd, "security.evm", buf, 1, 1); + ret = fsetxattr(fd, "security.evm", &eval, sizeof(eval), 1); --------------------------------------------------------------------------------------------------------------------- Best Regards, Xiao Yang >> --D >> >>> if (ret) >>> die("set evm"); >>> >>> -- >>> 2.16.4 >>> > > ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] src/t_attr_corruption: use security.capability instead of security.evm 2019-02-25 23:26 ` Darrick J. Wong 2019-02-26 2:07 ` Xiao Yang @ 2019-02-26 3:33 ` Xiao Yang 1 sibling, 0 replies; 6+ messages in thread From: Xiao Yang @ 2019-02-26 3:33 UTC (permalink / raw) To: Darrick J. Wong; +Cc: jeffm, fstests On 2019/02/26 7:26, Darrick J. Wong wrote: > On Mon, Feb 25, 2019 at 02:47:44PM -0800, Darrick J. Wong wrote: >> On Mon, Feb 25, 2019 at 02:10:52PM -0500, jeffm@suse.com wrote: >>> From: Jeff Mahoney<jeffm@suse.com> >>> >>> src/t_attr_corruption uses the security.evm extended attribute because >>> it sorts before security.posix_acl_access. The security.evm attribute >>> is a formatted structure and when passed an uninitialized buffer, it >>> will fail with EPERM. >>> >>> We see test failures like: >>> --- tests/generic/529.out2019-02-21 13:22:47.583406922 -0500 >>> +++ /opt/xfstests/results//generic/529.out.bad 2019-02-21 13:57:31.967406922 -0500 >>> @@ -1,2 +1,2 @@ >>> QA output created by 529 >>> -list attr: Numerical result out of range >>> +set evm: Operation not permitted >>> >>> This patch uses security.capability which also sorts where it needs to >>> do for the test and also accepts an unformatted buffer. >>> >>> Signed-off-by: Jeff Mahoney<jeffm@suse.com> >>> --- >>> src/t_attr_corruption.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/src/t_attr_corruption.c b/src/t_attr_corruption.c >>> index f26611f9..0c229dbc 100644 >>> --- a/src/t_attr_corruption.c >>> +++ b/src/t_attr_corruption.c >>> @@ -76,7 +76,7 @@ int main(int argc, char *argv[]) >>> if (ret) >>> die("set posix acl"); >>> >>> - ret = fsetxattr(fd, "security.evm", buf, 1, 1); >>> + ret = fsetxattr(fd, "security.capability", buf, 1, 1); >> This fails for me both with and without EVM configured into my kernel: >> >> fsetxattr(3, "security.capability", "\3", 1, XATTR_CREATE) = -1 EINVAL (Invalid argument) >> >> Judging from fs/xattr.c it looks as though security.capability also has >> a defined format that's parsed by security/commoncap.c... > And now that I've figured out how to reproduce the xfs bug without a > second attribute, NAK to this and I'll send a more complete fix shortly. > > --D Hi Darrick, Sorry, i missed this comment. Your v2 patch is better and simpler to me. Best Regards, Xiao Yang >> --D >> >>> if (ret) >>> die("set evm"); >>> >>> -- >>> 2.16.4 >>> > > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] src/t_attr_corruption: use security.capability instead of security.evm 2019-02-25 22:47 ` Darrick J. Wong 2019-02-25 23:26 ` Darrick J. Wong @ 2019-02-26 1:39 ` Jeff Mahoney 1 sibling, 0 replies; 6+ messages in thread From: Jeff Mahoney @ 2019-02-26 1:39 UTC (permalink / raw) To: Darrick J. Wong; +Cc: fstests [-- Attachment #1.1: Type: text/plain, Size: 2016 bytes --] On 2/25/19 5:47 PM, Darrick J. Wong wrote: > On Mon, Feb 25, 2019 at 02:10:52PM -0500, jeffm@suse.com wrote: >> From: Jeff Mahoney <jeffm@suse.com> >> >> src/t_attr_corruption uses the security.evm extended attribute because >> it sorts before security.posix_acl_access. The security.evm attribute >> is a formatted structure and when passed an uninitialized buffer, it >> will fail with EPERM. >> >> We see test failures like: >> --- tests/generic/529.out2019-02-21 13:22:47.583406922 -0500 >> +++ /opt/xfstests/results//generic/529.out.bad 2019-02-21 13:57:31.967406922 -0500 >> @@ -1,2 +1,2 @@ >> QA output created by 529 >> -list attr: Numerical result out of range >> +set evm: Operation not permitted >> >> This patch uses security.capability which also sorts where it needs to >> do for the test and also accepts an unformatted buffer. >> >> Signed-off-by: Jeff Mahoney <jeffm@suse.com> >> --- >> src/t_attr_corruption.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/src/t_attr_corruption.c b/src/t_attr_corruption.c >> index f26611f9..0c229dbc 100644 >> --- a/src/t_attr_corruption.c >> +++ b/src/t_attr_corruption.c >> @@ -76,7 +76,7 @@ int main(int argc, char *argv[]) >> if (ret) >> die("set posix acl"); >> >> - ret = fsetxattr(fd, "security.evm", buf, 1, 1); >> + ret = fsetxattr(fd, "security.capability", buf, 1, 1); > > This fails for me both with and without EVM configured into my kernel: > > fsetxattr(3, "security.capability", "\3", 1, XATTR_CREATE) = -1 EINVAL (Invalid argument) > > Judging from fs/xattr.c it looks as though security.capability also has > a defined format that's parsed by security/commoncap.c... Huh. Ok. That worked on my SLE12 SP3 host (4.4.x) but now that I test it on a 5.0-rc, I get EINVAL too. It looks like prior to v3 caps (v4.14), validation only happened when they were loaded as part of prepare_binprm. -Jeff -- Jeff Mahoney SUSE Labs [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-02-26 3:33 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2019-02-25 19:10 [PATCH] src/t_attr_corruption: use security.capability instead of security.evm jeffm 2019-02-25 22:47 ` Darrick J. Wong 2019-02-25 23:26 ` Darrick J. Wong 2019-02-26 2:07 ` Xiao Yang 2019-02-26 3:33 ` Xiao Yang 2019-02-26 1:39 ` Jeff Mahoney
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox