* [PATCH v6 11/12] ima: Implement support for module-style appended signatures
From: Thiago Jung Bauermann @ 2018-03-16 20:38 UTC (permalink / raw)
To: linux-integrity
Cc: linux-security-module, keyrings, linux-crypto, linuxppc-dev,
linux-kernel, Mimi Zohar, Dmitry Kasatkin, James Morris,
Serge E. Hallyn, David Howells, David Woodhouse, Jessica Yu,
Herbert Xu, David S. Miller, AKASHI, Takahiro,
Thiago Jung Bauermann
In-Reply-To: <20180316203837.10174-1-bauerman@linux.vnet.ibm.com>
This patch actually implements the appraise_type=imasig|modsig option,
allowing IMA to read and verify modsig signatures.
In case both are present in the same file, IMA will first check whether the
key used by the xattr signature is present in the kernel keyring. If not,
it will try the appended signature.
Signed-off-by: Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com>
---
security/integrity/ima/ima.h | 11 +++++++-
security/integrity/ima/ima_appraise.c | 53 +++++++++++++++++++++++++++++++----
security/integrity/ima/ima_main.c | 21 +++++++++++---
3 files changed, 74 insertions(+), 11 deletions(-)
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index 49aef56dc96d..c11ccb7c5bfb 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -157,7 +157,8 @@ void ima_init_template_list(void);
static inline bool is_ima_sig(const struct evm_ima_xattr_data *xattr_value)
{
- return xattr_value && xattr_value->type == EVM_IMA_XATTR_DIGSIG;
+ return xattr_value && (xattr_value->type == EVM_IMA_XATTR_DIGSIG ||
+ xattr_value->type == IMA_MODSIG);
}
/*
@@ -253,6 +254,8 @@ enum integrity_status ima_get_cache_status(struct integrity_iint_cache *iint,
enum ima_hooks func);
enum hash_algo ima_get_hash_algo(struct evm_ima_xattr_data *xattr_value,
int xattr_len);
+bool ima_xattr_sig_known_key(const struct evm_ima_xattr_data *xattr_value,
+ int xattr_len);
int ima_read_xattr(struct dentry *dentry,
struct evm_ima_xattr_data **xattr_value);
@@ -291,6 +294,12 @@ ima_get_hash_algo(struct evm_ima_xattr_data *xattr_value, int xattr_len)
return ima_hash_algo;
}
+static inline bool ima_xattr_sig_known_key(const struct evm_ima_xattr_data
+ *xattr_value, int xattr_len)
+{
+ return false;
+}
+
static inline int ima_read_xattr(struct dentry *dentry,
struct evm_ima_xattr_data **xattr_value)
{
diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
index 01172eab297b..84e0fd5a19c8 100644
--- a/security/integrity/ima/ima_appraise.c
+++ b/security/integrity/ima/ima_appraise.c
@@ -189,6 +189,22 @@ enum hash_algo ima_get_hash_algo(struct evm_ima_xattr_data *xattr_value,
return ima_hash_algo;
}
+bool ima_xattr_sig_known_key(const struct evm_ima_xattr_data *xattr_value,
+ int xattr_len)
+{
+ struct key *keyring;
+
+ if (xattr_value->type != EVM_IMA_XATTR_DIGSIG)
+ return false;
+
+ keyring = integrity_keyring_from_id(INTEGRITY_KEYRING_IMA);
+ if (IS_ERR(keyring))
+ return false;
+
+ return asymmetric_sig_has_known_key(keyring, (const char *) xattr_value,
+ xattr_len);
+}
+
int ima_read_xattr(struct dentry *dentry,
struct evm_ima_xattr_data **xattr_value)
{
@@ -221,8 +237,12 @@ int ima_appraise_measurement(enum ima_hooks func,
struct inode *inode = d_backing_inode(dentry);
enum integrity_status status = INTEGRITY_UNKNOWN;
int rc = xattr_len, hash_start = 0;
+ size_t xattr_contents_len;
+ void *xattr_contents;
- if (!(inode->i_opflags & IOP_XATTR))
+ /* If not appraising a modsig, we need an xattr. */
+ if ((xattr_value == NULL || xattr_value->type != IMA_MODSIG) &&
+ !(inode->i_opflags & IOP_XATTR))
return INTEGRITY_UNKNOWN;
if (rc <= 0) {
@@ -241,13 +261,29 @@ int ima_appraise_measurement(enum ima_hooks func,
goto out;
}
- status = evm_verifyxattr(dentry, XATTR_NAME_IMA, xattr_value, rc, iint);
+ /*
+ * If it's a modsig, we don't have the xattr contents to pass to
+ * evm_verifyxattr().
+ */
+ if (xattr_value->type == IMA_MODSIG) {
+ xattr_contents = NULL;
+ xattr_contents_len = 0;
+ } else {
+ xattr_contents = xattr_value;
+ xattr_contents_len = xattr_len;
+ }
+
+ status = evm_verifyxattr(dentry, XATTR_NAME_IMA, xattr_contents,
+ xattr_contents_len, iint);
switch (status) {
case INTEGRITY_PASS:
case INTEGRITY_PASS_IMMUTABLE:
case INTEGRITY_UNKNOWN:
break;
case INTEGRITY_NOXATTRS: /* No EVM protected xattrs. */
+ /* It's fine not to have xattrs when using a modsig. */
+ if (xattr_value->type == IMA_MODSIG)
+ break;
case INTEGRITY_NOLABEL: /* No security.evm xattr. */
cause = "missing-HMAC";
goto out;
@@ -288,11 +324,16 @@ int ima_appraise_measurement(enum ima_hooks func,
status = INTEGRITY_PASS;
break;
case EVM_IMA_XATTR_DIGSIG:
+ case IMA_MODSIG:
set_bit(IMA_DIGSIG, &iint->atomic_flags);
- rc = integrity_digsig_verify(INTEGRITY_KEYRING_IMA,
- (const char *)xattr_value, rc,
- iint->ima_hash->digest,
- iint->ima_hash->length);
+ if (xattr_value->type == EVM_IMA_XATTR_DIGSIG)
+ rc = integrity_digsig_verify(INTEGRITY_KEYRING_IMA,
+ (const char *)xattr_value,
+ rc, iint->ima_hash->digest,
+ iint->ima_hash->length);
+ else
+ rc = ima_modsig_verify(INTEGRITY_KEYRING_IMA,
+ xattr_value);
if (rc == -EOPNOTSUPP) {
status = INTEGRITY_UNKNOWN;
} else if (rc) {
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index 5d122daf5c8a..1b11c10f09df 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -183,7 +183,7 @@ static int process_measurement(struct file *file, const struct cred *cred,
struct evm_ima_xattr_data *xattr_value = NULL;
int xattr_len = 0;
bool violation_check;
- enum hash_algo hash_algo;
+ enum hash_algo hash_algo = HASH_ALGO__LAST;
if (!ima_policy_flag || !S_ISREG(inode->i_mode))
return 0;
@@ -277,11 +277,24 @@ static int process_measurement(struct file *file, const struct cred *cred,
template_desc = ima_template_desc_current();
if ((action & IMA_APPRAISE_SUBMASK) ||
- strcmp(template_desc->name, IMA_TEMPLATE_IMA_NAME) != 0)
+ strcmp(template_desc->name, IMA_TEMPLATE_IMA_NAME) != 0) {
/* read 'security.ima' */
xattr_len = ima_read_xattr(file_dentry(file), &xattr_value);
+ if (iint->flags & IMA_MODSIG_ALLOWED &&
+ (xattr_len <= 0 || !ima_xattr_sig_known_key(xattr_value,
+ xattr_len))) {
+ /*
+ * Even if we end up using a modsig, hash_algo should
+ * come from the xattr (or even the default hash algo).
+ */
+ hash_algo = ima_get_hash_algo(xattr_value, xattr_len);
+ ima_read_modsig(func, buf, size, &xattr_value,
+ &xattr_len);
+ }
+ }
- hash_algo = ima_get_hash_algo(xattr_value, xattr_len);
+ if (hash_algo == HASH_ALGO__LAST)
+ hash_algo = ima_get_hash_algo(xattr_value, xattr_len);
rc = ima_collect_measurement(iint, file, buf, size, hash_algo);
if (rc != 0 && rc != -EBADF && rc != -EINVAL)
@@ -309,7 +322,7 @@ static int process_measurement(struct file *file, const struct cred *cred,
!(iint->flags & IMA_NEW_FILE))
rc = -EACCES;
mutex_unlock(&iint->mutex);
- kfree(xattr_value);
+ ima_free_xattr_data(xattr_value);
out:
if (pathbuf)
__putname(pathbuf);
^ permalink raw reply related
* [PATCH v6 10/12] ima: Add functions to read and verify a modsig signature
From: Thiago Jung Bauermann @ 2018-03-16 20:38 UTC (permalink / raw)
To: linux-integrity
Cc: linux-security-module, keyrings, linux-crypto, linuxppc-dev,
linux-kernel, Mimi Zohar, Dmitry Kasatkin, James Morris,
Serge E. Hallyn, David Howells, David Woodhouse, Jessica Yu,
Herbert Xu, David S. Miller, AKASHI, Takahiro,
Thiago Jung Bauermann
In-Reply-To: <20180316203837.10174-1-bauerman@linux.vnet.ibm.com>
This is the code needed by IMA-appraise to work with modsig signatures.
It will be used by the next two patches.
Signed-off-by: Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com>
---
security/integrity/ima/Kconfig | 3 +
security/integrity/ima/ima.h | 41 ++++++++
security/integrity/ima/ima_modsig.c | 181 ++++++++++++++++++++++++++++++++++++
security/integrity/integrity.h | 1 +
4 files changed, 226 insertions(+)
diff --git a/security/integrity/ima/Kconfig b/security/integrity/ima/Kconfig
index ee278189e0bb..306601d62f0b 100644
--- a/security/integrity/ima/Kconfig
+++ b/security/integrity/ima/Kconfig
@@ -167,6 +167,9 @@ config IMA_APPRAISE_BOOTPARAM
config IMA_APPRAISE_MODSIG
bool "Support module-style signatures for appraisal"
depends on IMA_APPRAISE
+ depends on INTEGRITY_ASYMMETRIC_KEYS
+ select PKCS7_MESSAGE_PARSER
+ select MODULE_SIG_FORMAT
default n
help
Adds support for signatures appended to files. The format of the
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index c61d8fc5190d..49aef56dc96d 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -301,11 +301,52 @@ static inline int ima_read_xattr(struct dentry *dentry,
#ifdef CONFIG_IMA_APPRAISE_MODSIG
bool ima_hook_supports_modsig(enum ima_hooks func);
+int ima_read_modsig(enum ima_hooks func, const void *buf, loff_t buf_len,
+ struct evm_ima_xattr_data **xattr_value,
+ int *xattr_len);
+int ima_get_modsig_hash(struct evm_ima_xattr_data *hdr, enum hash_algo *algo,
+ const u8 **hash, u8 *len);
+int ima_modsig_serialize_data(struct evm_ima_xattr_data **data, int *data_len);
+int ima_modsig_verify(const unsigned int keyring_id,
+ struct evm_ima_xattr_data *hdr);
+void ima_free_xattr_data(struct evm_ima_xattr_data *hdr);
#else
static inline bool ima_hook_supports_modsig(enum ima_hooks func)
{
return false;
}
+
+static inline int ima_read_modsig(enum ima_hooks func, const void *buf,
+ loff_t buf_len,
+ struct evm_ima_xattr_data **xattr_value,
+ int *xattr_len)
+{
+ return -ENOTSUPP;
+}
+
+static inline int ima_get_modsig_hash(struct evm_ima_xattr_data *hdr,
+ enum hash_algo *algo, const u8 **hash,
+ u8 *len)
+{
+ return -ENOTSUPP;
+}
+
+static inline int ima_modsig_serialize_data(struct evm_ima_xattr_data **data,
+ int *data_len)
+{
+ return -ENOTSUPP;
+}
+
+static inline int ima_modsig_verify(const unsigned int keyring_id,
+ struct evm_ima_xattr_data *hdr)
+{
+ return -ENOTSUPP;
+}
+
+static inline void ima_free_xattr_data(struct evm_ima_xattr_data *hdr)
+{
+ kfree(hdr);
+}
#endif /* CONFIG_IMA_APPRAISE_MODSIG */
/* LSM based policy rules require audit */
diff --git a/security/integrity/ima/ima_modsig.c b/security/integrity/ima/ima_modsig.c
index d8ea811b6f74..105fd04d585e 100644
--- a/security/integrity/ima/ima_modsig.c
+++ b/security/integrity/ima/ima_modsig.c
@@ -8,8 +8,25 @@
* Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com>
*/
+#include <linux/types.h>
+#include <linux/module_signature.h>
+#include <keys/asymmetric-type.h>
+#include <crypto/pkcs7.h>
+
#include "ima.h"
+struct modsig_hdr {
+ uint8_t type; /* Should be IMA_MODSIG. */
+ struct pkcs7_message *pkcs7_msg;
+ int raw_pkcs7_len;
+
+ /*
+ * This is what will go to the measurement list if the template requires
+ * storing the signature.
+ */
+ struct evm_ima_xattr_data raw_pkcs7;
+};
+
/**
* ima_hook_supports_modsig - can the policy allow modsig for this hook?
*
@@ -29,3 +46,167 @@ bool ima_hook_supports_modsig(enum ima_hooks func)
return false;
}
}
+
+static bool modsig_has_known_key(struct modsig_hdr *hdr)
+{
+ const struct public_key_signature *pks;
+ struct key *keyring;
+ struct key *key;
+
+ keyring = integrity_keyring_from_id(INTEGRITY_KEYRING_IMA);
+ if (IS_ERR(keyring))
+ return false;
+
+ pks = pkcs7_get_message_sig(hdr->pkcs7_msg);
+ if (!pks)
+ return false;
+
+ key = find_asymmetric_key(keyring, pks->auth_ids[0], NULL, false);
+ if (IS_ERR(key))
+ return false;
+
+ key_put(key);
+
+ return true;
+}
+
+int ima_read_modsig(enum ima_hooks func, const void *buf, loff_t buf_len,
+ struct evm_ima_xattr_data **xattr_value,
+ int *xattr_len)
+{
+ const size_t marker_len = sizeof(MODULE_SIG_STRING) - 1;
+ const struct module_signature *sig;
+ struct modsig_hdr *hdr;
+ size_t sig_len;
+ const void *p;
+ int rc;
+
+ /*
+ * Not supposed to happen. Hooks that support modsig are whitelisted
+ * when parsing the policy using ima_hooks_supports_modsig().
+ */
+ if (!buf || !buf_len) {
+ WARN_ONCE(true, "%s doesn't support modsig\n",
+ func_tokens[func]);
+ return -ENOENT;
+ } else if (buf_len <= marker_len + sizeof(*sig))
+ return -ENOENT;
+
+ p = buf + buf_len - marker_len;
+ if (memcmp(p, MODULE_SIG_STRING, marker_len))
+ return -ENOENT;
+
+ buf_len -= marker_len;
+ sig = (const struct module_signature *) (p - sizeof(*sig));
+
+ rc = validate_module_sig(sig, buf_len);
+ if (rc)
+ return rc;
+
+ sig_len = be32_to_cpu(sig->sig_len);
+ buf_len -= sig_len + sizeof(*sig);
+
+ /* Allocate sig_len additional bytes to hold the raw PKCS#7 data. */
+ hdr = kmalloc(sizeof(*hdr) + sig_len, GFP_KERNEL);
+ if (!hdr)
+ return -ENOMEM;
+
+ hdr->pkcs7_msg = pkcs7_parse_message(buf + buf_len, sig_len);
+ if (IS_ERR(hdr->pkcs7_msg)) {
+ rc = PTR_ERR(hdr->pkcs7_msg);
+ goto err_no_msg;
+ }
+
+ rc = pkcs7_supply_detached_data(hdr->pkcs7_msg, buf, buf_len);
+ if (rc)
+ goto err;
+
+ if (!modsig_has_known_key(hdr)) {
+ rc = -ENOKEY;
+ goto err;
+ }
+
+ memcpy(hdr->raw_pkcs7.data, buf + buf_len, sig_len);
+ hdr->raw_pkcs7_len = sig_len + 1;
+ hdr->raw_pkcs7.type = IMA_MODSIG;
+
+ hdr->type = IMA_MODSIG;
+
+ *xattr_value = (typeof(*xattr_value)) hdr;
+ *xattr_len = sizeof(*hdr);
+
+ return 0;
+
+ err:
+ pkcs7_free_message(hdr->pkcs7_msg);
+ err_no_msg:
+ kfree(hdr);
+ return rc;
+}
+
+int ima_get_modsig_hash(struct evm_ima_xattr_data *hdr, enum hash_algo *algo,
+ const u8 **hash, u8 *len)
+{
+ struct modsig_hdr *modsig = (typeof(modsig)) hdr;
+ const struct public_key_signature *pks;
+ int i;
+
+ if (!hdr || hdr->type != IMA_MODSIG)
+ return -EINVAL;
+
+ pks = pkcs7_get_message_sig(modsig->pkcs7_msg);
+ if (!pks)
+ return -EBADMSG;
+
+ for (i = 0; i < HASH_ALGO__LAST; i++)
+ if (!strcmp(hash_algo_name[i], pks->hash_algo))
+ break;
+
+ *algo = i;
+
+ return pkcs7_get_digest(modsig->pkcs7_msg, hash, len);
+}
+
+int ima_modsig_serialize_data(struct evm_ima_xattr_data **data, int *data_len)
+{
+ struct modsig_hdr *modsig = (struct modsig_hdr *) *data;
+
+ if (!*data || (*data)->type != IMA_MODSIG)
+ return -EINVAL;
+
+ *data = &modsig->raw_pkcs7;
+ *data_len = modsig->raw_pkcs7_len;
+
+ return 0;
+}
+
+int ima_modsig_verify(const unsigned int keyring_id,
+ struct evm_ima_xattr_data *hdr)
+{
+ struct modsig_hdr *modsig = (struct modsig_hdr *) hdr;
+ struct key *keyring;
+
+ if (!hdr || hdr->type != IMA_MODSIG)
+ return -EINVAL;
+
+ keyring = integrity_keyring_from_id(keyring_id);
+ if (IS_ERR(keyring))
+ return PTR_ERR(keyring);
+
+ return verify_pkcs7_message_sig(NULL, 0, modsig->pkcs7_msg, keyring,
+ VERIFYING_MODULE_SIGNATURE, NULL, NULL);
+}
+
+void ima_free_xattr_data(struct evm_ima_xattr_data *hdr)
+{
+ if (!hdr)
+ return;
+
+ if (hdr->type == IMA_MODSIG) {
+ struct modsig_hdr *modsig = (struct modsig_hdr *) hdr;
+
+ pkcs7_free_message(modsig->pkcs7_msg);
+ }
+
+ kfree(hdr);
+}
diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h
index 6643c6550787..4acb1fb86b3b 100644
--- a/security/integrity/integrity.h
+++ b/security/integrity/integrity.h
@@ -74,6 +74,7 @@ enum evm_ima_xattr_type {
EVM_IMA_XATTR_DIGSIG,
IMA_XATTR_DIGEST_NG,
EVM_XATTR_PORTABLE_DIGSIG,
+ IMA_MODSIG,
IMA_XATTR_LAST
};
^ permalink raw reply related
* [PATCH v6 09/12] ima: Add modsig appraise_type option for module-style appended signatures
From: Thiago Jung Bauermann @ 2018-03-16 20:38 UTC (permalink / raw)
To: linux-integrity
Cc: linux-security-module, keyrings, linux-crypto, linuxppc-dev,
linux-kernel, Mimi Zohar, Dmitry Kasatkin, James Morris,
Serge E. Hallyn, David Howells, David Woodhouse, Jessica Yu,
Herbert Xu, David S. Miller, AKASHI, Takahiro,
Thiago Jung Bauermann
In-Reply-To: <20180316203837.10174-1-bauerman@linux.vnet.ibm.com>
This patch introduces the modsig keyword to the IMA policy syntax to
specify that a given hook should expect the file to have the IMA signature
appended to it. Here is how it can be used in a rule:
appraise func=KEXEC_KERNEL_CHECK appraise_type=imasig|modsig
With this rule, IMA will accept either a signature stored in the extended
attribute or an appended signature.
For now, the rule above will behave exactly the same as if
appraise_type=imasig was specified because the actual modsig implementation
will be introduced in a separate patch.
Suggested-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
Signed-off-by: Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com>
---
Documentation/ABI/testing/ima_policy | 6 +++++-
security/integrity/ima/Kconfig | 10 ++++++++++
security/integrity/ima/Makefile | 1 +
security/integrity/ima/ima.h | 9 +++++++++
security/integrity/ima/ima_modsig.c | 31 +++++++++++++++++++++++++++++++
security/integrity/ima/ima_policy.c | 12 ++++++++++--
security/integrity/integrity.h | 1 +
7 files changed, 67 insertions(+), 3 deletions(-)
diff --git a/Documentation/ABI/testing/ima_policy b/Documentation/ABI/testing/ima_policy
index b8465e00ba5f..835d9da9b26e 100644
--- a/Documentation/ABI/testing/ima_policy
+++ b/Documentation/ABI/testing/ima_policy
@@ -37,7 +37,7 @@ Description:
euid:= decimal value
fowner:= decimal value
lsm: are LSM specific
- option: appraise_type:= [imasig]
+ option: appraise_type:= [imasig] [imasig|modsig]
pcr:= decimal value
default policy:
@@ -103,3 +103,7 @@ Description:
measure func=KEXEC_KERNEL_CHECK pcr=4
measure func=KEXEC_INITRAMFS_CHECK pcr=5
+
+ Example of appraise rule allowing modsig appended signatures:
+
+ appraise func=KEXEC_KERNEL_CHECK appraise_type=imasig|modsig
diff --git a/security/integrity/ima/Kconfig b/security/integrity/ima/Kconfig
index 6a8f67714c83..ee278189e0bb 100644
--- a/security/integrity/ima/Kconfig
+++ b/security/integrity/ima/Kconfig
@@ -164,6 +164,16 @@ config IMA_APPRAISE_BOOTPARAM
This option enables the different "ima_appraise=" modes
(eg. fix, log) from the boot command line.
+config IMA_APPRAISE_MODSIG
+ bool "Support module-style signatures for appraisal"
+ depends on IMA_APPRAISE
+ default n
+ help
+ Adds support for signatures appended to files. The format of the
+ appended signature is the same used for signed kernel modules.
+ The modsig keyword can be used in the IMA policy to allow a hook
+ to accept such signatures.
+
config IMA_TRUSTED_KEYRING
bool "Require all keys on the .ima keyring be signed (deprecated)"
depends on IMA_APPRAISE && SYSTEM_TRUSTED_KEYRING
diff --git a/security/integrity/ima/Makefile b/security/integrity/ima/Makefile
index d921dc4f9eb0..31d57cdf2421 100644
--- a/security/integrity/ima/Makefile
+++ b/security/integrity/ima/Makefile
@@ -9,5 +9,6 @@ obj-$(CONFIG_IMA) += ima.o
ima-y := ima_fs.o ima_queue.o ima_init.o ima_main.o ima_crypto.o ima_api.o \
ima_policy.o ima_template.o ima_template_lib.o
ima-$(CONFIG_IMA_APPRAISE) += ima_appraise.o
+ima-$(CONFIG_IMA_APPRAISE_MODSIG) += ima_modsig.o
ima-$(CONFIG_HAVE_IMA_KEXEC) += ima_kexec.o
obj-$(CONFIG_IMA_BLACKLIST_KEYRING) += ima_mok.o
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index 99f2bab15009..c61d8fc5190d 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -299,6 +299,15 @@ static inline int ima_read_xattr(struct dentry *dentry,
#endif /* CONFIG_IMA_APPRAISE */
+#ifdef CONFIG_IMA_APPRAISE_MODSIG
+bool ima_hook_supports_modsig(enum ima_hooks func);
+#else
+static inline bool ima_hook_supports_modsig(enum ima_hooks func)
+{
+ return false;
+}
+#endif /* CONFIG_IMA_APPRAISE_MODSIG */
+
/* LSM based policy rules require audit */
#ifdef CONFIG_IMA_LSM_RULES
diff --git a/security/integrity/ima/ima_modsig.c b/security/integrity/ima/ima_modsig.c
new file mode 100644
index 000000000000..d8ea811b6f74
--- /dev/null
+++ b/security/integrity/ima/ima_modsig.c
@@ -0,0 +1,31 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * IMA support for appraising module-style appended signatures.
+ *
+ * Copyright (C) 2018 IBM Corporation
+ *
+ * Author:
+ * Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com>
+ */
+
+#include "ima.h"
+
+/**
+ * ima_hook_supports_modsig - can the policy allow modsig for this hook?
+ *
+ * modsig is only supported by hooks using ima_post_read_file, because only they
+ * preload the contents of the file in a buffer. FILE_CHECK does that in some
+ * cases, but not when reached from vfs_open. POLICY_CHECK can support it, but
+ * it's not useful in practice because it's a text file so deny.
+ */
+bool ima_hook_supports_modsig(enum ima_hooks func)
+{
+ switch (func) {
+ case FIRMWARE_CHECK:
+ case KEXEC_KERNEL_CHECK:
+ case KEXEC_INITRAMFS_CHECK:
+ return true;
+ default:
+ return false;
+ }
+}
diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index 833da24c7009..e7421fd94277 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -887,6 +887,10 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
ima_log_string(ab, "appraise_type", args[0].from);
if ((strcmp(args[0].from, "imasig")) == 0)
entry->flags |= IMA_DIGSIG_REQUIRED;
+ else if (ima_hook_supports_modsig(entry->func) &&
+ strcmp(args[0].from, "imasig|modsig") == 0)
+ entry->flags |= IMA_DIGSIG_REQUIRED
+ | IMA_MODSIG_ALLOWED;
else
result = -EINVAL;
break;
@@ -1176,8 +1180,12 @@ int ima_policy_show(struct seq_file *m, void *v)
}
}
}
- if (entry->flags & IMA_DIGSIG_REQUIRED)
- seq_puts(m, "appraise_type=imasig ");
+ if (entry->flags & IMA_DIGSIG_REQUIRED) {
+ if (entry->flags & IMA_MODSIG_ALLOWED)
+ seq_puts(m, "appraise_type=imasig|modsig ");
+ else
+ seq_puts(m, "appraise_type=imasig ");
+ }
if (entry->flags & IMA_PERMIT_DIRECTIO)
seq_puts(m, "permit_directio ");
rcu_read_unlock();
diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h
index 4c381b992e11..6643c6550787 100644
--- a/security/integrity/integrity.h
+++ b/security/integrity/integrity.h
@@ -35,6 +35,7 @@
#define IMA_NEW_FILE 0x04000000
#define EVM_IMMUTABLE_DIGSIG 0x08000000
#define IMA_FAIL_UNVERIFIABLE_SIGS 0x10000000
+#define IMA_MODSIG_ALLOWED 0x20000000
#define IMA_DO_MASK (IMA_MEASURE | IMA_APPRAISE | IMA_AUDIT | \
IMA_HASH | IMA_APPRAISE_SUBMASK)
^ permalink raw reply related
* [PATCH v6 08/12] ima: Export func_tokens
From: Thiago Jung Bauermann @ 2018-03-16 20:38 UTC (permalink / raw)
To: linux-integrity
Cc: linux-security-module, keyrings, linux-crypto, linuxppc-dev,
linux-kernel, Mimi Zohar, Dmitry Kasatkin, James Morris,
Serge E. Hallyn, David Howells, David Woodhouse, Jessica Yu,
Herbert Xu, David S. Miller, AKASHI, Takahiro,
Thiago Jung Bauermann
In-Reply-To: <20180316203837.10174-1-bauerman@linux.vnet.ibm.com>
ima_read_modsig() will need it so that it can show an error message.
Signed-off-by: Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com>
---
security/integrity/ima/ima.h | 2 ++
security/integrity/ima/ima_policy.c | 12 ++++++------
2 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index 4bafa6a97967..99f2bab15009 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -196,6 +196,8 @@ enum ima_hooks {
__ima_hooks(__ima_hook_enumify)
};
+extern const char *const func_tokens[];
+
/* LIM API function definitions */
int ima_get_action(struct inode *inode, const struct cred *cred, u32 secid,
int mask, enum ima_hooks func, int *pcr);
diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index 51a4cd999a49..833da24c7009 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -992,6 +992,12 @@ void ima_delete_rules(void)
}
}
+#define __ima_hook_stringify(str) (#str),
+
+const char *const func_tokens[] = {
+ __ima_hooks(__ima_hook_stringify)
+};
+
#ifdef CONFIG_IMA_READ_POLICY
enum {
mask_exec = 0, mask_write, mask_read, mask_append
@@ -1004,12 +1010,6 @@ static const char *const mask_tokens[] = {
"MAY_APPEND"
};
-#define __ima_hook_stringify(str) (#str),
-
-static const char *const func_tokens[] = {
- __ima_hooks(__ima_hook_stringify)
-};
-
void *ima_policy_start(struct seq_file *m, loff_t *pos)
{
loff_t l = *pos;
^ permalink raw reply related
* [PATCH v6 07/12] integrity: Select CONFIG_KEYS instead of depending on it
From: Thiago Jung Bauermann @ 2018-03-16 20:38 UTC (permalink / raw)
To: linux-integrity
Cc: linux-security-module, keyrings, linux-crypto, linuxppc-dev,
linux-kernel, Mimi Zohar, Dmitry Kasatkin, James Morris,
Serge E. Hallyn, David Howells, David Woodhouse, Jessica Yu,
Herbert Xu, David S. Miller, AKASHI, Takahiro,
Thiago Jung Bauermann
In-Reply-To: <20180316203837.10174-1-bauerman@linux.vnet.ibm.com>
This avoids a dependency cycle in CONFIG_IMA_APPRAISE_MODSIG (introduced by
a later patch in this series): it will select CONFIG_MODULE_SIG_FORMAT
which in turn selects CONFIG_KEYS. Kconfig then complains that
CONFIG_INTEGRITY_SIGNATURE depends on CONFIG_KEYS.
Signed-off-by: Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com>
---
security/integrity/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/security/integrity/Kconfig b/security/integrity/Kconfig
index da9565891738..0d642e0317c7 100644
--- a/security/integrity/Kconfig
+++ b/security/integrity/Kconfig
@@ -17,8 +17,8 @@ if INTEGRITY
config INTEGRITY_SIGNATURE
bool "Digital signature verification using multiple keyrings"
- depends on KEYS
default n
+ select KEYS
select SIGNATURE
help
This option enables digital signature verification support
^ permalink raw reply related
* [PATCH v6 06/12] integrity: Introduce asymmetric_sig_has_known_key()
From: Thiago Jung Bauermann @ 2018-03-16 20:38 UTC (permalink / raw)
To: linux-integrity
Cc: linux-security-module, keyrings, linux-crypto, linuxppc-dev,
linux-kernel, Mimi Zohar, Dmitry Kasatkin, James Morris,
Serge E. Hallyn, David Howells, David Woodhouse, Jessica Yu,
Herbert Xu, David S. Miller, AKASHI, Takahiro,
Thiago Jung Bauermann
In-Reply-To: <20180316203837.10174-1-bauerman@linux.vnet.ibm.com>
IMA will only look for a modsig if the xattr sig references a key which is
not in the expected kernel keyring. To that end, introduce
asymmetric_sig_has_known_key().
The logic of extracting the key used in the xattr sig is factored out from
asymmetric_verify() so that it can be used by the new function.
Signed-off-by: Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com>
---
security/integrity/digsig_asymmetric.c | 44 +++++++++++++++++++++++++---------
security/integrity/integrity.h | 8 +++++++
2 files changed, 41 insertions(+), 11 deletions(-)
diff --git a/security/integrity/digsig_asymmetric.c b/security/integrity/digsig_asymmetric.c
index ab6a029062a1..241647970c19 100644
--- a/security/integrity/digsig_asymmetric.c
+++ b/security/integrity/digsig_asymmetric.c
@@ -79,26 +79,48 @@ static struct key *request_asymmetric_key(struct key *keyring, uint32_t keyid)
return key;
}
-int asymmetric_verify(struct key *keyring, const char *sig,
- int siglen, const char *data, int datalen)
+static struct key *asymmetric_key_from_sig(struct key *keyring, const char *sig,
+ int siglen)
{
- struct public_key_signature pks;
- struct signature_v2_hdr *hdr = (struct signature_v2_hdr *)sig;
- struct key *key;
- int ret = -ENOMEM;
+ const struct signature_v2_hdr *hdr = (struct signature_v2_hdr *) sig;
if (siglen <= sizeof(*hdr))
- return -EBADMSG;
+ return ERR_PTR(-EBADMSG);
siglen -= sizeof(*hdr);
if (siglen != be16_to_cpu(hdr->sig_size))
- return -EBADMSG;
+ return ERR_PTR(-EBADMSG);
if (hdr->hash_algo >= HASH_ALGO__LAST)
- return -ENOPKG;
+ return ERR_PTR(-ENOPKG);
+
+ return request_asymmetric_key(keyring, be32_to_cpu(hdr->keyid));
+}
+
+bool asymmetric_sig_has_known_key(struct key *keyring, const char *sig,
+ int siglen)
+{
+ struct key *key;
+
+ key = asymmetric_key_from_sig(keyring, sig, siglen);
+ if (IS_ERR_OR_NULL(key))
+ return false;
+
+ key_put(key);
+
+ return true;
+}
+
+int asymmetric_verify(struct key *keyring, const char *sig,
+ int siglen, const char *data, int datalen)
+{
+ struct public_key_signature pks;
+ struct signature_v2_hdr *hdr = (struct signature_v2_hdr *)sig;
+ struct key *key;
+ int ret = -ENOMEM;
- key = request_asymmetric_key(keyring, be32_to_cpu(hdr->keyid));
+ key = asymmetric_key_from_sig(keyring, sig, siglen);
if (IS_ERR(key))
return PTR_ERR(key);
@@ -109,7 +131,7 @@ int asymmetric_verify(struct key *keyring, const char *sig,
pks.digest = (u8 *)data;
pks.digest_size = datalen;
pks.s = hdr->sig;
- pks.s_size = siglen;
+ pks.s_size = siglen - sizeof(*hdr);
ret = verify_signature(key, &pks);
key_put(key);
pr_debug("%s() = %d\n", __func__, ret);
diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h
index 2d245f44ca26..4c381b992e11 100644
--- a/security/integrity/integrity.h
+++ b/security/integrity/integrity.h
@@ -179,12 +179,20 @@ static inline int integrity_init_keyring(const unsigned int id)
#ifdef CONFIG_INTEGRITY_ASYMMETRIC_KEYS
int asymmetric_verify(struct key *keyring, const char *sig,
int siglen, const char *data, int datalen);
+bool asymmetric_sig_has_known_key(struct key *keyring, const char *sig,
+ int siglen);
#else
static inline int asymmetric_verify(struct key *keyring, const char *sig,
int siglen, const char *data, int datalen)
{
return -EOPNOTSUPP;
}
+
+static inline bool asymmetric_sig_has_known_key(struct key *keyring,
+ const char *sig, int siglen)
+{
+ return false;
+}
#endif
#ifdef CONFIG_IMA_LOAD_X509
^ permalink raw reply related
* [PATCH v6 05/12] integrity: Introduce integrity_keyring_from_id()
From: Thiago Jung Bauermann @ 2018-03-16 20:38 UTC (permalink / raw)
To: linux-integrity
Cc: linux-security-module, keyrings, linux-crypto, linuxppc-dev,
linux-kernel, Mimi Zohar, Dmitry Kasatkin, James Morris,
Serge E. Hallyn, David Howells, David Woodhouse, Jessica Yu,
Herbert Xu, David S. Miller, AKASHI, Takahiro,
Thiago Jung Bauermann
In-Reply-To: <20180316203837.10174-1-bauerman@linux.vnet.ibm.com>
IMA will need to obtain the keyring used to verify file signatures so that
it can verify the module-style signature appended to files.
Signed-off-by: Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com>
---
security/integrity/digsig.c | 28 +++++++++++++++++++++-------
security/integrity/integrity.h | 6 ++++++
2 files changed, 27 insertions(+), 7 deletions(-)
diff --git a/security/integrity/digsig.c b/security/integrity/digsig.c
index 6f9e4ce568cd..e641a67b9fc7 100644
--- a/security/integrity/digsig.c
+++ b/security/integrity/digsig.c
@@ -48,11 +48,10 @@ static bool init_keyring __initdata;
#define restrict_link_to_ima restrict_link_by_builtin_trusted
#endif
-int integrity_digsig_verify(const unsigned int id, const char *sig, int siglen,
- const char *digest, int digestlen)
+struct key *integrity_keyring_from_id(const unsigned int id)
{
- if (id >= INTEGRITY_KEYRING_MAX || siglen < 2)
- return -EINVAL;
+ if (id >= INTEGRITY_KEYRING_MAX)
+ return ERR_PTR(-EINVAL);
if (!keyring[id]) {
keyring[id] =
@@ -61,17 +60,32 @@ int integrity_digsig_verify(const unsigned int id, const char *sig, int siglen,
int err = PTR_ERR(keyring[id]);
pr_err("no %s keyring: %d\n", keyring_name[id], err);
keyring[id] = NULL;
- return err;
+ return ERR_PTR(err);
}
}
+ return keyring[id];
+}
+
+int integrity_digsig_verify(const unsigned int id, const char *sig, int siglen,
+ const char *digest, int digestlen)
+{
+ struct key *keyring;
+
+ if (siglen < 2)
+ return -EINVAL;
+
+ keyring = integrity_keyring_from_id(id);
+ if (IS_ERR(keyring))
+ return PTR_ERR(keyring);
+
switch (sig[1]) {
case 1:
/* v1 API expect signature without xattr type */
- return digsig_verify(keyring[id], sig + 1, siglen - 1,
+ return digsig_verify(keyring, sig + 1, siglen - 1,
digest, digestlen);
case 2:
- return asymmetric_verify(keyring[id], sig, siglen,
+ return asymmetric_verify(keyring, sig, siglen,
digest, digestlen);
}
diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h
index 79799a0d9195..2d245f44ca26 100644
--- a/security/integrity/integrity.h
+++ b/security/integrity/integrity.h
@@ -150,6 +150,7 @@ int integrity_kernel_read(struct file *file, loff_t offset,
#ifdef CONFIG_INTEGRITY_SIGNATURE
+struct key *integrity_keyring_from_id(const unsigned int id);
int integrity_digsig_verify(const unsigned int id, const char *sig, int siglen,
const char *digest, int digestlen);
@@ -157,6 +158,11 @@ int __init integrity_init_keyring(const unsigned int id);
int __init integrity_load_x509(const unsigned int id, const char *path);
#else
+static inline struct key *integrity_keyring_from_id(const unsigned int id)
+{
+ return ERR_PTR(-EINVAL);
+}
+
static inline int integrity_digsig_verify(const unsigned int id,
const char *sig, int siglen,
const char *digest, int digestlen)
^ permalink raw reply related
* [PATCH v6 04/12] ima: Introduce is_ima_sig()
From: Thiago Jung Bauermann @ 2018-03-16 20:38 UTC (permalink / raw)
To: linux-integrity
Cc: linux-security-module, keyrings, linux-crypto, linuxppc-dev,
linux-kernel, Mimi Zohar, Dmitry Kasatkin, James Morris,
Serge E. Hallyn, David Howells, David Woodhouse, Jessica Yu,
Herbert Xu, David S. Miller, AKASHI, Takahiro,
Thiago Jung Bauermann
In-Reply-To: <20180316203837.10174-1-bauerman@linux.vnet.ibm.com>
With the introduction of another IMA signature type (modsig), some places
will need to check for both of them. It is cleaner to do that if there's a
helper function to tell whether an xattr_value represents an IMA
signature.
Suggested-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
Signed-off-by: Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com>
---
security/integrity/ima/ima.h | 5 +++++
security/integrity/ima/ima_appraise.c | 7 +++----
security/integrity/ima/ima_template_lib.c | 2 +-
3 files changed, 9 insertions(+), 5 deletions(-)
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index 35fe91aa1fc9..4bafa6a97967 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -155,6 +155,11 @@ unsigned long ima_get_binary_runtime_size(void);
int ima_init_template(void);
void ima_init_template_list(void);
+static inline bool is_ima_sig(const struct evm_ima_xattr_data *xattr_value)
+{
+ return xattr_value && xattr_value->type == EVM_IMA_XATTR_DIGSIG;
+}
+
/*
* used to protect h_table and sha_table
*/
diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
index a6b2995b7d0b..01172eab297b 100644
--- a/security/integrity/ima/ima_appraise.c
+++ b/security/integrity/ima/ima_appraise.c
@@ -325,15 +325,14 @@ int ima_appraise_measurement(enum ima_hooks func,
} else if (status != INTEGRITY_PASS) {
/* Fix mode, but don't replace file signatures. */
if ((ima_appraise & IMA_APPRAISE_FIX) &&
- (!xattr_value ||
- xattr_value->type != EVM_IMA_XATTR_DIGSIG)) {
+ !is_ima_sig(xattr_value)) {
if (!ima_fix_xattr(dentry, iint))
status = INTEGRITY_PASS;
}
/* Permit new files with file signatures, but without data. */
if (inode->i_size == 0 && iint->flags & IMA_NEW_FILE &&
- xattr_value && xattr_value->type == EVM_IMA_XATTR_DIGSIG) {
+ is_ima_sig(xattr_value)) {
status = INTEGRITY_PASS;
}
@@ -448,7 +447,7 @@ int ima_inode_setxattr(struct dentry *dentry, const char *xattr_name,
if (!xattr_value_len || (xvalue->type >= IMA_XATTR_LAST))
return -EINVAL;
ima_reset_appraise_flags(d_backing_inode(dentry),
- xvalue->type == EVM_IMA_XATTR_DIGSIG);
+ is_ima_sig(xvalue));
result = 0;
}
return result;
diff --git a/security/integrity/ima/ima_template_lib.c b/security/integrity/ima/ima_template_lib.c
index 5afaa53decc5..afb52a90e532 100644
--- a/security/integrity/ima/ima_template_lib.c
+++ b/security/integrity/ima/ima_template_lib.c
@@ -380,7 +380,7 @@ int ima_eventsig_init(struct ima_event_data *event_data,
{
struct evm_ima_xattr_data *xattr_value = event_data->xattr_value;
- if ((!xattr_value) || (xattr_value->type != EVM_IMA_XATTR_DIGSIG))
+ if (!is_ima_sig(xattr_value))
return 0;
return ima_write_template_field_data(xattr_value, event_data->xattr_len,
^ permalink raw reply related
* [PATCH v6 03/12] PKCS#7: Introduce pkcs7_get_digest()
From: Thiago Jung Bauermann @ 2018-03-16 20:38 UTC (permalink / raw)
To: linux-integrity
Cc: linux-security-module, keyrings, linux-crypto, linuxppc-dev,
linux-kernel, Mimi Zohar, Dmitry Kasatkin, James Morris,
Serge E. Hallyn, David Howells, David Woodhouse, Jessica Yu,
Herbert Xu, David S. Miller, AKASHI, Takahiro,
Thiago Jung Bauermann
In-Reply-To: <20180316203837.10174-1-bauerman@linux.vnet.ibm.com>
IMA will need to access the digest of the PKCS7 message (as calculated by
the kernel) before the signature is verified, so introduce
pkcs7_get_digest() for that purpose.
Also, modify pkcs7_digest() to detect when the digest was already
calculated so that it doesn't have to do redundant work. Verifying that
sinfo->sig->digest isn't NULL is sufficient because both places which
allocate sinfo->sig (pkcs7_parse_message() and pkcs7_note_signed_info())
use kzalloc() so sig->digest is always initialized to zero.
Signed-off-by: Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com>
Cc: David Howells <dhowells@redhat.com>
Cc: Herbert Xu <herbert@gondor.apana.org.au>
Cc: "David S. Miller" <davem@davemloft.net>
---
crypto/asymmetric_keys/pkcs7_verify.c | 25 +++++++++++++++++++++++++
include/crypto/pkcs7.h | 3 +++
2 files changed, 28 insertions(+)
diff --git a/crypto/asymmetric_keys/pkcs7_verify.c b/crypto/asymmetric_keys/pkcs7_verify.c
index 39e6de0c2761..bd02360f8be5 100644
--- a/crypto/asymmetric_keys/pkcs7_verify.c
+++ b/crypto/asymmetric_keys/pkcs7_verify.c
@@ -33,6 +33,10 @@ static int pkcs7_digest(struct pkcs7_message *pkcs7,
kenter(",%u,%s", sinfo->index, sinfo->sig->hash_algo);
+ /* The digest was calculated already. */
+ if (sig->digest)
+ return 0;
+
if (!sinfo->sig->hash_algo)
return -ENOPKG;
@@ -122,6 +126,27 @@ static int pkcs7_digest(struct pkcs7_message *pkcs7,
return ret;
}
+int pkcs7_get_digest(struct pkcs7_message *pkcs7, const u8 **buf, u8 *len)
+{
+ struct pkcs7_signed_info *sinfo = pkcs7->signed_infos;
+ int ret;
+
+ /*
+ * This function doesn't support messages with more than one signature.
+ */
+ if (sinfo == NULL || sinfo->next != NULL)
+ return -EBADMSG;
+
+ ret = pkcs7_digest(pkcs7, sinfo);
+ if (ret)
+ return ret;
+
+ *buf = sinfo->sig->digest;
+ *len = sinfo->sig->digest_size;
+
+ return 0;
+}
+
/*
* Find the key (X.509 certificate) to use to verify a PKCS#7 message. PKCS#7
* uses the issuer's name and the issuing certificate serial number for
diff --git a/include/crypto/pkcs7.h b/include/crypto/pkcs7.h
index 6f51d0cb6d12..cfaea9c37f4a 100644
--- a/include/crypto/pkcs7.h
+++ b/include/crypto/pkcs7.h
@@ -46,4 +46,7 @@ extern int pkcs7_verify(struct pkcs7_message *pkcs7,
extern int pkcs7_supply_detached_data(struct pkcs7_message *pkcs7,
const void *data, size_t datalen);
+extern int pkcs7_get_digest(struct pkcs7_message *pkcs7, const u8 **buf,
+ u8 *len);
+
#endif /* _CRYPTO_PKCS7_H */
^ permalink raw reply related
* [PATCH v6 01/12] MODSIGN: Export module signature definitions
From: Thiago Jung Bauermann @ 2018-03-16 20:38 UTC (permalink / raw)
To: linux-integrity
Cc: linux-security-module, keyrings, linux-crypto, linuxppc-dev,
linux-kernel, Mimi Zohar, Dmitry Kasatkin, James Morris,
Serge E. Hallyn, David Howells, David Woodhouse, Jessica Yu,
Herbert Xu, David S. Miller, AKASHI, Takahiro,
Thiago Jung Bauermann
In-Reply-To: <20180316203837.10174-1-bauerman@linux.vnet.ibm.com>
IMA will use the module_signature format for append signatures, so export
the relevant definitions and factor out the code which verifies that the
appended signature trailer is valid.
Also, create a CONFIG_MODULE_SIG_FORMAT option so that IMA can select it
and be able to use validate_module_sig() without having to depend on
CONFIG_MODULE_SIG.
Signed-off-by: Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com>
Reviewed-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
Cc: Jessica Yu <jeyu@kernel.org>
---
include/linux/module.h | 3 --
include/linux/module_signature.h | 44 +++++++++++++++++++++++
init/Kconfig | 6 +++-
kernel/Makefile | 2 +-
kernel/module.c | 1 +
kernel/module_signing.c | 77 ++++++++++++++++++----------------------
6 files changed, 85 insertions(+), 48 deletions(-)
diff --git a/include/linux/module.h b/include/linux/module.h
index d44df9b2c131..275cbc2579eb 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -24,9 +24,6 @@
#include <linux/percpu.h>
#include <asm/module.h>
-/* In stripped ARM and x86-64 modules, ~ is surprisingly rare. */
-#define MODULE_SIG_STRING "~Module signature appended~\n"
-
/* Not Yet Implemented */
#define MODULE_SUPPORTED_DEVICE(name)
diff --git a/include/linux/module_signature.h b/include/linux/module_signature.h
new file mode 100644
index 000000000000..890135437b6b
--- /dev/null
+++ b/include/linux/module_signature.h
@@ -0,0 +1,44 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * Module signature handling.
+ *
+ * Copyright (C) 2012 Red Hat, Inc. All Rights Reserved.
+ * Written by David Howells (dhowells@redhat.com)
+ */
+
+#ifndef _LINUX_MODULE_SIGNATURE_H
+#define _LINUX_MODULE_SIGNATURE_H
+
+/* In stripped ARM and x86-64 modules, ~ is surprisingly rare. */
+#define MODULE_SIG_STRING "~Module signature appended~\n"
+
+enum pkey_id_type {
+ PKEY_ID_PGP, /* OpenPGP generated key ID */
+ PKEY_ID_X509, /* X.509 arbitrary subjectKeyIdentifier */
+ PKEY_ID_PKCS7, /* Signature in PKCS#7 message */
+};
+
+/*
+ * Module signature information block.
+ *
+ * The constituents of the signature section are, in order:
+ *
+ * - Signer's name
+ * - Key identifier
+ * - Signature data
+ * - Information block
+ */
+struct module_signature {
+ u8 algo; /* Public-key crypto algorithm [0] */
+ u8 hash; /* Digest algorithm [0] */
+ u8 id_type; /* Key identifier type [PKEY_ID_PKCS7] */
+ u8 signer_len; /* Length of signer's name [0] */
+ u8 key_id_len; /* Length of key identifier [0] */
+ u8 __pad[3];
+ __be32 sig_len; /* Length of signature data */
+};
+
+int validate_module_sig(const struct module_signature *ms, size_t file_len);
+int mod_verify_sig(const void *mod, unsigned long *_modlen);
+
+#endif /* _LINUX_MODULE_SIGNATURE_H */
diff --git a/init/Kconfig b/init/Kconfig
index e37f4b2a6445..56a9679671b5 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -1760,7 +1760,7 @@ config MODULE_SRCVERSION_ALL
config MODULE_SIG
bool "Module signature verification"
depends on MODULES
- select SYSTEM_DATA_VERIFICATION
+ select MODULE_SIG_FORMAT
help
Check modules for valid signatures upon load: the signature
is simply appended to the module. For more information see
@@ -1775,6 +1775,10 @@ config MODULE_SIG
debuginfo strip done by some packagers (such as rpmbuild) and
inclusion into an initramfs that wants the module size reduced.
+config MODULE_SIG_FORMAT
+ def_bool n
+ select SYSTEM_DATA_VERIFICATION
+
config MODULE_SIG_FORCE
bool "Require modules to be validly signed"
depends on MODULE_SIG
diff --git a/kernel/Makefile b/kernel/Makefile
index f85ae5dfa474..a4c7a7a0cce7 100644
--- a/kernel/Makefile
+++ b/kernel/Makefile
@@ -58,7 +58,7 @@ obj-y += up.o
endif
obj-$(CONFIG_UID16) += uid16.o
obj-$(CONFIG_MODULES) += module.o
-obj-$(CONFIG_MODULE_SIG) += module_signing.o
+obj-$(CONFIG_MODULE_SIG_FORMAT) += module_signing.o
obj-$(CONFIG_KALLSYMS) += kallsyms.o
obj-$(CONFIG_BSD_PROCESS_ACCT) += acct.o
obj-$(CONFIG_CRASH_CORE) += crash_core.o
diff --git a/kernel/module.c b/kernel/module.c
index ad2d420024f6..292eee54e390 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -19,6 +19,7 @@
#include <linux/export.h>
#include <linux/extable.h>
#include <linux/moduleloader.h>
+#include <linux/module_signature.h>
#include <linux/trace_events.h>
#include <linux/init.h>
#include <linux/kallsyms.h>
diff --git a/kernel/module_signing.c b/kernel/module_signing.c
index 937c844bee4a..a7c2c461c598 100644
--- a/kernel/module_signing.c
+++ b/kernel/module_signing.c
@@ -11,36 +11,41 @@
#include <linux/kernel.h>
#include <linux/errno.h>
+#include <linux/module_signature.h>
#include <linux/string.h>
#include <linux/verification.h>
#include <crypto/public_key.h>
#include "module-internal.h"
-enum pkey_id_type {
- PKEY_ID_PGP, /* OpenPGP generated key ID */
- PKEY_ID_X509, /* X.509 arbitrary subjectKeyIdentifier */
- PKEY_ID_PKCS7, /* Signature in PKCS#7 message */
-};
-
-/*
- * Module signature information block.
+/**
+ * validate_module_sig - validate that the given signature is sane
*
- * The constituents of the signature section are, in order:
- *
- * - Signer's name
- * - Key identifier
- * - Signature data
- * - Information block
+ * @ms: Signature to validate.
+ * @file_len: Size of the file to which @ms is appended.
*/
-struct module_signature {
- u8 algo; /* Public-key crypto algorithm [0] */
- u8 hash; /* Digest algorithm [0] */
- u8 id_type; /* Key identifier type [PKEY_ID_PKCS7] */
- u8 signer_len; /* Length of signer's name [0] */
- u8 key_id_len; /* Length of key identifier [0] */
- u8 __pad[3];
- __be32 sig_len; /* Length of signature data */
-};
+int validate_module_sig(const struct module_signature *ms, size_t file_len)
+{
+ if (be32_to_cpu(ms->sig_len) >= file_len - sizeof(*ms))
+ return -EBADMSG;
+
+ if (ms->id_type != PKEY_ID_PKCS7) {
+ pr_err("Module is not signed with expected PKCS#7 message\n");
+ return -ENOPKG;
+ }
+
+ if (ms->algo != 0 ||
+ ms->hash != 0 ||
+ ms->signer_len != 0 ||
+ ms->key_id_len != 0 ||
+ ms->__pad[0] != 0 ||
+ ms->__pad[1] != 0 ||
+ ms->__pad[2] != 0) {
+ pr_err("PKCS#7 signature info has unexpected non-zero params\n");
+ return -EBADMSG;
+ }
+
+ return 0;
+}
/*
* Verify the signature on a module.
@@ -49,6 +54,7 @@ int mod_verify_sig(const void *mod, unsigned long *_modlen)
{
struct module_signature ms;
size_t modlen = *_modlen, sig_len;
+ int ret;
pr_devel("==>%s(,%zu)\n", __func__, modlen);
@@ -56,30 +62,15 @@ int mod_verify_sig(const void *mod, unsigned long *_modlen)
return -EBADMSG;
memcpy(&ms, mod + (modlen - sizeof(ms)), sizeof(ms));
- modlen -= sizeof(ms);
+
+ ret = validate_module_sig(&ms, modlen);
+ if (ret)
+ return ret;
sig_len = be32_to_cpu(ms.sig_len);
- if (sig_len >= modlen)
- return -EBADMSG;
- modlen -= sig_len;
+ modlen -= sig_len + sizeof(ms);
*_modlen = modlen;
- if (ms.id_type != PKEY_ID_PKCS7) {
- pr_err("Module is not signed with expected PKCS#7 message\n");
- return -ENOPKG;
- }
-
- if (ms.algo != 0 ||
- ms.hash != 0 ||
- ms.signer_len != 0 ||
- ms.key_id_len != 0 ||
- ms.__pad[0] != 0 ||
- ms.__pad[1] != 0 ||
- ms.__pad[2] != 0) {
- pr_err("PKCS#7 signature info has unexpected non-zero params\n");
- return -EBADMSG;
- }
-
return verify_pkcs7_signature(mod, modlen, mod + modlen, sig_len,
NULL, VERIFYING_MODULE_SIGNATURE,
NULL, NULL);
^ permalink raw reply related
* [PATCH v6 00/12] Appended signatures support for IMA appraisal
From: Thiago Jung Bauermann @ 2018-03-16 20:38 UTC (permalink / raw)
To: linux-integrity
Cc: linux-security-module, keyrings, linux-crypto, linuxppc-dev,
linux-kernel, Mimi Zohar, Dmitry Kasatkin, James Morris,
Serge E. Hallyn, David Howells, David Woodhouse, Jessica Yu,
Herbert Xu, David S. Miller, AKASHI, Takahiro,
Thiago Jung Bauermann
Hello,
The main highlight in this version is that it's not necessary to appraise
the file before storing its measurement anymore. This is possible due to a
new approach that Mimi suggested: we decide whether the modsig should be
used or not at the time it is read from the file, while before we would
only make that decision when trying to verify its signature (i.e., at
appraisal time).
Now the modsig is only ignored if it references a signature that is not
present in IMA's keyring (or if there's a parsing error, obviously). If the
signature verification fails, then appraisal fails and the xattr signature
(if there is one) is not used as a fallback. With this change, we already
know which signature will be used for appraisal at the time the measurement
is stored in the measurement list.
Also, now IMA first tries to use the xattr signature and only if it doesn't
exist or uses a key which isn't in IMA's keyring it will look for a modsig.
This is because an xattr sig was most likely placed there by the system
admin, while the modsig most likely came from the OS vendor and thus the
former should be given preference. Also, since modsig is only allowed in
hooks which aren't in any hotpath, there's no practical gain in speed by
avoiding to read the xattr.
There's also a new template field called 'd-sig' which will store the
digest used for verification of the modsig in the measurement list.
Finally, I dropped the patches removing superfluous parentheses from
expressions, since they add a lot of churn and whether they're an actual
improvement or not is subjective.
These patches apply on top of today's linux-integrity/next-integrity.
They also require patch 4/4 from my cleanups series I posted a few days
ago, which isn't in next-integrity yet:
https://lkml.org/lkml/2018/3/14/1029
For convenience, I pushed these patches to the following branch:
https://github.com/bauermann/linux.git ima-modsig
Original cover letter:
On the OpenPOWER platform, secure boot and trusted boot are being
implemented using IMA for taking measurements and verifying signatures.
Since the kernel image on Power servers is an ELF binary, kernels are
signed using the scripts/sign-file tool and thus use the same signature
format as signed kernel modules.
This patch series adds support in IMA for verifying those signatures.
It adds flexibility to OpenPOWER secure boot, because it allows it to boot
kernels with the signature appended to them as well as kernels where the
signature is stored in the IMA extended attribute.
Changes since v5:
- Patch "ima: Remove some superfluous parentheses"
- Dropped.
- Patch "evm, ima: Remove superfluous parentheses"
- Dropped.
- Patch "evm, ima: Remove more superfluous parentheses"
- Dropped.
- Patch "ima: Don't pass xattr value to EVM xattr verification."
- Dropped.
- Patch "ima: Store measurement after appraisal"
- Dropped.
- Patch "MODSIGN: Export module signature definitions"
- Reduced changes to the code that was moved into validate_module_sig()
to the minimum necessary (suggested by Mimi Zohar).
- Added SPDX license identifier.
- Patch "PKCS#7: Introduce pkcs7_get_message_sig() and verify_pkcs7_message_sig()"
- In the hypothetical case that there's more than one sinfo, changed
pkcs7_get_message_sig() to return NULL instead of the first sinfo's sig.
- Dropped Mimi's Reviewed-by because of the code change above.
- Patch "PKCS#7: Introduce pkcs7_get_digest()"
- New patch.
- Patch "integrity: Introduce integrity_keyring_from_id"
- Add stub in case CONFIG_INTEGRITY_SIGNATURE isn't set.
- Patch "integrity: Introduce asymmetric_sig_has_known_key()"
- New patch.
- Patch "ima: Introduce is_ima_sig"
- New patch, with code from "ima: Improvements in ima_appraise_measurement"
- Patch "ima: Add modsig appraise_type option for module-style appended signatures"
- Changed appraise_type to accept "imasig|modsig" instead of
"modsig|imasig" to reflect the fact that now IMA only looks for
the modsig after failing to find a suitable imasig stored in the xattr.
- Added SPDX license identifier.
- Patch "ima: Add functions to read and verify a modsig signature"
- Changed ima_read_modsig() to abort loading the modsig if it uses a key
which isn't known to IMA.
- Changed ima_get_modsig_hash() to use pkcs7_get_digest().
- Patch "ima: Implement support for module-style appended signatures"
- Added ima_xattr_sig_known_key() auxiliary function.
- Call ima_read_modsig() directly from process_measurement() instead of
from ima_appraise_measurement(), and only if there's no xattr signature
or if the xattr signature uses a key which isn't known to IMA.
- hash_algo in process_measurement() is always obtained from the xattr
signature, never from the modsig.
- Changes to ima_appraise_measurement() are a lot simpler now, and don't
involve going back to the main switch statement a second time.
- Pass xattr_value to evm_verifyxattr() unless xattr_value is a modsig.
- Patch "ima: Write modsig to the measurement list"
- Since now we determine whether we'll use an xattr sig or a modsig
at the time they are read, there's no need to store a measurement
again in the modsig case. Thus, this patch doesn't need to change
ima_store_measurement() nor process_measurement() anymore.
- Define new "d-sig" template field which holds the digest that is
expected to match the one contained in the modsig.
- Moved addition of ima_modsig_serialize_data() to patch "ima: Add
functions to read and verify a modsig signature".
- Increase MAX_TEMPLATE_NAME_LEN to 24.
Changes since v4:
- Patch "ima: Remove redundant conditional operator"
- New patch.
- Patch "ima: Remove some superfluous parentheses"
- New patch.
- Patch "evm, ima: Remove superfluous parentheses"
- New patch.
- Patch "evm, ima: Remove more superfluous parentheses"
- New patch.
- Patch "ima: Simplify ima_eventsig_init"
- New patch.
- Patch "ima: Improvements in ima_appraise_measurement"
- New patch.
- Patch "ima: Don't pass xattr value to EVM xattr verification."
- New patch.
- Patch "ima: Export func_tokens"
- Split from patch "ima: Support module-style appended signatures for
appraisal".
- Patch "ima: Add modsig appraise_type option for module-style appended
signatures"
- Split from patch "ima: Support module-style appended signatures for
appraisal".
- Mention modsig option in Documentation/ABI/testing/ima_policy
(suggested by Mimi Zohar).
- Patch "ima: Add functions to read and verify a modsig signature"
- Split from patch "ima: Support module-style appended signatures for
appraisal".
- Patch "ima: Implement support for module-style appended signatures"
- Split from patch "ima: Support module-style appended signatures for
appraisal".
- In ima_appraise_measurement, change the logic of dealing with xattr
errors in case the modsig verification fails. With this,
process_xattr_error isn't needed anymore.
- Patch "ima: Write modsig to the measurement list"
- Split from patch "ima: Support module-style appended signatures for
appraisal".
- Added ima_current_template_has_sig function.
- Removed hdr parameter from ima_modsig_serialize_data.
- In ima_store_measurement, continue processing even if the given PCR
is already measured if it's for a modsig.
- In process_measurement, add exception to store measurement even if
IMA_MEASURE is not set when appraising a modsig (suggested by
Mimi Zohar).
- Call is_ima_sig in ima_eventsig_init.
Changes since v3:
- Patch "integrity: Introduce struct evm_hmac_xattr"
- Renamed new struct to evm_xattr.
- Define struct evm_xattr using struct evm_ima_xattr_data, and moved it
from evm.h to integrity.h (suggested by Mimi Zohar).
- Patch "PKCS#7: Introduce verify_pkcs7_message_sig"
- Also introduce pkcs7_get_message_sig.
- Patch "ima: Support appended signatures for appraisal"
- Moved check for buffer presence and size from ima_appraise_measurement
to ima_read_modsig (suggested by Mimi Zohar).
- Factored out handling of ima_read_xattr return value into
process_xattr_error in ima_appraise_measurement so that it can be used
if the modsig verification fails.
- Pass NULL xattr_value to evm_verifyxattr even in the case of xattr
signature in ima_appraise_measurement (suggested by Mimi Zohar).
- Use switch statement provided by Mimi Zohar to check result of
evm_verifyxattr.
- If the modsig verification succeeds, copy the hash calculated during
the verification to the iint cache (suggested by Mimi Zohar).
- Substitute recursion in ima_appraise_measurement by a goto statement
back to the main switch statement (suggested by Mimi Zohar).
Thiago Jung Bauermann (12):
MODSIGN: Export module signature definitions
PKCS#7: Introduce pkcs7_get_message_sig() and
verify_pkcs7_message_sig()
PKCS#7: Introduce pkcs7_get_digest()
ima: Introduce is_ima_sig()
integrity: Introduce integrity_keyring_from_id()
integrity: Introduce asymmetric_sig_has_known_key()
integrity: Select CONFIG_KEYS instead of depending on it
ima: Export func_tokens
ima: Add modsig appraise_type option for module-style appended
signatures
ima: Add functions to read and verify a modsig signature
ima: Implement support for module-style appended signatures
ima: Write modsig to the measurement list
Documentation/ABI/testing/ima_policy | 6 +-
Documentation/security/IMA-templates.rst | 5 +
certs/system_keyring.c | 61 ++++++---
crypto/asymmetric_keys/pkcs7_parser.c | 16 +++
crypto/asymmetric_keys/pkcs7_verify.c | 25 ++++
include/crypto/pkcs7.h | 5 +
include/linux/module.h | 3 -
include/linux/module_signature.h | 44 +++++++
include/linux/verification.h | 10 ++
init/Kconfig | 6 +-
kernel/Makefile | 2 +-
kernel/module.c | 1 +
kernel/module_signing.c | 77 +++++------
security/integrity/Kconfig | 2 +-
security/integrity/digsig.c | 28 +++-
security/integrity/digsig_asymmetric.c | 44 +++++--
security/integrity/ima/Kconfig | 13 ++
security/integrity/ima/Makefile | 1 +
security/integrity/ima/ima.h | 66 ++++++++++
security/integrity/ima/ima_appraise.c | 60 +++++++--
security/integrity/ima/ima_main.c | 21 ++-
security/integrity/ima/ima_modsig.c | 212 ++++++++++++++++++++++++++++++
security/integrity/ima/ima_policy.c | 24 ++--
security/integrity/ima/ima_template.c | 4 +-
security/integrity/ima/ima_template_lib.c | 49 ++++++-
security/integrity/ima/ima_template_lib.h | 2 +
security/integrity/integrity.h | 16 +++
27 files changed, 693 insertions(+), 110 deletions(-)
create mode 100644 include/linux/module_signature.h
create mode 100644 security/integrity/ima/ima_modsig.c
^ permalink raw reply
* [PATCH] drivers: macintosh: rack-meter: really fix bogus memsets
From: Aaro Koskinen @ 2018-03-16 20:17 UTC (permalink / raw)
To: PowerPC, Stephen Rothwell, Michael Ellerman,
Benjamin Herrenschmidt
Cc: Aaro Koskinen
We should zero an array using sizeof instead of number of elements.
Fixes the following compiler (GCC 7.3.0) warnings:
drivers/macintosh/rack-meter.c: In function 'rackmeter_do_pause':
drivers/macintosh/rack-meter.c:157:2: warning: 'memset' used with length equal to number of elements without multiplication by element size [-Wmemset-elt-size]
drivers/macintosh/rack-meter.c:158:2: warning: 'memset' used with length equal to number of elements without multiplication by element size [-Wmemset-elt-size]
Fixes: 4f7bef7a9f69 ("drivers: macintosh: rack-meter: fix bogus memsets")
Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
Signed-off-by: Aaro Koskinen <aaro.koskinen@iki.fi>
---
drivers/macintosh/rack-meter.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/macintosh/rack-meter.c b/drivers/macintosh/rack-meter.c
index 910b5b6..eb65b6e 100644
--- a/drivers/macintosh/rack-meter.c
+++ b/drivers/macintosh/rack-meter.c
@@ -154,8 +154,8 @@ static void rackmeter_do_pause(struct rackmeter *rm, int pause)
DBDMA_DO_STOP(rm->dma_regs);
return;
}
- memset(rdma->buf1, 0, ARRAY_SIZE(rdma->buf1));
- memset(rdma->buf2, 0, ARRAY_SIZE(rdma->buf2));
+ memset(rdma->buf1, 0, sizeof(rdma->buf1));
+ memset(rdma->buf2, 0, sizeof(rdma->buf2));
rm->dma_buf_v->mark = 0;
--
2.9.2
^ permalink raw reply related
* Re: [PATCH v4] mm, pkey: treat pkey-0 special
From: Ram Pai @ 2018-03-16 19:31 UTC (permalink / raw)
To: Balbir Singh
Cc: Michael Ellerman, Ingo Molnar, akpm@linux-foundation.org,
open list:LINUX FOR POWERPC (32-BIT AND 64-BIT), linux-mm,
maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT), linux-arch,
linux-kernel@vger.kernel.org, Dave Hansen, Benjamin Herrenschmidt,
Paul Mackerras, Anshuman Khandual, Aneesh Kumar KV,
Haren Myneni/Beaverton/IBM, Michal Hocko, Thiago Jung Bauermann,
Eric W. Biederman, Jonathan Corbet, Arnd Bergmann, fweimer,
msuchanek, Thomas Gleixner, Ulrich.Weigand, Ram Pai
In-Reply-To: <CAKTCnzmSCT+VecdSRpyY2Rb_AW2ngCi3UTZfLE3VOLNSQn6vsA@mail.gmail.com>
On Fri, Mar 16, 2018 at 10:02:22PM +1100, Balbir Singh wrote:
> On Fri, Mar 16, 2018 at 9:33 PM, Ram Pai <linuxram@us.ibm.com> wrote:
> > Applications need the ability to associate an address-range with some
> > key and latter revert to its initial default key. Pkey-0 comes close to
> > providing this function but falls short, because the current
> > implementation disallows applications to explicitly associate pkey-0 to
> > the address range.
> >
> > Clarify the semantics of pkey-0 and provide the corresponding
> > implementation.
> >
> > Pkey-0 is special with the following semantics.
> > (a) it is implicitly allocated and can never be freed. It always exists.
> > (b) it is the default key assigned to any address-range.
> > (c) it can be explicitly associated with any address-range.
> >
> > Tested on powerpc only. Could not test on x86.
>
>
> Ram,
>
> I was wondering if we should check the AMOR values on the ppc side to make sure
> that pkey0 is indeed available for use as default. I am still of the
> opinion that we
AMOR cannot be read/written by the OS in priviledge-non-hypervisor-mode.
We could try testing if key-0 is available to the OS by temproarily
changing the bits key-0 bits of AMR or IAMR register. But will be
dangeorous to do, for you might disable read,execute of all the pages,
since all pages are asscoiated with key-0 bydefault.
May be we can play with UAMOR register and check if its key-0 can be
modified. That is a good indication that key-0 is available.
If it is not available, disable the pkey-subsystem, and operate
the legacy way; no pkeys.
> should consider non-0 default pkey in the long run. I'm OK with the patches for
> now, but really 0 is not special except for it being the default bit
> values present
> in the PTE.
it will be a pain. Any new pte that gets instantiated will now have to
explicitly initialize its key to this default-non-zero-key. I hope
we or any architecture goes there ever.
--
Ram Pai
^ permalink raw reply
* Re: [PATCH RFC rebase 3/9] powerpc/64: Use barrier_nospec in syscall entry
From: Linus Torvalds @ 2018-03-16 17:08 UTC (permalink / raw)
To: Michal Suchánek
Cc: Nicholas Piggin, Kate Stewart, Madhavan Srinivasan,
Mahesh Salgaonkar, Al Viro, Paul Mackerras, Michael Neuling,
Bryant G. Ly, Naveen N. Rao, Daniel Axtens, Cédric Le Goater,
David Gibson, Greg Kroah-Hartman, Linux Kernel Mailing List,
Sergey Senozhatsky, Masami Hiramatsu, Andrew Donnellan,
Philippe Ombredanne, Joe Perches, Oliver O'Halloran,
Andrew Morton, ppc-dev, Tobin C. Harding
In-Reply-To: <20180316101549.31238bdf@naga.suse.cz>
On Fri, Mar 16, 2018 at 2:15 AM, Michal Such=C3=A1nek <msuchanek@suse.de> w=
rote:
>
> As far as I understand barriers they separate code before the barrier
> and code after the barrier.
Almost certainly not. Even if you were to do an expensive
serialization before the branch, the branch will still predict after
the serialization.
The thing is, it doesn't make sense to insert a barrier before a
conditional branch for Spectre mitigation.
The problem is not that the data isn't ready for the branch - the
problem is that the branch is predicted _regardless_ of the data.
Sure, some micro-architecture might not predict branches at all if
they have a stable conditional, so a barrier _can_ make sense.
But fundamentally, good branch prediction - in order to be optimal -
has to happen before instructions have even been parsed, much less
things like "stable conditional register state" having been decided
on. You'll want to do I$ prefetching etc.
So the problem is that even if the data is ready, the branch will be
predicted according to some unrelated historical data, and a barrier
to make the branch conditional be stable is pointless.
A barrier *after* the branch, making sure that you don't actually
start executing instructions past it (even if you might have predicted
and fetched stuff past it) *if* you have mis-predicted the previous
branch, is what a sane architecture would specify.
Of course, on x86, we mostly tried to avoid branch prediction being
the critical problem and having to have barriers by just making it an
address generation dependency instead. That should presumably work on
powerpc too, since address generation is part of the memory ordering
definition. But obviously a microarchitecture *could* end up
speculating and just redoing even for memory ordering, and maybe the
ppc architects prefer the barrier since they are already used to crazy
and not very well architected barriers elsewhere.
Linus
^ permalink raw reply
* Re: [PATCH 2/3] rfi-flush: Make it possible to call setup_rfi_flush() again
From: Mauricio Faria de Oliveira @ 2018-03-16 16:51 UTC (permalink / raw)
To: Michael Ellerman; +Cc: Michal Suchánek, linuxppc-dev
In-Reply-To: <87lgesgl2y.fsf@concordia.ellerman.id.au>
Michael,
On 03/16/2018 11:18 AM, Michael Ellerman wrote:
>> I still think the correct, informative messages are a good way to go:)
> Yeah I agree.
>
> We probably want to do both, print what's available at boot, and print
> what's actually patched when the patching happens.
Nice. Not sure you had a chance to review yet, but 'PATCH v3 4/5' does
exactly that :- )
I think its implementation of the latter part looks a bit strange, but I
couldn't figure an elegant way to fit that in (either that one or string
array indexed by flush-type possible values or a long if-else chain).
I'd be happy with suggestions if it's preferred in some other way.
cheers,
Mauricio
^ permalink raw reply
* Re: [PATCH v3 1/5] rfi-flush: Move the logic to avoid a redo into the debugfs code
From: Mauricio Faria de Oliveira @ 2018-03-16 16:42 UTC (permalink / raw)
To: Michal Suchánek, Murilo Opsfelder Araujo; +Cc: linuxppc-dev
In-Reply-To: <20180316095200.78f31a50@naga.suse.cz>
Hi Murilo and Michal,
On 03/16/2018 05:52 AM, Michal Suchánek wrote:
>> Do we need to take into account if no_rfi_flush == true?
> I think it makes sense you are able to override that using debugfs.
>
> It's interface used for diagnostics and testing.
>
> If this was in sysfs it would be a different story.
Yes, I agree. The debugfs is way to override the cmdline option.
Thanks for looking carefully at this :)
cheers,
Mauricio
^ permalink raw reply
* Re: [mm] b33ddf50eb: INFO:trying_to_register_non-static_key
From: Laurent Dufour @ 2018-03-16 16:38 UTC (permalink / raw)
To: kernel test robot
Cc: paulmck, peterz, akpm, kirill, ak, mhocko, dave, jack,
Matthew Wilcox, benh, mpe, paulus, Thomas Gleixner, Ingo Molnar,
hpa, Will Deacon, Sergey Senozhatsky, Andrea Arcangeli,
Alexei Starovoitov, kemi.wang, sergey.senozhatsky.work,
Daniel Jordan, linux-kernel, linux-mm, haren, khandual, npiggin,
bsingharora, Tim Chen, linuxppc-dev, x86, lkp
In-Reply-To: <20180316102359.pzjwi24hbkhnyk2a@inn>
On 16/03/2018 11:23, kernel test robot wrote:
> FYI, we noticed the following commit (built with gcc-7):
>
> commit: b33ddf50ebcc740b990dd2e0e8ff0b92c7acf58e ("mm: Protect mm_rb tree with a rwlock")
> url: https://github.com/0day-ci/linux/commits/Laurent-Dufour/Speculative-page-faults/20180316-151833
>
>
> in testcase: boot
>
> on test machine: qemu-system-x86_64 -enable-kvm -cpu host -smp 2 -m 4G
>
> caused below changes (please refer to attached dmesg/kmsg for entire log/backtrace):
>
>
> +----------------------------------------+------------+------------+
> | | 7f3f7b4e80 | b33ddf50eb |
> +----------------------------------------+------------+------------+
> | boot_successes | 8 | 0 |
> | boot_failures | 0 | 6 |
> | INFO:trying_to_register_non-static_key | 0 | 6 |
> +----------------------------------------+------------+------------+
>
>
>
> [ 22.218186] INFO: trying to register non-static key.
> [ 22.220252] the code is fine but needs lockdep annotation.
> [ 22.222471] turning off the locking correctness validator.
> [ 22.224839] CPU: 0 PID: 1 Comm: init Not tainted 4.16.0-rc4-next-20180309-00017-gb33ddf5 #1
> [ 22.228528] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.10.2-1 04/01/2014
> [ 22.232443] Call Trace:
> [ 22.234234] dump_stack+0x85/0xbc
> [ 22.236085] register_lock_class+0x237/0x477
> [ 22.238057] __lock_acquire+0xd0/0xf15
> [ 22.240032] lock_acquire+0x19c/0x1ce
> [ 22.241927] ? do_mmap+0x3aa/0x3ff
> [ 22.243749] mmap_region+0x37a/0x4c0
> [ 22.245619] ? do_mmap+0x3aa/0x3ff
> [ 22.247425] do_mmap+0x3aa/0x3ff
> [ 22.249175] vm_mmap_pgoff+0xa1/0xea
> [ 22.251083] elf_map+0x6d/0x134
> [ 22.252873] load_elf_binary+0x56f/0xe07
> [ 22.254853] search_binary_handler+0x75/0x1f8
> [ 22.256934] do_execveat_common+0x661/0x92b
> [ 22.259164] ? rest_init+0x22e/0x22e
> [ 22.261082] do_execve+0x1f/0x21
> [ 22.262884] kernel_init+0x5a/0xf0
> [ 22.264722] ret_from_fork+0x3a/0x50
> [ 22.303240] systemd[1]: RTC configured in localtime, applying delta of 480 minutes to system time.
> [ 22.313544] systemd[1]: Failed to insert module 'autofs4': No such file or directory
Thanks a lot for reporting this.
I found the issue introduced in that patch.
I mistakenly remove in the call to seqcount_init(&vma->vm_sequence) in
__vma_link_rb().
This doesn't have a functional impact as the vm_sequence is incremented
monotonically.
I'll fix that in the next series.
Laurent.
^ permalink raw reply
* Re: [PATCH] cxl: Add new kernel traces
From: Frederic Barrat @ 2018-03-16 15:56 UTC (permalink / raw)
To: Christophe Lombard, linuxppc-dev, vaibhav, andrew.donnellan
In-Reply-To: <1520933440-24652-1-git-send-email-clombard@linux.vnet.ibm.com>
Le 13/03/2018 à 10:30, Christophe Lombard a écrit :
> This patch adds new kernel traces in the current in-kernel 'library'
> which can be called by other drivers to help interacting with an
> IBM XSL on a POWER9 system.
>
> If some kernel traces exist in the 'normal path' to handle a page or a
> segment fault, some others are missing when a page fault is handle
> through cxllib.
>
> Signed-off-by: Christophe Lombard <clombard@linux.vnet.ibm.com>
> ---
Thanks!
Acked-by: Frederic Barrat <fbarrat@linux.vnet.ibm.com>
> drivers/misc/cxl/cxllib.c | 3 ++
> drivers/misc/cxl/fault.c | 2 +
> drivers/misc/cxl/irq.c | 2 +-
> drivers/misc/cxl/trace.h | 115 ++++++++++++++++++++++++++--------------------
> 4 files changed, 72 insertions(+), 50 deletions(-)
>
> diff --git a/drivers/misc/cxl/cxllib.c b/drivers/misc/cxl/cxllib.c
> index 30ccba4..91cfb69 100644
> --- a/drivers/misc/cxl/cxllib.c
> +++ b/drivers/misc/cxl/cxllib.c
> @@ -13,6 +13,7 @@
> #include <misc/cxllib.h>
>
> #include "cxl.h"
> +#include "trace.h"
>
> #define CXL_INVALID_DRA ~0ull
> #define CXL_DUMMY_READ_SIZE 128
> @@ -218,6 +219,8 @@ int cxllib_handle_fault(struct mm_struct *mm, u64 addr, u64 size, u64 flags)
> if (mm == NULL)
> return -EFAULT;
>
> + trace_cxl_lib_handle_fault(addr, size, flags);
> +
> down_read(&mm->mmap_sem);
>
> vma = find_vma(mm, addr);
> diff --git a/drivers/misc/cxl/fault.c b/drivers/misc/cxl/fault.c
> index 70dbb6d..1c4fd74 100644
> --- a/drivers/misc/cxl/fault.c
> +++ b/drivers/misc/cxl/fault.c
> @@ -138,6 +138,8 @@ int cxl_handle_mm_fault(struct mm_struct *mm, u64 dsisr, u64 dar)
> int result;
> unsigned long access, flags, inv_flags = 0;
>
> + trace_cxl_handle_mm_fault(dsisr, dar);
> +
> /*
> * Add the fault handling cpu to task mm cpumask so that we
> * can do a safe lockless page table walk when inserting the
> diff --git a/drivers/misc/cxl/irq.c b/drivers/misc/cxl/irq.c
> index ce08a9f..79b8b49 100644
> --- a/drivers/misc/cxl/irq.c
> +++ b/drivers/misc/cxl/irq.c
> @@ -41,7 +41,7 @@ irqreturn_t cxl_irq_psl9(int irq, struct cxl_context *ctx, struct cxl_irq_info *
> dsisr = irq_info->dsisr;
> dar = irq_info->dar;
>
> - trace_cxl_psl9_irq(ctx, irq, dsisr, dar);
> + trace_cxl_psl_irq(ctx, irq, dsisr, dar);
>
> pr_devel("CXL interrupt %i for afu pe: %i DSISR: %#llx DAR: %#llx\n", irq, ctx->pe, dsisr, dar);
>
> diff --git a/drivers/misc/cxl/trace.h b/drivers/misc/cxl/trace.h
> index b8e300a..8eb2607 100644
> --- a/drivers/misc/cxl/trace.h
> +++ b/drivers/misc/cxl/trace.h
> @@ -26,19 +26,20 @@
> { CXL_PSL9_DSISR_An_OC, "OC" }, \
> { CXL_PSL9_DSISR_An_S, "S" })
>
> -#define DSISR_FLAGS \
> - { CXL_PSL_DSISR_An_DS, "DS" }, \
> - { CXL_PSL_DSISR_An_DM, "DM" }, \
> - { CXL_PSL_DSISR_An_ST, "ST" }, \
> - { CXL_PSL_DSISR_An_UR, "UR" }, \
> - { CXL_PSL_DSISR_An_PE, "PE" }, \
> - { CXL_PSL_DSISR_An_AE, "AE" }, \
> - { CXL_PSL_DSISR_An_OC, "OC" }, \
> - { CXL_PSL_DSISR_An_M, "M" }, \
> - { CXL_PSL_DSISR_An_P, "P" }, \
> - { CXL_PSL_DSISR_An_A, "A" }, \
> - { CXL_PSL_DSISR_An_S, "S" }, \
> - { CXL_PSL_DSISR_An_K, "K" }
> +#define dsisr_psl8_flags(flags) \
> + __print_flags(flags, "|", \
> + { CXL_PSL_DSISR_An_DS, "DS" }, \
> + { CXL_PSL_DSISR_An_DM, "DM" }, \
> + { CXL_PSL_DSISR_An_ST, "ST" }, \
> + { CXL_PSL_DSISR_An_UR, "UR" }, \
> + { CXL_PSL_DSISR_An_PE, "PE" }, \
> + { CXL_PSL_DSISR_An_AE, "AE" }, \
> + { CXL_PSL_DSISR_An_OC, "OC" }, \
> + { CXL_PSL_DSISR_An_M, "M" }, \
> + { CXL_PSL_DSISR_An_P, "P" }, \
> + { CXL_PSL_DSISR_An_A, "A" }, \
> + { CXL_PSL_DSISR_An_S, "S" }, \
> + { CXL_PSL_DSISR_An_K, "K" })
>
> #define TFC_FLAGS \
> { CXL_PSL_TFC_An_A, "A" }, \
> @@ -163,7 +164,7 @@ TRACE_EVENT(cxl_afu_irq,
> )
> );
>
> -TRACE_EVENT(cxl_psl9_irq,
> +TRACE_EVENT(cxl_psl_irq,
> TP_PROTO(struct cxl_context *ctx, int irq, u64 dsisr, u64 dar),
>
> TP_ARGS(ctx, irq, dsisr, dar),
> @@ -192,40 +193,8 @@ TRACE_EVENT(cxl_psl9_irq,
> __entry->pe,
> __entry->irq,
> __entry->dsisr,
> - dsisr_psl9_flags(__entry->dsisr),
> - __entry->dar
> - )
> -);
> -
> -TRACE_EVENT(cxl_psl_irq,
> - TP_PROTO(struct cxl_context *ctx, int irq, u64 dsisr, u64 dar),
> -
> - TP_ARGS(ctx, irq, dsisr, dar),
> -
> - TP_STRUCT__entry(
> - __field(u8, card)
> - __field(u8, afu)
> - __field(u16, pe)
> - __field(int, irq)
> - __field(u64, dsisr)
> - __field(u64, dar)
> - ),
> -
> - TP_fast_assign(
> - __entry->card = ctx->afu->adapter->adapter_num;
> - __entry->afu = ctx->afu->slice;
> - __entry->pe = ctx->pe;
> - __entry->irq = irq;
> - __entry->dsisr = dsisr;
> - __entry->dar = dar;
> - ),
> -
> - TP_printk("afu%i.%i pe=%i irq=%i dsisr=%s dar=0x%016llx",
> - __entry->card,
> - __entry->afu,
> - __entry->pe,
> - __entry->irq,
> - __print_flags(__entry->dsisr, "|", DSISR_FLAGS),
> + cxl_is_power8() ? dsisr_psl8_flags(__entry->dsisr) :
> + dsisr_psl9_flags(__entry->dsisr),
> __entry->dar
> )
> );
> @@ -342,11 +311,59 @@ TRACE_EVENT(cxl_pte_miss,
> __entry->card,
> __entry->afu,
> __entry->pe,
> - __print_flags(__entry->dsisr, "|", DSISR_FLAGS),
> + cxl_is_power8() ? dsisr_psl8_flags(__entry->dsisr) :
> + dsisr_psl9_flags(__entry->dsisr),
> + __entry->dar
> + )
> +);
> +
> +TRACE_EVENT(cxl_handle_mm_fault,
> + TP_PROTO(u64 dsisr, u64 dar),
> +
> + TP_ARGS(dsisr, dar),
> +
> + TP_STRUCT__entry(
> + __field(u64, dsisr)
> + __field(u64, dar)
> + ),
> +
> + TP_fast_assign(
> + __entry->dsisr = dsisr;
> + __entry->dar = dar;
> + ),
> +
> + TP_printk("dsisr=0x%016llx(%s), dar=0x%016llx",
> + __entry->dsisr,
> + cxl_is_power8() ? dsisr_psl8_flags(__entry->dsisr) :
> + dsisr_psl9_flags(__entry->dsisr),
> __entry->dar
> )
> );
>
> +TRACE_EVENT(cxl_lib_handle_fault,
> + TP_PROTO(u64 addr, u64 size, u64 flags),
> +
> + TP_ARGS(addr, size, flags),
> +
> + TP_STRUCT__entry(
> + __field(u64, addr)
> + __field(u64, size)
> + __field(u64, flags)
> + ),
> +
> + TP_fast_assign(
> + __entry->addr = addr;
> + __entry->size = size;
> + __entry->flags = flags;
> + ),
> +
> + TP_printk("addr=0x%016llx, size=0x%016llx, flags=0x%016llx",
> + __entry->addr,
> + __entry->size,
> + __entry->flags
> + )
> +);
> +
> TRACE_EVENT(cxl_llcmd,
> TP_PROTO(struct cxl_context *ctx, u64 cmd),
>
^ permalink raw reply
* Re: [PATCH] powerpc: Use common error handling code in setup_new_fdt()
From: Dan Carpenter @ 2018-03-16 10:35 UTC (permalink / raw)
To: Michael Ellerman
Cc: Thiago Jung Bauermann, SF Markus Elfring, linuxppc-dev,
Benjamin Herrenschmidt, Borislav Petkov, Brijesh Singh,
Josh Sklar, Kees Cook, Paul Mackerras, Thomas Gleixner,
Tom Lendacky, LKML, kernel-janitors
In-Reply-To: <87bmfofh8y.fsf@concordia.ellerman.id.au>
On Fri, Mar 16, 2018 at 09:26:53PM +1100, Michael Ellerman wrote:
> Dan Carpenter <dan.carpenter@oracle.com> writes:
> > On Wed, Mar 14, 2018 at 06:22:07PM -0300, Thiago Jung Bauermann wrote:
> >> SF Markus Elfring <elfring@users.sourceforge.net> writes:
> >> > From: Markus Elfring <elfring@users.sourceforge.net>
> >> > Date: Sun, 11 Mar 2018 09:03:42 +0100
> >> >
> >> > Add a jump target so that a bit of exception handling can be better reused
> >> > at the end of this function.
> >> >
> >> > This issue was detected by using the Coccinelle software.
> >> >
> >> > Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> >> > ---
> >> > arch/powerpc/kernel/machine_kexec_file_64.c | 28 ++++++++++++----------------
> >> > 1 file changed, 12 insertions(+), 16 deletions(-)
> >>
> >> I liked it. Thanks!
> >>
> >> Reviewed-by: Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com>
> >
> > You know that compilers already re-use string constants so this doesn't
> > actually save memory?
>
> Sure, but it's still clearer to only have the string appear once in the
> code.
>
To me the original was better.
> > Also we should be preserving the error codes
> > instead of always returning -EINVAL.
>
> The error codes come from libfdt code, so they don't necessarily make
> sense in the kernel. eg. FDT_ERR_NOSPACE == 3 == ESRCH.
>
> Perhaps we should be trying harder to convert them, but that's a
> criticism of the original code not this patch.
Ah. You're right. I look at the patch in context, sorry.
regards,
dan carpenter
^ permalink raw reply
* Re: [RFC PATCH 2/2] powerpc: Only support DYNAMIC_FTRACE not static
From: Steven Rostedt @ 2018-03-16 14:42 UTC (permalink / raw)
To: Michael Ellerman; +Cc: linuxppc-dev, linux-kernel, linux-arch, naveen.n.rao
In-Reply-To: <20180316134633.10584-2-mpe@ellerman.id.au>
On Sat, 17 Mar 2018 00:46:33 +1100
Michael Ellerman <mpe@ellerman.id.au> wrote:
> We've had dynamic ftrace support for over 9 years since Steve first
> wrote it, all the distros use dynamic, and static is basically
> untested these days, so drop support for static ftrace.
>
> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
> ---
> arch/powerpc/Kconfig | 1 +
> arch/powerpc/include/asm/ftrace.h | 4 +---
> arch/powerpc/include/asm/module.h | 5 -----
> arch/powerpc/kernel/trace/ftrace.c | 2 --
> arch/powerpc/kernel/trace/ftrace_32.S | 20 ------------------
> arch/powerpc/kernel/trace/ftrace_64.S | 29 --------------------------
> arch/powerpc/kernel/trace/ftrace_64_mprofile.S | 3 ---
> arch/powerpc/kernel/trace/ftrace_64_pg.S | 2 --
> 8 files changed, 2 insertions(+), 64 deletions(-)
>
> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> index 73ce5dd07642..23a325df784a 100644
> --- a/arch/powerpc/Kconfig
> +++ b/arch/powerpc/Kconfig
> @@ -189,6 +189,7 @@ config PPC
> select HAVE_DEBUG_STACKOVERFLOW
> select HAVE_DMA_API_DEBUG
> select HAVE_DYNAMIC_FTRACE
> + select HAVE_DYNAMIC_FTRACE_ONLY
I still think adding:
select DYNAMIC_FTRACE if FUNCTION_TRACER
is the better approach.
But I'm all for this patch. I've debated doing the same thing for x86,
but the only reason I have not, was because it's the only way I test
the !DYNAMIC_FTRACE code. I've broken the static function tracing
several times and only find out during my test suite that still tests
that case. But yeah, it would be nice to just nuke static function
tracing for all archs. Perhaps after we finish removing unused archs,
that may be the way to go forward.
-- Steve
> select HAVE_DYNAMIC_FTRACE_WITH_REGS if MPROFILE_KERNEL
> select HAVE_EBPF_JIT if PPC64
> select HAVE_EFFICIENT_UNALIGNED_ACCESS if !(CPU_LITTLE_ENDIAN && POWER7_CPU)
^ permalink raw reply
* Re: [RFC PATCH 1/2] ftrace: Allow arches to opt-out of static ftrace
From: Steven Rostedt @ 2018-03-16 14:40 UTC (permalink / raw)
To: Michael Ellerman; +Cc: linuxppc-dev, linux-kernel, linux-arch, naveen.n.rao
In-Reply-To: <20180316134633.10584-1-mpe@ellerman.id.au>
On Sat, 17 Mar 2018 00:46:32 +1100
Michael Ellerman <mpe@ellerman.id.au> wrote:
> There is a small but non-zero amount of code required by arches to
> suppory non-dynamic (static) ftrace, and more importantly there is the
> added work of testing both configurations.
>
> There are also almost no down sides to dynamic ftrace once it's well
> tested, other than a small increase in code/data size.
>
> So give arches the option to opt-out of supporting static ftrace.
>
> This is implemented as a DYNAMIC_FTRACE_CHOICE option, which controls
> whether DYNAMIC_FTRACE is presented as a user-selectable option or if
> it is just enabled based on its dependencies being enabled (because
> it's already default y).
>
> Then the CHOICE option depends on an arch *not* selecting
> HAVE_DYNAMIC_FTRACE_ONLY. This would be more natural in reverse, as a
> HAVE_STATIC_FTRACE option, but that would require updating every arch.
>
> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
Why not just add in arch/powerpc/Kconfig:
config PPC
[..]
select DYNAMIC_FTRACE if FUNCTION_TRACER
?
It seems to work for me.
-- Steve
^ permalink raw reply
* Re: [PATCH 2/3] rfi-flush: Make it possible to call setup_rfi_flush() again
From: Michael Ellerman @ 2018-03-16 14:18 UTC (permalink / raw)
To: Mauricio Faria de Oliveira, Michal Suchánek; +Cc: linuxppc-dev
In-Reply-To: <ff878619-a2c4-2dd2-8d39-ed0a535a9b8d@linux.vnet.ibm.com>
Mauricio Faria de Oliveira <mauricfo@linux.vnet.ibm.com> writes:
> On 03/13/2018 03:36 PM, Michal Such=C3=A1nek wrote:
>> On Tue, 13 Mar 2018 15:13:11 -0300
>> Mauricio Faria de Oliveira<mauricfo@linux.vnet.ibm.com> wrote:
>>=20
>>> On 03/13/2018 02:59 PM, Michal Such=C3=A1nek wrote:
>>>> Maybe it would make more sense to move the messages to the function
>>>> that actually patches in the instructions?
>
>>> That helps, but if the instructions are not patched (e.g.,
>>> no_rfi_flush) then there is no information about what the system
>>> actually supports, which is useful for diagnostics/debugging (and
>>> patch verification!:-) )
>
>> Can't you patch with debugfs in that case?
>
> For development purposes, yes, sure; but unfortunately sometimes only a
> dmesg output or other offline/postmortem data is available.
>
> And there's the user case where he is not aware/willing/allowed to use
> the debugfs switch.
>
> I still think the correct, informative messages are a good way to go :)
Yeah I agree.
We probably want to do both, print what's available at boot, and print
what's actually patched when the patching happens.
cheers
^ permalink raw reply
* Re: [PATCH 03/14] powerpc/64s: allocate lppacas individually
From: Michael Ellerman @ 2018-03-16 14:16 UTC (permalink / raw)
To: Nicholas Piggin; +Cc: linuxppc-dev
In-Reply-To: <20180313225438.0c853b89@roar.ozlabs.ibm.com>
Nicholas Piggin <npiggin@gmail.com> writes:
> On Tue, 13 Mar 2018 23:41:46 +1100
> Michael Ellerman <mpe@ellerman.id.au> wrote:
>> Nicholas Piggin <npiggin@gmail.com> writes:
>> > diff --git a/arch/powerpc/platforms/pseries/kexec.c b/arch/powerpc/platforms/pseries/kexec.c
>> > index eeb13429d685..3fe126796975 100644
>> > --- a/arch/powerpc/platforms/pseries/kexec.c
>> > +++ b/arch/powerpc/platforms/pseries/kexec.c
>> > @@ -23,7 +23,12 @@
>> >
>> > void pseries_kexec_cpu_down(int crash_shutdown, int secondary)
>> > {
>> > - /* Don't risk a hypervisor call if we're crashing */
>> > + /*
>> > + * Don't risk a hypervisor call if we're crashing
>> > + * XXX: Why? The hypervisor is not crashing. It might be better
>> > + * to at least attempt unregister to avoid the hypervisor stepping
>> > + * on our memory.
>> > + */
>>
>> Because every extra line of code we run in the crashed kernel is another
>> opportunity to screw up and not make it into the kdump kernel.
>>
>> For example the hcalls we do to unregister the VPA might trigger hcall
>> tracing which runs a bunch of code and might trip up on something. We
>> could modify those hcalls to not be traced, but then we can't trace them
>> in normal operation.
>
> We really make no other hcalls in a crash? I didn't think of that.
We do, but they're explicitly written to use plpar_hcall_raw().
And TBH I haven't tested a kdump with hcall tracing enabled lately, so
for all I know it's broken, but that's the theory at least.
cheers
^ permalink raw reply
* Re: dtc warnings
From: Michael Ellerman @ 2018-03-16 14:13 UTC (permalink / raw)
To: Stephen Rothwell, ppc-dev
In-Reply-To: <20180314161854.51b994ff@canb.auug.org.au>
Stephen Rothwell <sfr@canb.auug.org.au> writes:
> Hi all,
>
>
> I get the following from a powerpc_ppc44x defconfig build in current
> linux-next:
>
> arch/powerpc/boot/ebony.dtb: Warning (pci_bridge): /plb/pci@20ec00000: missing bus-range for PCI bridge
> arch/powerpc/boot/ebony.dtb: Warning (pci_device_bus_num): Failed prerequisite 'pci_bridge'
> arch/powerpc/boot/ebony.dtb: Warning (chosen_node_stdout_path): /chosen:linux,stdout-path: Use 'stdout-path' instead
> arch/powerpc/boot/sequoia.dtb: Warning (pci_bridge): /plb/pci@1ec000000: missing bus-range for PCI bridge
> arch/powerpc/boot/sequoia.dtb: Warning (pci_device_bus_num): Failed prerequisite 'pci_bridge'
> arch/powerpc/boot/sequoia.dtb: Warning (chosen_node_stdout_path): /chosen:linux,stdout-path: Use 'stdout-path' instead
> arch/powerpc/boot/sam440ep.dtb: Warning (pci_bridge): /plb/pci@ec000000: missing bus-range for PCI bridge
> arch/powerpc/boot/sam440ep.dtb: Warning (pci_device_bus_num): Failed prerequisite 'pci_bridge'
> arch/powerpc/boot/sam440ep.dtb: Warning (chosen_node_stdout_path): /chosen:linux,stdout-path: Use 'stdout-path' instead
> arch/powerpc/boot/rainier.dtb: Warning (pci_bridge): /plb/pci@1ec000000: missing bus-range for PCI bridge
> arch/powerpc/boot/rainier.dtb: Warning (pci_device_bus_num): Failed prerequisite 'pci_bridge'
> arch/powerpc/boot/rainier.dtb: Warning (chosen_node_stdout_path): /chosen:linux,stdout-path: Use 'stdout-path' instead
> arch/powerpc/boot/taishan.dtb: Warning (pci_bridge): /plb/pci@20ec00000: missing bus-range for PCI bridge
> arch/powerpc/boot/taishan.dtb: Warning (pci_device_bus_num): Failed prerequisite 'pci_bridge'
> arch/powerpc/boot/taishan.dtb: Warning (chosen_node_stdout_path): /chosen:linux,stdout-path: Use 'stdout-path' instead
> arch/powerpc/boot/bamboo.dtb: Warning (pci_bridge): /plb/pci@ec000000: missing bus-range for PCI bridge
> arch/powerpc/boot/bamboo.dtb: Warning (pci_device_bus_num): Failed prerequisite 'pci_bridge'
> arch/powerpc/boot/bamboo.dtb: Warning (chosen_node_stdout_path): /chosen:linux,stdout-path: Use 'stdout-path' instead
> arch/powerpc/boot/katmai.dtb: Warning (pci_bridge): /plb/pciex@d00000000: node name is not "pci" or "pcie"
> arch/powerpc/boot/katmai.dtb: Warning (pci_bridge): /plb/pciex@d20000000: node name is not "pci" or "pcie"
> arch/powerpc/boot/katmai.dtb: Warning (pci_bridge): /plb/pciex@d40000000: node name is not "pci" or "pcie"
> arch/powerpc/boot/katmai.dtb: Warning (pci_device_bus_num): Failed prerequisite 'pci_bridge'
> arch/powerpc/boot/katmai.dtb: Warning (chosen_node_stdout_path): /chosen:linux,stdout-path: Use 'stdout-path' instead
> arch/powerpc/boot/warp.dtb: Warning (chosen_node_stdout_path): /chosen:linux,stdout-path: Use 'stdout-path' instead
> arch/powerpc/boot/yosemite.dtb: Warning (pci_bridge): /plb/pci@ec000000: missing bus-range for PCI bridge
> arch/powerpc/boot/yosemite.dtb: Warning (pci_device_bus_num): Failed prerequisite 'pci_bridge'
> arch/powerpc/boot/yosemite.dtb: Warning (chosen_node_stdout_path): /chosen:linux,stdout-path: Use 'stdout-path' instead
>
> I though someone might like to do something about them ...
Feel free!
Oh that's not what you meant :P
Rob sent a patch to fix most of them:
http://patchwork.ozlabs.org/patch/879475/
I thought he was going to merge it, but actually I'm not sure why I
thought that. So I should probably merge it.
cheers
^ permalink raw reply
* Re: [PATCH 04/21] powerpc: Mark both tmp variables as unused
From: Michael Ellerman @ 2018-03-16 14:08 UTC (permalink / raw)
To: Christophe LEROY, Mathieu Malaterre
Cc: linux-kernel, Paul Mackerras, Jiri Slaby, linuxppc-dev
In-Reply-To: <3f881ec2-eb0f-77e6-f1c5-95530b8f84ad@c-s.fr>
Christophe LEROY <christophe.leroy@c-s.fr> writes:
> Le 25/02/2018 =C3=A0 18:22, Mathieu Malaterre a =C3=A9crit=C2=A0:
>> Since the value of `tmp` is never intended to be read, declare both `tmp`
>> variables as unused. Fix warning (treated as error in W=3D1):
>
> What about using fault_in_pages_readable() instead ?
Yeah that looks like it would work.
I'd be happy to take a tested patch :D
cheers
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox