From e7d021e28328e0cc47b21cb9c6d8885326b0c2f5 Mon Sep 17 00:00:00 2001 From: Dmitry Kasatkin Date: Fri, 15 Aug 2014 14:09:19 +0300 Subject: evm: fix checkpatch warnings This patch fixes checkpatch 'return' warnings introduced with commit 9819cf2 "checkpatch: warn on unnecessary void function return statements". Use scripts/checkpatch.pl --file security/integrity/evm/evm_main.c to produce the warnings. Signed-off-by: Dmitry Kasatkin Signed-off-by: Mimi Zohar --- security/integrity/evm/evm_main.c | 3 --- 1 file changed, 3 deletions(-) (limited to 'security/integrity/evm/evm_main.c') diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c index 3bcb80df4d0..fb71f55295d 100644 --- a/security/integrity/evm/evm_main.c +++ b/security/integrity/evm/evm_main.c @@ -352,7 +352,6 @@ void evm_inode_post_setxattr(struct dentry *dentry, const char *xattr_name, return; evm_update_evmxattr(dentry, xattr_name, xattr_value, xattr_value_len); - return; } /** @@ -372,7 +371,6 @@ void evm_inode_post_removexattr(struct dentry *dentry, const char *xattr_name) mutex_lock(&inode->i_mutex); evm_update_evmxattr(dentry, xattr_name, NULL, 0); mutex_unlock(&inode->i_mutex); - return; } /** @@ -414,7 +412,6 @@ void evm_inode_post_setattr(struct dentry *dentry, int ia_valid) if (ia_valid & (ATTR_MODE | ATTR_UID | ATTR_GID)) evm_update_evmxattr(dentry, NULL, NULL, 0); - return; } /* -- cgit v1.2.3-70-g09d2 From 1f1009791b2e81f106d4809007720495ba3ed90c Mon Sep 17 00:00:00 2001 From: Dmitry Kasatkin Date: Fri, 15 Aug 2014 13:49:22 +0300 Subject: evm: prevent passing integrity check if xattr read fails This patch fixes a bug, where evm_verify_hmac() returns INTEGRITY_PASS if inode->i_op->getxattr() returns an error in evm_find_protected_xattrs. Signed-off-by: Dmitry Kasatkin --- security/integrity/evm/evm_main.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) (limited to 'security/integrity/evm/evm_main.c') diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c index fb71f55295d..40220124364 100644 --- a/security/integrity/evm/evm_main.c +++ b/security/integrity/evm/evm_main.c @@ -126,14 +126,15 @@ static enum integrity_status evm_verify_hmac(struct dentry *dentry, rc = vfs_getxattr_alloc(dentry, XATTR_NAME_EVM, (char **)&xattr_data, 0, GFP_NOFS); if (rc <= 0) { - if (rc == 0) - evm_status = INTEGRITY_FAIL; /* empty */ - else if (rc == -ENODATA) { + evm_status = INTEGRITY_FAIL; + if (rc == -ENODATA) { rc = evm_find_protected_xattrs(dentry); if (rc > 0) evm_status = INTEGRITY_NOLABEL; else if (rc == 0) evm_status = INTEGRITY_NOXATTRS; /* new file */ + } else if (rc == -EOPNOTSUPP) { + evm_status = INTEGRITY_UNKNOWN; } goto out; } -- cgit v1.2.3-70-g09d2 From 3dcbad52cf18c3c379e96b992d22815439ebbe53 Mon Sep 17 00:00:00 2001 From: Dmitry Kasatkin Date: Tue, 2 Sep 2014 16:31:43 +0300 Subject: evm: properly handle INTEGRITY_NOXATTRS EVM status Unless an LSM labels a file during d_instantiate(), newly created files are not labeled with an initial security.evm xattr, until the file closes. EVM, before allowing a protected, security xattr to be written, verifies the existing 'security.evm' value is good. For newly created files without a security.evm label, this verification prevents writing any protected, security xattrs, until the file closes. Following is the example when this happens: fd = open("foo", O_CREAT | O_WRONLY, 0644); setxattr("foo", "security.SMACK64", value, sizeof(value), 0); close(fd); While INTEGRITY_NOXATTRS status is handled in other places, such as evm_inode_setattr(), it does not handle it in all cases in evm_protect_xattr(). By limiting the use of INTEGRITY_NOXATTRS to newly created files, we can now allow setting "protected" xattrs. Changelog: - limit the use of INTEGRITY_NOXATTRS to IMA identified new files Signed-off-by: Dmitry Kasatkin Signed-off-by: Mimi Zohar Cc: 3.14+ --- security/integrity/evm/evm_main.c | 7 +++++++ 1 file changed, 7 insertions(+) (limited to 'security/integrity/evm/evm_main.c') diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c index 40220124364..9685af330de 100644 --- a/security/integrity/evm/evm_main.c +++ b/security/integrity/evm/evm_main.c @@ -285,6 +285,13 @@ static int evm_protect_xattr(struct dentry *dentry, const char *xattr_name, goto out; } evm_status = evm_verify_current_integrity(dentry); + if (evm_status == INTEGRITY_NOXATTRS) { + struct integrity_iint_cache *iint; + + iint = integrity_iint_find(dentry->d_inode); + if (iint && (iint->flags & IMA_NEW_FILE)) + return 0; + } out: if (evm_status != INTEGRITY_PASS) integrity_audit_msg(AUDIT_INTEGRITY_METADATA, dentry->d_inode, -- cgit v1.2.3-70-g09d2 From 3b1deef6b1289a99505858a3b212c5b50adf0c2f Mon Sep 17 00:00:00 2001 From: Dmitry Kasatkin Date: Tue, 28 Oct 2014 14:28:49 +0200 Subject: evm: check xattr value length and type in evm_inode_setxattr() evm_inode_setxattr() can be called with no value. The function does not check the length so that following command can be used to produce the kernel oops: setfattr -n security.evm FOO. This patch fixes it. Changes in v3: * there is no reason to return different error codes for EVM_XATTR_HMAC and non EVM_XATTR_HMAC. Remove unnecessary test then. Changes in v2: * testing for validity of xattr type [ 1106.396921] BUG: unable to handle kernel NULL pointer dereference at (null) [ 1106.398192] IP: [] evm_inode_setxattr+0x2a/0x48 [ 1106.399244] PGD 29048067 PUD 290d7067 PMD 0 [ 1106.399953] Oops: 0000 [#1] SMP [ 1106.400020] Modules linked in: bridge stp llc evdev serio_raw i2c_piix4 button fuse [ 1106.400020] CPU: 0 PID: 3635 Comm: setxattr Not tainted 3.16.0-kds+ #2936 [ 1106.400020] Hardware name: Bochs Bochs, BIOS Bochs 01/01/2011 [ 1106.400020] task: ffff8800291a0000 ti: ffff88002917c000 task.ti: ffff88002917c000 [ 1106.400020] RIP: 0010:[] [] evm_inode_setxattr+0x2a/0x48 [ 1106.400020] RSP: 0018:ffff88002917fd50 EFLAGS: 00010246 [ 1106.400020] RAX: 0000000000000000 RBX: ffff88002917fdf8 RCX: 0000000000000000 [ 1106.400020] RDX: 0000000000000000 RSI: ffffffff818136d3 RDI: ffff88002917fdf8 [ 1106.400020] RBP: ffff88002917fd68 R08: 0000000000000000 R09: 00000000003ec1df [ 1106.400020] R10: 0000000000000000 R11: 0000000000000000 R12: ffff8800438a0a00 [ 1106.400020] R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000 [ 1106.400020] FS: 00007f7dfa7d7740(0000) GS:ffff88005da00000(0000) knlGS:0000000000000000 [ 1106.400020] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 1106.400020] CR2: 0000000000000000 CR3: 000000003763e000 CR4: 00000000000006f0 [ 1106.400020] Stack: [ 1106.400020] ffff8800438a0a00 ffff88002917fdf8 0000000000000000 ffff88002917fd98 [ 1106.400020] ffffffff812a1030 ffff8800438a0a00 ffff88002917fdf8 0000000000000000 [ 1106.400020] 0000000000000000 ffff88002917fde0 ffffffff8116d08a ffff88002917fdc8 [ 1106.400020] Call Trace: [ 1106.400020] [] security_inode_setxattr+0x5d/0x6a [ 1106.400020] [] vfs_setxattr+0x6b/0x9f [ 1106.400020] [] setxattr+0x122/0x16c [ 1106.400020] [] ? mnt_want_write+0x21/0x45 [ 1106.400020] [] ? __sb_start_write+0x10f/0x143 [ 1106.400020] [] ? mnt_want_write+0x21/0x45 [ 1106.400020] [] ? __mnt_want_write+0x48/0x4f [ 1106.400020] [] SyS_setxattr+0x6e/0xb0 [ 1106.400020] [] system_call_fastpath+0x16/0x1b [ 1106.400020] Code: c3 0f 1f 44 00 00 55 48 89 e5 41 55 49 89 d5 41 54 49 89 fc 53 48 89 f3 48 c7 c6 d3 36 81 81 48 89 df e8 18 22 04 00 85 c0 75 07 <41> 80 7d 00 02 74 0d 48 89 de 4c 89 e7 e8 5a fe ff ff eb 03 83 [ 1106.400020] RIP [] evm_inode_setxattr+0x2a/0x48 [ 1106.400020] RSP [ 1106.400020] CR2: 0000000000000000 [ 1106.428061] ---[ end trace ae08331628ba3050 ]--- Reported-by: Jan Kara Signed-off-by: Dmitry Kasatkin Cc: stable@vger.kernel.org Signed-off-by: Mimi Zohar --- security/integrity/evm/evm_main.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) (limited to 'security/integrity/evm/evm_main.c') diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c index 9685af330de..c5ee1a7c5e8 100644 --- a/security/integrity/evm/evm_main.c +++ b/security/integrity/evm/evm_main.c @@ -319,9 +319,12 @@ int evm_inode_setxattr(struct dentry *dentry, const char *xattr_name, { const struct evm_ima_xattr_data *xattr_data = xattr_value; - if ((strcmp(xattr_name, XATTR_NAME_EVM) == 0) - && (xattr_data->type == EVM_XATTR_HMAC)) - return -EPERM; + if (strcmp(xattr_name, XATTR_NAME_EVM) == 0) { + if (!xattr_value_len) + return -EINVAL; + if (xattr_data->type != EVM_IMA_XATTR_DIGSIG) + return -EPERM; + } return evm_protect_xattr(dentry, xattr_name, xattr_value, xattr_value_len); } -- cgit v1.2.3-70-g09d2