From 0c343af8065be5ceb0c03a876af7c513e960e2ff Mon Sep 17 00:00:00 2001 From: Matthew Garrett Date: Fri, 11 May 2018 16:12:34 -0700 Subject: integrity: Add an integrity directory in securityfs We want to add additional evm control nodes, and it'd be preferable not to clutter up the securityfs root directory any further. Create a new integrity directory, move the ima directory into it, create an evm directory for the evm attribute and add compatibility symlinks. Signed-off-by: Matthew Garrett Signed-off-by: Mimi Zohar --- security/integrity/evm/evm_secfs.c | 27 ++++++++++++++++++++++++--- 1 file changed, 24 insertions(+), 3 deletions(-) (limited to 'security/integrity/evm') diff --git a/security/integrity/evm/evm_secfs.c b/security/integrity/evm/evm_secfs.c index feba03bbedae..e44380f0cb45 100644 --- a/security/integrity/evm/evm_secfs.c +++ b/security/integrity/evm/evm_secfs.c @@ -19,7 +19,9 @@ #include #include "evm.h" +static struct dentry *evm_dir; static struct dentry *evm_init_tpm; +static struct dentry *evm_symlink; /** * evm_read_key - read() for /evm @@ -111,9 +113,28 @@ int __init evm_init_secfs(void) { int error = 0; - evm_init_tpm = securityfs_create_file("evm", S_IRUSR | S_IRGRP, - NULL, NULL, &evm_key_ops); - if (!evm_init_tpm || IS_ERR(evm_init_tpm)) + evm_dir = securityfs_create_dir("evm", integrity_dir); + if (!evm_dir || IS_ERR(evm_dir)) + return -EFAULT; + + evm_init_tpm = securityfs_create_file("evm", 0660, + evm_dir, NULL, &evm_key_ops); + if (!evm_init_tpm || IS_ERR(evm_init_tpm)) { + error = -EFAULT; + goto out; + } + + evm_symlink = securityfs_create_symlink("evm", NULL, + "integrity/evm/evm", NULL); + if (!evm_symlink || IS_ERR(evm_symlink)) { error = -EFAULT; + goto out; + } + + return 0; +out: + securityfs_remove(evm_symlink); + securityfs_remove(evm_init_tpm); + securityfs_remove(evm_dir); return error; } -- cgit v1.2.3-59-g8ed1b From 21af76631476030709f85f48e20bb9429a912b6f Mon Sep 17 00:00:00 2001 From: Matthew Garrett Date: Fri, 11 May 2018 16:12:35 -0700 Subject: EVM: turn evm_config_xattrnames into a list Use a list of xattrs rather than an array - this makes it easier to extend the list at runtime. Signed-off-by: Matthew Garrett Reviewed-by: James Morris Signed-off-by: Mimi Zohar --- security/integrity/evm/evm.h | 7 +++- security/integrity/evm/evm_crypto.c | 10 ++--- security/integrity/evm/evm_main.c | 79 +++++++++++++++++++++---------------- 3 files changed, 57 insertions(+), 39 deletions(-) (limited to 'security/integrity/evm') diff --git a/security/integrity/evm/evm.h b/security/integrity/evm/evm.h index 45c4a89c02ff..1257c3c24723 100644 --- a/security/integrity/evm/evm.h +++ b/security/integrity/evm/evm.h @@ -30,6 +30,11 @@ #define EVM_INIT_MASK (EVM_INIT_HMAC | EVM_INIT_X509 | EVM_SETUP_COMPLETE | \ EVM_ALLOW_METADATA_WRITES) +struct xattr_list { + struct list_head list; + char *name; +}; + extern int evm_initialized; #define EVM_ATTR_FSUUID 0x0001 @@ -40,7 +45,7 @@ extern struct crypto_shash *hmac_tfm; extern struct crypto_shash *hash_tfm; /* List of EVM protected security xattrs */ -extern char *evm_config_xattrnames[]; +extern struct list_head evm_config_xattrnames; int evm_init_key(void); int evm_update_evmxattr(struct dentry *dentry, diff --git a/security/integrity/evm/evm_crypto.c b/security/integrity/evm/evm_crypto.c index a46fba322340..caeea20670cc 100644 --- a/security/integrity/evm/evm_crypto.c +++ b/security/integrity/evm/evm_crypto.c @@ -192,8 +192,8 @@ static int evm_calc_hmac_or_hash(struct dentry *dentry, char type, char *digest) { struct inode *inode = d_backing_inode(dentry); + struct xattr_list *xattr; struct shash_desc *desc; - char **xattrname; size_t xattr_size = 0; char *xattr_value = NULL; int error; @@ -208,14 +208,14 @@ static int evm_calc_hmac_or_hash(struct dentry *dentry, return PTR_ERR(desc); error = -ENODATA; - for (xattrname = evm_config_xattrnames; *xattrname != NULL; xattrname++) { + list_for_each_entry(xattr, &evm_config_xattrnames, list) { bool is_ima = false; - if (strcmp(*xattrname, XATTR_NAME_IMA) == 0) + if (strcmp(xattr->name, XATTR_NAME_IMA) == 0) is_ima = true; if ((req_xattr_name && req_xattr_value) - && !strcmp(*xattrname, req_xattr_name)) { + && !strcmp(xattr->name, req_xattr_name)) { error = 0; crypto_shash_update(desc, (const u8 *)req_xattr_value, req_xattr_value_len); @@ -223,7 +223,7 @@ static int evm_calc_hmac_or_hash(struct dentry *dentry, ima_present = true; continue; } - size = vfs_getxattr_alloc(dentry, *xattrname, + size = vfs_getxattr_alloc(dentry, xattr->name, &xattr_value, xattr_size, GFP_NOFS); if (size == -ENOMEM) { error = -ENOMEM; diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c index 9ea9c19a545c..09582d4fc4a8 100644 --- a/security/integrity/evm/evm_main.c +++ b/security/integrity/evm/evm_main.c @@ -35,28 +35,29 @@ static const char * const integrity_status_msg[] = { }; int evm_hmac_attrs; -char *evm_config_xattrnames[] = { +static struct xattr_list evm_config_default_xattrnames[] __ro_after_init = { #ifdef CONFIG_SECURITY_SELINUX - XATTR_NAME_SELINUX, + {.name = XATTR_NAME_SELINUX}, #endif #ifdef CONFIG_SECURITY_SMACK - XATTR_NAME_SMACK, + {.name = XATTR_NAME_SMACK}, #ifdef CONFIG_EVM_EXTRA_SMACK_XATTRS - XATTR_NAME_SMACKEXEC, - XATTR_NAME_SMACKTRANSMUTE, - XATTR_NAME_SMACKMMAP, + {.name = XATTR_NAME_SMACKEXEC}, + {.name = XATTR_NAME_SMACKTRANSMUTE}, + {.name = XATTR_NAME_SMACKMMAP}, #endif #endif #ifdef CONFIG_SECURITY_APPARMOR - XATTR_NAME_APPARMOR, + {.name = XATTR_NAME_APPARMOR}, #endif #ifdef CONFIG_IMA_APPRAISE - XATTR_NAME_IMA, + {.name = XATTR_NAME_IMA}, #endif - XATTR_NAME_CAPS, - NULL + {.name = XATTR_NAME_CAPS}, }; +LIST_HEAD(evm_config_xattrnames); + static int evm_fixmode; static int __init evm_set_fixmode(char *str) { @@ -68,6 +69,17 @@ __setup("evm=", evm_set_fixmode); static void __init evm_init_config(void) { + int i, xattrs; + + xattrs = ARRAY_SIZE(evm_config_default_xattrnames); + + pr_info("Initialising EVM extended attributes:\n"); + for (i = 0; i < xattrs; i++) { + pr_info("%s\n", evm_config_default_xattrnames[i].name); + list_add_tail(&evm_config_default_xattrnames[i].list, + &evm_config_xattrnames); + } + #ifdef CONFIG_EVM_ATTR_FSUUID evm_hmac_attrs |= EVM_ATTR_FSUUID; #endif @@ -82,15 +94,15 @@ static bool evm_key_loaded(void) static int evm_find_protected_xattrs(struct dentry *dentry) { struct inode *inode = d_backing_inode(dentry); - char **xattr; + struct xattr_list *xattr; int error; int count = 0; if (!(inode->i_opflags & IOP_XATTR)) return -EOPNOTSUPP; - for (xattr = evm_config_xattrnames; *xattr != NULL; xattr++) { - error = __vfs_getxattr(dentry, inode, *xattr, NULL, 0); + list_for_each_entry(xattr, &evm_config_xattrnames, list) { + error = __vfs_getxattr(dentry, inode, xattr->name, NULL, 0); if (error < 0) { if (error == -ENODATA) continue; @@ -211,24 +223,25 @@ out: static int evm_protected_xattr(const char *req_xattr_name) { - char **xattrname; int namelen; int found = 0; + struct xattr_list *xattr; namelen = strlen(req_xattr_name); - for (xattrname = evm_config_xattrnames; *xattrname != NULL; xattrname++) { - if ((strlen(*xattrname) == namelen) - && (strncmp(req_xattr_name, *xattrname, namelen) == 0)) { + list_for_each_entry(xattr, &evm_config_xattrnames, list) { + if ((strlen(xattr->name) == namelen) + && (strncmp(req_xattr_name, xattr->name, namelen) == 0)) { found = 1; break; } if (strncmp(req_xattr_name, - *xattrname + XATTR_SECURITY_PREFIX_LEN, + xattr->name + XATTR_SECURITY_PREFIX_LEN, strlen(req_xattr_name)) == 0) { found = 1; break; } } + return found; } @@ -544,35 +557,35 @@ void __init evm_load_x509(void) static int __init init_evm(void) { int error; + struct list_head *pos, *q; + struct xattr_list *xattr; evm_init_config(); error = integrity_init_keyring(INTEGRITY_KEYRING_EVM); if (error) - return error; + goto error; error = evm_init_secfs(); if (error < 0) { pr_info("Error registering secfs\n"); - return error; + goto error; } - return 0; -} - -/* - * evm_display_config - list the EVM protected security extended attributes - */ -static int __init evm_display_config(void) -{ - char **xattrname; +error: + if (error != 0) { + if (!list_empty(&evm_config_xattrnames)) { + list_for_each_safe(pos, q, &evm_config_xattrnames) { + xattr = list_entry(pos, struct xattr_list, + list); + list_del(pos); + } + } + } - for (xattrname = evm_config_xattrnames; *xattrname != NULL; xattrname++) - pr_info("%s\n", *xattrname); - return 0; + return error; } -pure_initcall(evm_display_config); late_initcall(init_evm); MODULE_DESCRIPTION("Extended Verification Module"); -- cgit v1.2.3-59-g8ed1b From fa516b66a1bfce1d72f1620c54bdfebc493000d1 Mon Sep 17 00:00:00 2001 From: Matthew Garrett Date: Tue, 15 May 2018 10:38:26 -0700 Subject: EVM: Allow runtime modification of the set of verified xattrs Sites may wish to provide additional metadata alongside files in order to make more fine-grained security decisions[1]. The security of this is enhanced if this metadata is protected, something that EVM makes possible. However, the kernel cannot know about the set of extended attributes that local admins may wish to protect, and hardcoding this policy in the kernel makes it difficult to change over time and less convenient for distributions to enable. This patch adds a new /sys/kernel/security/integrity/evm/evm_xattrs node, which can be read to obtain the current set of EVM-protected extended attributes or written to in order to add new entries. Extending this list will not change the validity of any existing signatures provided that the file in question does not have any of the additional extended attributes - missing xattrs are skipped when calculating the EVM hash. [1] For instance, a package manager could install information about the package uploader in an additional extended attribute. Local LSM policy could then be associated with that extended attribute in order to restrict the privileges available to packages from less trusted uploaders. Signed-off-by: Matthew Garrett Reviewed-by: James Morris Signed-off-by: Mimi Zohar --- Documentation/ABI/testing/evm | 13 +++ include/uapi/linux/audit.h | 1 + security/integrity/evm/Kconfig | 11 +++ security/integrity/evm/evm_crypto.c | 2 +- security/integrity/evm/evm_main.c | 6 +- security/integrity/evm/evm_secfs.c | 173 ++++++++++++++++++++++++++++++++++++ 6 files changed, 202 insertions(+), 4 deletions(-) (limited to 'security/integrity/evm') diff --git a/Documentation/ABI/testing/evm b/Documentation/ABI/testing/evm index d12cb2eae9ee..201d10319fa1 100644 --- a/Documentation/ABI/testing/evm +++ b/Documentation/ABI/testing/evm @@ -57,3 +57,16 @@ Description: dracut (via 97masterkey and 98integrity) and systemd (via core/ima-setup) have support for loading keys at boot time. + +What: security/integrity/evm/evm_xattrs +Date: April 2018 +Contact: Matthew Garrett +Description: + Shows the set of extended attributes used to calculate or + validate the EVM signature, and allows additional attributes + to be added at runtime. Any signatures generated after + additional attributes are added (and on files posessing those + additional attributes) will only be valid if the same + additional attributes are configured on system boot. Writing + a single period (.) will lock the xattr list from any further + modification. diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h index 4e61a9e05132..65d9293f1fb8 100644 --- a/include/uapi/linux/audit.h +++ b/include/uapi/linux/audit.h @@ -147,6 +147,7 @@ #define AUDIT_INTEGRITY_HASH 1803 /* Integrity HASH type */ #define AUDIT_INTEGRITY_PCR 1804 /* PCR invalidation msgs */ #define AUDIT_INTEGRITY_RULE 1805 /* policy rule */ +#define AUDIT_INTEGRITY_EVM_XATTR 1806 /* New EVM-covered xattr */ #define AUDIT_KERNEL 2000 /* Asynchronous audit record. NOT A REQUEST. */ diff --git a/security/integrity/evm/Kconfig b/security/integrity/evm/Kconfig index e825e0ae78e7..d593346d0bba 100644 --- a/security/integrity/evm/Kconfig +++ b/security/integrity/evm/Kconfig @@ -42,6 +42,17 @@ config EVM_EXTRA_SMACK_XATTRS additional info to the calculation, requires existing EVM labeled file systems to be relabeled. +config EVM_ADD_XATTRS + bool "Add additional EVM extended attributes at runtime" + depends on EVM + default n + help + Allow userland to provide additional xattrs for HMAC calculation. + + When this option is enabled, root can add additional xattrs to the + list used by EVM by writing them into + /sys/kernel/security/integrity/evm/evm_xattrs. + config EVM_LOAD_X509 bool "Load an X509 certificate onto the '.evm' trusted keyring" depends on EVM && INTEGRITY_TRUSTED_KEYRING diff --git a/security/integrity/evm/evm_crypto.c b/security/integrity/evm/evm_crypto.c index caeea20670cc..494da5fcc092 100644 --- a/security/integrity/evm/evm_crypto.c +++ b/security/integrity/evm/evm_crypto.c @@ -208,7 +208,7 @@ static int evm_calc_hmac_or_hash(struct dentry *dentry, return PTR_ERR(desc); error = -ENODATA; - list_for_each_entry(xattr, &evm_config_xattrnames, list) { + list_for_each_entry_rcu(xattr, &evm_config_xattrnames, list) { bool is_ima = false; if (strcmp(xattr->name, XATTR_NAME_IMA) == 0) diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c index 09582d4fc4a8..f9eff5041e4c 100644 --- a/security/integrity/evm/evm_main.c +++ b/security/integrity/evm/evm_main.c @@ -35,7 +35,7 @@ static const char * const integrity_status_msg[] = { }; int evm_hmac_attrs; -static struct xattr_list evm_config_default_xattrnames[] __ro_after_init = { +static struct xattr_list evm_config_default_xattrnames[] = { #ifdef CONFIG_SECURITY_SELINUX {.name = XATTR_NAME_SELINUX}, #endif @@ -101,7 +101,7 @@ static int evm_find_protected_xattrs(struct dentry *dentry) if (!(inode->i_opflags & IOP_XATTR)) return -EOPNOTSUPP; - list_for_each_entry(xattr, &evm_config_xattrnames, list) { + list_for_each_entry_rcu(xattr, &evm_config_xattrnames, list) { error = __vfs_getxattr(dentry, inode, xattr->name, NULL, 0); if (error < 0) { if (error == -ENODATA) @@ -228,7 +228,7 @@ static int evm_protected_xattr(const char *req_xattr_name) struct xattr_list *xattr; namelen = strlen(req_xattr_name); - list_for_each_entry(xattr, &evm_config_xattrnames, list) { + list_for_each_entry_rcu(xattr, &evm_config_xattrnames, list) { if ((strlen(xattr->name) == namelen) && (strncmp(req_xattr_name, xattr->name, namelen) == 0)) { found = 1; diff --git a/security/integrity/evm/evm_secfs.c b/security/integrity/evm/evm_secfs.c index e44380f0cb45..a7a0a1acae99 100644 --- a/security/integrity/evm/evm_secfs.c +++ b/security/integrity/evm/evm_secfs.c @@ -15,14 +15,22 @@ #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt +#include #include #include +#include #include "evm.h" static struct dentry *evm_dir; static struct dentry *evm_init_tpm; static struct dentry *evm_symlink; +#ifdef CONFIG_EVM_ADD_XATTRS +static struct dentry *evm_xattrs; +static DEFINE_MUTEX(xattr_list_mutex); +static int evm_xattrs_locked; +#endif + /** * evm_read_key - read() for /evm * @@ -109,6 +117,166 @@ static const struct file_operations evm_key_ops = { .write = evm_write_key, }; +#ifdef CONFIG_EVM_ADD_XATTRS +/** + * evm_read_xattrs - read() for /evm_xattrs + * + * @filp: file pointer, not actually used + * @buf: where to put the result + * @count: maximum to send along + * @ppos: where to start + * + * Returns number of bytes read or error code, as appropriate + */ +static ssize_t evm_read_xattrs(struct file *filp, char __user *buf, + size_t count, loff_t *ppos) +{ + char *temp; + int offset = 0; + ssize_t rc, size = 0; + struct xattr_list *xattr; + + if (*ppos != 0) + return 0; + + rc = mutex_lock_interruptible(&xattr_list_mutex); + if (rc) + return -ERESTARTSYS; + + list_for_each_entry(xattr, &evm_config_xattrnames, list) + size += strlen(xattr->name) + 1; + + temp = kmalloc(size + 1, GFP_KERNEL); + if (!temp) + return -ENOMEM; + + list_for_each_entry(xattr, &evm_config_xattrnames, list) { + sprintf(temp + offset, "%s\n", xattr->name); + offset += strlen(xattr->name) + 1; + } + + mutex_unlock(&xattr_list_mutex); + rc = simple_read_from_buffer(buf, count, ppos, temp, strlen(temp)); + + return rc; +} + +/** + * evm_write_xattrs - write() for /evm_xattrs + * @file: file pointer, not actually used + * @buf: where to get the data from + * @count: bytes sent + * @ppos: where to start + * + * Returns number of bytes written or error code, as appropriate + */ +static ssize_t evm_write_xattrs(struct file *file, const char __user *buf, + size_t count, loff_t *ppos) +{ + int len, err; + struct xattr_list *xattr, *tmp; + struct audit_buffer *ab; + struct iattr newattrs; + struct inode *inode; + + if (!capable(CAP_SYS_ADMIN) || evm_xattrs_locked) + return -EPERM; + + if (*ppos != 0) + return -EINVAL; + + if (count > XATTR_NAME_MAX) + return -E2BIG; + + ab = audit_log_start(NULL, GFP_KERNEL, AUDIT_INTEGRITY_EVM_XATTR); + if (IS_ERR(ab)) + return PTR_ERR(ab); + + xattr = kmalloc(sizeof(struct xattr_list), GFP_KERNEL); + if (!xattr) { + err = -ENOMEM; + goto out; + } + + xattr->name = memdup_user_nul(buf, count); + if (IS_ERR(xattr->name)) { + err = PTR_ERR(xattr->name); + xattr->name = NULL; + goto out; + } + + /* Remove any trailing newline */ + len = strlen(xattr->name); + if (xattr->name[len-1] == '\n') + xattr->name[len-1] = '\0'; + + if (strcmp(xattr->name, ".") == 0) { + evm_xattrs_locked = 1; + newattrs.ia_mode = S_IFREG | 0440; + newattrs.ia_valid = ATTR_MODE; + inode = evm_xattrs->d_inode; + inode_lock(inode); + err = simple_setattr(evm_xattrs, &newattrs); + inode_unlock(inode); + audit_log_format(ab, "locked"); + if (!err) + err = count; + goto out; + } + + audit_log_format(ab, "xattr="); + audit_log_untrustedstring(ab, xattr->name); + + if (strncmp(xattr->name, XATTR_SECURITY_PREFIX, + XATTR_SECURITY_PREFIX_LEN) != 0) { + err = -EINVAL; + goto out; + } + + /* Guard against races in evm_read_xattrs */ + mutex_lock(&xattr_list_mutex); + list_for_each_entry(tmp, &evm_config_xattrnames, list) { + if (strcmp(xattr->name, tmp->name) == 0) { + err = -EEXIST; + mutex_unlock(&xattr_list_mutex); + goto out; + } + } + list_add_tail_rcu(&xattr->list, &evm_config_xattrnames); + mutex_unlock(&xattr_list_mutex); + + audit_log_format(ab, " res=0"); + audit_log_end(ab); + return count; +out: + audit_log_format(ab, " res=%d", err); + audit_log_end(ab); + kfree(xattr->name); + kfree(xattr); + return err; +} + +static const struct file_operations evm_xattr_ops = { + .read = evm_read_xattrs, + .write = evm_write_xattrs, +}; + +static int evm_init_xattrs(void) +{ + evm_xattrs = securityfs_create_file("evm_xattrs", 0660, evm_dir, NULL, + &evm_xattr_ops); + if (!evm_xattrs || IS_ERR(evm_xattrs)) + return -EFAULT; + + return 0; +} +#else +static int evm_init_xattrs(void) +{ + return 0; +} +#endif + int __init evm_init_secfs(void) { int error = 0; @@ -131,6 +299,11 @@ int __init evm_init_secfs(void) goto out; } + if (evm_init_xattrs() != 0) { + error = -EFAULT; + goto out; + } + return 0; out: securityfs_remove(evm_symlink); -- cgit v1.2.3-59-g8ed1b From 825b8650dc3dd064969ce343e918d0eb6bf907fb Mon Sep 17 00:00:00 2001 From: Colin Ian King Date: Sun, 27 May 2018 23:15:02 +0100 Subject: EVM: fix memory leak of temporary buffer 'temp' The allocation of 'temp' is not kfree'd and hence there is a memory leak on each call of evm_read_xattrs. Fix this by kfree'ing it after copying data from it back to the user space buffer 'buf'. Detected by CoverityScan, CID#1469386 ("Resource Leak") Fixes: fa516b66a1bf ("EVM: Allow runtime modification of the set of verified xattrs") Signed-off-by: Colin Ian King Signed-off-by: Mimi Zohar --- security/integrity/evm/evm_secfs.c | 2 ++ 1 file changed, 2 insertions(+) (limited to 'security/integrity/evm') diff --git a/security/integrity/evm/evm_secfs.c b/security/integrity/evm/evm_secfs.c index a7a0a1acae99..fb8bc950aceb 100644 --- a/security/integrity/evm/evm_secfs.c +++ b/security/integrity/evm/evm_secfs.c @@ -158,6 +158,8 @@ static ssize_t evm_read_xattrs(struct file *filp, char __user *buf, mutex_unlock(&xattr_list_mutex); rc = simple_read_from_buffer(buf, count, ppos, temp, strlen(temp)); + kfree(temp); + return rc; } -- cgit v1.2.3-59-g8ed1b From 72acd64df4561593d2ec3227b4aca9b0d7ded50e Mon Sep 17 00:00:00 2001 From: Colin Ian King Date: Sun, 27 May 2018 23:55:10 +0100 Subject: EVM: Fix null dereference on xattr when xattr fails to allocate In the case where the allocation of xattr fails and xattr is NULL, the error exit return path via label 'out' will dereference xattr when kfree'ing xattr-name. Fix this by only kfree'ing xattr->name and xattr when xattr is non-null. Detected by CoverityScan, CID#1469366 ("Dereference after null check") Fixes: fa516b66a1bf ("EVM: Allow runtime modification of the set of verified xattrs") Signed-off-by: Colin Ian King Signed-off-by: Mimi Zohar --- security/integrity/evm/evm_secfs.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) (limited to 'security/integrity/evm') diff --git a/security/integrity/evm/evm_secfs.c b/security/integrity/evm/evm_secfs.c index fb8bc950aceb..cf5cd303d7c0 100644 --- a/security/integrity/evm/evm_secfs.c +++ b/security/integrity/evm/evm_secfs.c @@ -253,8 +253,10 @@ static ssize_t evm_write_xattrs(struct file *file, const char __user *buf, out: audit_log_format(ab, " res=%d", err); audit_log_end(ab); - kfree(xattr->name); - kfree(xattr); + if (xattr) { + kfree(xattr->name); + kfree(xattr); + } return err; } -- cgit v1.2.3-59-g8ed1b From a41d80acfa2764e9b1ce49aa303a263e609d91f7 Mon Sep 17 00:00:00 2001 From: Dan Carpenter Date: Tue, 29 May 2018 16:11:28 +0300 Subject: EVM: prevent array underflow in evm_write_xattrs() If the user sets xattr->name[0] to NUL then we would read one character before the start of the array. This bug seems harmless as far as I can see but perhaps it would trigger a warning in KASAN. Fixes: fa516b66a1bf ("EVM: Allow runtime modification of the set of verified xattrs") Signed-off-by: Dan Carpenter Signed-off-by: Mimi Zohar --- security/integrity/evm/evm_secfs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'security/integrity/evm') diff --git a/security/integrity/evm/evm_secfs.c b/security/integrity/evm/evm_secfs.c index cf5cd303d7c0..3cefef3919e5 100644 --- a/security/integrity/evm/evm_secfs.c +++ b/security/integrity/evm/evm_secfs.c @@ -209,7 +209,7 @@ static ssize_t evm_write_xattrs(struct file *file, const char __user *buf, /* Remove any trailing newline */ len = strlen(xattr->name); - if (xattr->name[len-1] == '\n') + if (len && xattr->name[len-1] == '\n') xattr->name[len-1] = '\0'; if (strcmp(xattr->name, ".") == 0) { -- cgit v1.2.3-59-g8ed1b From b5c90a7526fe39164c2204f0404ce8f8ff21e522 Mon Sep 17 00:00:00 2001 From: Dan Carpenter Date: Fri, 1 Jun 2018 11:00:05 +0300 Subject: EVM: unlock on error path in evm_read_xattrs() We need to unlock before returning on this error path. Fixes: fa516b66a1bf ("EVM: Allow runtime modification of the set of verified xattrs") Signed-off-by: Dan Carpenter Signed-off-by: Mimi Zohar --- security/integrity/evm/evm_secfs.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'security/integrity/evm') diff --git a/security/integrity/evm/evm_secfs.c b/security/integrity/evm/evm_secfs.c index 3cefef3919e5..637eb999e340 100644 --- a/security/integrity/evm/evm_secfs.c +++ b/security/integrity/evm/evm_secfs.c @@ -147,8 +147,10 @@ static ssize_t evm_read_xattrs(struct file *filp, char __user *buf, size += strlen(xattr->name) + 1; temp = kmalloc(size + 1, GFP_KERNEL); - if (!temp) + if (!temp) { + mutex_unlock(&xattr_list_mutex); return -ENOMEM; + } list_for_each_entry(xattr, &evm_config_xattrnames, list) { sprintf(temp + offset, "%s\n", xattr->name); -- cgit v1.2.3-59-g8ed1b