Re: [PATCH] src/t_attr_corruption: use security.capability instead of security.evm

[Date Prev] [Date Next] [Thread Prev] [Thread Next] [Date Index] [Thread Index]



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@xxxxxxxx wrote:
From: Jeff Mahoney<jeffm@xxxxxxxx>

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@xxxxxxxx>
---
  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









[Index of Archives]     [Linux Filesystems Development]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux