* [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 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
* 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
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 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.