Re: [PATCH v3 3/4] powerpc/mm/ptdump: debugfs handler for W+X checks at runtime
Russell Currey writes:
> Very rudimentary, just
>
> echo 1 > [debugfs]/check_wx_pages
>
> and check the kernel log. Useful for testing strict module RWX.
I was very confused that this requires the boot-time testing to be
enabled to appear in debugfs. Could you change the kconfig snippet for
PPC_DEBUG_WX to mention the runtime testing?
Kind regards,
Daniel
>
> Also fixed a typo.
>
> Signed-off-by: Russell Currey
> ---
> arch/powerpc/mm/ptdump/ptdump.c | 31 +--
> 1 file changed, 25 insertions(+), 6 deletions(-)
>
> diff --git a/arch/powerpc/mm/ptdump/ptdump.c b/arch/powerpc/mm/ptdump/ptdump.c
> index 2f9ddc29c535..0547cd9f264e 100644
> --- a/arch/powerpc/mm/ptdump/ptdump.c
> +++ b/arch/powerpc/mm/ptdump/ptdump.c
> @@ -4,7 +4,7 @@
> *
> * This traverses the kernel pagetables and dumps the
> * information about the used sections of memory to
> - * /sys/kernel/debug/kernel_pagetables.
> + * /sys/kernel/debug/kernel_page_tables.
> *
> * Derived from the arm64 implementation:
> * Copyright (c) 2014, The Linux Foundation, Laura Abbott.
> @@ -409,16 +409,35 @@ void ptdump_check_wx(void)
> else
> pr_info("Checked W+X mappings: passed, no W+X pages found\n");
> }
> +
> +static int check_wx_debugfs_set(void *data, u64 val)
> +{
> + if (val != 1ULL)
> + return -EINVAL;
> +
> + ptdump_check_wx();
> +
> + return 0;
> +}
> +
> +DEFINE_SIMPLE_ATTRIBUTE(check_wx_fops, NULL, check_wx_debugfs_set, "%llu\n");
> #endif
>
> static int ptdump_init(void)
> {
> - struct dentry *debugfs_file;
> -
> populate_markers();
> build_pgtable_complete_mask();
> - debugfs_file = debugfs_create_file("kernel_page_tables", 0400, NULL,
> - NULL, _fops);
> - return debugfs_file ? 0 : -ENOMEM;
> +
> + if (!debugfs_create_file("kernel_page_tables", 0400, NULL,
> + NULL, _fops))
> + return -ENOMEM;
> +
> +#ifdef CONFIG_PPC_DEBUG_WX
> + if (!debugfs_create_file("check_wx_pages", 0200, NULL,
> + NULL, _wx_fops))
> + return -ENOMEM;
> +#endif
> +
> + return 0;
> }
> device_initcall(ptdump_init);
> --
> 2.23.0
RE: [PATCH v4 11/11] misc: pci_endpoint_test: Add LS1088a in pci_device_id table
> -Original Message-
> From: Andrew Murray
> Sent: 2019年9月30日 22:57
> To: Xiaowei Bao
> Cc: robh...@kernel.org; mark.rutl...@arm.com; shawn...@kernel.org; Leo
> Li ; kis...@ti.com; lorenzo.pieral...@arm.com; M.h.
> Lian ; Mingkai Hu ; Roy
> Zang ; jingooh...@gmail.com;
> gustavo.pimen...@synopsys.com; linux-...@vger.kernel.org;
> linux-arm-ker...@lists.infradead.org; devicet...@vger.kernel.org;
> linux-ker...@vger.kernel.org; linuxppc-dev@lists.ozlabs.org
> Subject: Re: [PATCH v4 11/11] misc: pci_endpoint_test: Add LS1088a in
> pci_device_id table
>
> On Tue, Sep 24, 2019 at 10:18:49AM +0800, Xiaowei Bao wrote:
> > Add LS1088a in pci_device_id table so that pci-epf-test can be used
> > for testing PCIe EP in LS1088a.
> >
> > Signed-off-by: Xiaowei Bao
> > ---
> > v2:
> > - No change.
> > v3:
> > - No change.
> > v4:
> > - Use a maco to define the LS1088a device ID.
> >
> > drivers/misc/pci_endpoint_test.c | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/drivers/misc/pci_endpoint_test.c
> > b/drivers/misc/pci_endpoint_test.c
> > index 6e208a0..8c222a6 100644
> > --- a/drivers/misc/pci_endpoint_test.c
> > +++ b/drivers/misc/pci_endpoint_test.c
> > @@ -65,6 +65,7 @@
> > #define PCI_ENDPOINT_TEST_IRQ_NUMBER 0x28
> >
> > #define PCI_DEVICE_ID_TI_AM654 0xb00c
> > +#define PCI_DEVICE_ID_LS1088A 0x80c0
>
> Reviewed-by: Andrew Murray
Thanks Andrew.
>
> >
> > #define is_am654_pci_dev(pdev) \
> > ((pdev)->device == PCI_DEVICE_ID_TI_AM654) @@ -793,6 +794,7
> @@
> > static const struct pci_device_id pci_endpoint_test_tbl[] = {
> > { PCI_DEVICE(PCI_VENDOR_ID_TI, PCI_DEVICE_ID_TI_DRA74x) },
> > { PCI_DEVICE(PCI_VENDOR_ID_TI, PCI_DEVICE_ID_TI_DRA72x) },
> > { PCI_DEVICE(PCI_VENDOR_ID_FREESCALE, 0x81c0) },
> > + { PCI_DEVICE(PCI_VENDOR_ID_FREESCALE, PCI_DEVICE_ID_LS1088A) },
> > { PCI_DEVICE_DATA(SYNOPSYS, EDDA, NULL) },
> > { PCI_DEVICE(PCI_VENDOR_ID_TI, PCI_DEVICE_ID_TI_AM654),
> > .driver_data = (kernel_ulong_t)_data
> > --
> > 2.9.5
> >
[PATCH v7 8/8] powerpc/ima: update ima arch policy to check for blacklist
This patch updates the arch specific policies for PowernV systems
to add check against blacklisted binary hashes before doing the
verification.
Signed-off-by: Nayna Jain
---
arch/powerpc/kernel/ima_arch.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/kernel/ima_arch.c b/arch/powerpc/kernel/ima_arch.c
index 88bfe4a1a9a5..4fa41537b846 100644
--- a/arch/powerpc/kernel/ima_arch.c
+++ b/arch/powerpc/kernel/ima_arch.c
@@ -25,9 +25,9 @@ bool arch_ima_get_secureboot(void)
static const char *const arch_rules[] = {
"measure func=KEXEC_KERNEL_CHECK template=ima-modsig",
"measure func=MODULE_CHECK template=ima-modsig",
- "appraise func=KEXEC_KERNEL_CHECK appraise_type=imasig|modsig",
+ "appraise func=KEXEC_KERNEL_CHECK appraise_flag=check_blacklist
appraise_type=imasig|modsig",
#if !IS_ENABLED(CONFIG_MODULE_SIG_FORCE)
- "appraise func=MODULE_CHECK appraise_type=imasig|modsig",
+ "appraise func=MODULE_CHECK appraise_flag=check_blacklist
appraise_type=imasig|modsig",
#endif
NULL
};
--
2.20.1
[PATCH v7 7/8] ima: check against blacklisted hashes for files with modsig
Asymmetric private keys are used to sign multiple files. The kernel
currently support checking against the blacklisted keys. However, if the
public key is blacklisted, any file signed by the blacklisted key will
automatically fail signature verification. We might not want to blacklist
all the files signed by a particular key, but just a single file.
Blacklisting the public key is not fine enough granularity.
This patch adds support for blacklisting binaries with appended signatures,
based on the IMA policy. Defined is a new policy option
"appraise_flag=check_blacklist".
Signed-off-by: Nayna Jain
---
Documentation/ABI/testing/ima_policy | 1 +
security/integrity/ima/ima.h | 9 +++
security/integrity/ima/ima_appraise.c | 39 +++
security/integrity/ima/ima_main.c | 12 ++---
security/integrity/ima/ima_policy.c | 10 +--
security/integrity/integrity.h| 1 +
6 files changed, 66 insertions(+), 6 deletions(-)
diff --git a/Documentation/ABI/testing/ima_policy
b/Documentation/ABI/testing/ima_policy
index 29ebe9afdac4..4c97afcc0f3c 100644
--- a/Documentation/ABI/testing/ima_policy
+++ b/Documentation/ABI/testing/ima_policy
@@ -25,6 +25,7 @@ Description:
lsm:[[subj_user=] [subj_role=] [subj_type=]
[obj_user=] [obj_role=] [obj_type=]]
option: [[appraise_type=]] [template=] [permit_directio]
+ [appraise_flag=[check_blacklist]]
base: func:=
[BPRM_CHECK][MMAP_CHECK][CREDS_CHECK][FILE_CHECK][MODULE_CHECK]
[FIRMWARE_CHECK]
[KEXEC_KERNEL_CHECK] [KEXEC_INITRAMFS_CHECK]
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index ed86c1f70d7f..63e20ccc91ce 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -256,6 +256,8 @@ int ima_policy_show(struct seq_file *m, void *v);
#define IMA_APPRAISE_KEXEC 0x40
#ifdef CONFIG_IMA_APPRAISE
+int ima_check_blacklist(struct integrity_iint_cache *iint,
+ const struct modsig *modsig, int action, int pcr);
int ima_appraise_measurement(enum ima_hooks func,
struct integrity_iint_cache *iint,
struct file *file, const unsigned char *filename,
@@ -271,6 +273,13 @@ int ima_read_xattr(struct dentry *dentry,
struct evm_ima_xattr_data **xattr_value);
#else
+static inline int ima_check_blacklist(struct integrity_iint_cache *iint,
+ const struct modsig *modsig, int action,
+ int pcr)
+{
+ return 0;
+}
+
static inline int ima_appraise_measurement(enum ima_hooks func,
struct integrity_iint_cache *iint,
struct file *file,
diff --git a/security/integrity/ima/ima_appraise.c
b/security/integrity/ima/ima_appraise.c
index 136ae4e0ee92..fe34d64a684c 100644
--- a/security/integrity/ima/ima_appraise.c
+++ b/security/integrity/ima/ima_appraise.c
@@ -12,6 +12,7 @@
#include
#include
#include
+#include
#include "ima.h"
@@ -303,6 +304,44 @@ static int modsig_verify(enum ima_hooks func, const struct
modsig *modsig,
return rc;
}
+/*
+ * ima_blacklist_measurement - Checks whether the binary is blacklisted. If
+ * yes, then adds the hash of the blacklisted binary to the measurement list.
+ *
+ * Returns -EPERM if the hash is blacklisted.
+ */
+int ima_check_blacklist(struct integrity_iint_cache *iint,
+ const struct modsig *modsig, int action, int pcr)
+{
+ enum hash_algo hash_algo;
+ const u8 *digest = NULL;
+ u32 digestsize = 0;
+ u32 secid;
+ int rc = 0;
+ struct ima_template_desc *template_desc;
+
+ template_desc = lookup_template_desc("ima-buf");
+ template_desc_init_fields(template_desc->fmt, &(template_desc->fields),
+ &(template_desc->num_fields));
+
+ if (!(iint->flags & IMA_CHECK_BLACKLIST))
+ return 0;
+
+ if (iint->flags & IMA_MODSIG_ALLOWED) {
+ security_task_getsecid(current, );
+ ima_get_modsig_digest(modsig, _algo, , );
+ rc = is_binary_blacklisted(digest, digestsize);
+
+ /* Returns -EPERM on blacklisted hash found */
+ if ((rc == -EPERM) && (iint->flags & IMA_MEASURE))
+ process_buffer_measurement(digest, digestsize,
+ "blacklisted-hash", pcr,
+ template_desc);
+ }
+
+ return rc;
+}
+
/*
* ima_appraise_measurement - appraise file measurement
*
diff --git a/security/integrity/ima/ima_main.c
b/security/integrity/ima/ima_main.c
index 77115e884496..40d30ab17cbe 100644
---
[PATCH v7 6/8] certs: add wrapper function to check blacklisted binary hash
The existing is_hash_blacklisted() function returns -EKEYREJECTED
error code for both the blacklisted keys and binaries.
This patch adds a wrapper function is_binary_blacklisted() to check
against binary hashes and returns -EPERM.
Signed-off-by: Nayna Jain
---
certs/blacklist.c | 9 +
include/keys/system_keyring.h | 6 ++
2 files changed, 15 insertions(+)
diff --git a/certs/blacklist.c b/certs/blacklist.c
index ec00bf337eb6..6514f9ebc943 100644
--- a/certs/blacklist.c
+++ b/certs/blacklist.c
@@ -135,6 +135,15 @@ int is_hash_blacklisted(const u8 *hash, size_t hash_len,
const char *type)
}
EXPORT_SYMBOL_GPL(is_hash_blacklisted);
+int is_binary_blacklisted(const u8 *hash, size_t hash_len)
+{
+ if (is_hash_blacklisted(hash, hash_len, "bin") == -EKEYREJECTED)
+ return -EPERM;
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(is_binary_blacklisted);
+
/*
* Initialise the blacklist
*/
diff --git a/include/keys/system_keyring.h b/include/keys/system_keyring.h
index c1a96fdf598b..fb8b07daa9d1 100644
--- a/include/keys/system_keyring.h
+++ b/include/keys/system_keyring.h
@@ -35,12 +35,18 @@ extern int restrict_link_by_builtin_and_secondary_trusted(
extern int mark_hash_blacklisted(const char *hash);
extern int is_hash_blacklisted(const u8 *hash, size_t hash_len,
const char *type);
+extern int is_binary_blacklisted(const u8 *hash, size_t hash_len);
#else
static inline int is_hash_blacklisted(const u8 *hash, size_t hash_len,
const char *type)
{
return 0;
}
+
+static inline int is_binary_blacklisted(const u8 *hash, size_t hash_len)
+{
+ return 0;
+}
#endif
#ifdef CONFIG_IMA_BLACKLIST_KEYRING
--
2.20.1
[PATCH v7 5/8] ima: make process_buffer_measurement() generic
An additional measurement record is needed to indicate the blacklisted
binary. The record will measure the blacklisted binary hash.
This patch makes the function process_buffer_measurement() generic to be
called by the blacklisting function. It modifies the function to handle
more than just the KEXEC_CMDLINE.
Signed-off-by: Nayna Jain
---
security/integrity/ima/ima.h | 3 +++
security/integrity/ima/ima_main.c | 29 ++---
2 files changed, 17 insertions(+), 15 deletions(-)
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index 3689081aaf38..ed86c1f70d7f 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -217,6 +217,9 @@ void ima_store_measurement(struct integrity_iint_cache
*iint, struct file *file,
struct evm_ima_xattr_data *xattr_value,
int xattr_len, const struct modsig *modsig, int pcr,
struct ima_template_desc *template_desc);
+void process_buffer_measurement(const void *buf, int size,
+ const char *eventname, int pcr,
+ struct ima_template_desc *template_desc);
void ima_audit_measurement(struct integrity_iint_cache *iint,
const unsigned char *filename);
int ima_alloc_init_template(struct ima_event_data *event_data,
diff --git a/security/integrity/ima/ima_main.c
b/security/integrity/ima/ima_main.c
index 60027c643ecd..77115e884496 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -626,14 +626,14 @@ int ima_load_data(enum kernel_load_data_id id)
* @buf: pointer to the buffer that needs to be added to the log.
* @size: size of buffer(in bytes).
* @eventname: event name to be used for the buffer entry.
- * @cred: a pointer to a credentials structure for user validation.
- * @secid: the secid of the task to be validated.
+ * @pcr: pcr to extend the measurement
+ * @template_desc: template description
*
* Based on policy, the buffer is measured into the ima log.
*/
-static void process_buffer_measurement(const void *buf, int size,
- const char *eventname,
- const struct cred *cred, u32 secid)
+void process_buffer_measurement(const void *buf, int size,
+ const char *eventname, int pcr,
+ struct ima_template_desc *template_desc)
{
int ret = 0;
struct ima_template_entry *entry = NULL;
@@ -642,19 +642,11 @@ static void process_buffer_measurement(const void *buf,
int size,
.filename = eventname,
.buf = buf,
.buf_len = size};
- struct ima_template_desc *template_desc = NULL;
struct {
struct ima_digest_data hdr;
char digest[IMA_MAX_DIGEST_SIZE];
} hash = {};
int violation = 0;
- int pcr = CONFIG_IMA_MEASURE_PCR_IDX;
- int action = 0;
-
- action = ima_get_action(NULL, cred, secid, 0, KEXEC_CMDLINE, ,
- _desc);
- if (!(action & IMA_MEASURE))
- return;
iint.ima_hash =
iint.ima_hash->algo = ima_hash_algo;
@@ -686,12 +678,19 @@ static void process_buffer_measurement(const void *buf,
int size,
*/
void ima_kexec_cmdline(const void *buf, int size)
{
+ int pcr = CONFIG_IMA_MEASURE_PCR_IDX;
+ struct ima_template_desc *template_desc = NULL;
+ int action;
u32 secid;
if (buf && size != 0) {
security_task_getsecid(current, );
- process_buffer_measurement(buf, size, "kexec-cmdline",
- current_cred(), secid);
+ action = ima_get_action(NULL, current_cred(), secid, 0,
+ KEXEC_CMDLINE, , _desc);
+ if (!(action & IMA_MEASURE))
+ return;
+ process_buffer_measurement(buf, size, "kexec-cmdline", pcr,
+ template_desc);
}
}
--
2.20.1
[PATCH v7 4/8] powerpc/ima: add measurement rules to ima arch specific policy
This patch adds the measurement rules to the arch specific policies on
trusted boot enabled systems.
Signed-off-by: Nayna Jain
Reviewed-by: Mimi Zohar
---
arch/powerpc/kernel/ima_arch.c | 45 +++---
1 file changed, 42 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/kernel/ima_arch.c b/arch/powerpc/kernel/ima_arch.c
index c22d82965eb4..88bfe4a1a9a5 100644
--- a/arch/powerpc/kernel/ima_arch.c
+++ b/arch/powerpc/kernel/ima_arch.c
@@ -12,8 +12,19 @@ bool arch_ima_get_secureboot(void)
return is_powerpc_os_secureboot_enabled();
}
-/* Defines IMA appraise rules for secureboot */
+/*
+ * The "arch_rules" contains both the securebot and trustedboot rules for
adding
+ * the kexec kernel image and kernel modules file hashes to the IMA measurement
+ * list and verifying the file signatures against known good values.
+ *
+ * The "appraise_type=imasig|modsig" option allows the good signature to be
+ * stored as an xattr or as an appended signature. The "template=ima-modsig"
+ * option includes the appended signature, when available, in the IMA
+ * measurement list.
+ */
static const char *const arch_rules[] = {
+ "measure func=KEXEC_KERNEL_CHECK template=ima-modsig",
+ "measure func=MODULE_CHECK template=ima-modsig",
"appraise func=KEXEC_KERNEL_CHECK appraise_type=imasig|modsig",
#if !IS_ENABLED(CONFIG_MODULE_SIG_FORCE)
"appraise func=MODULE_CHECK appraise_type=imasig|modsig",
@@ -22,12 +33,40 @@ static const char *const arch_rules[] = {
};
/*
- * Returns the relevant IMA arch policies based on the system secureboot state.
+ * The "measure_rules" are enabled only on "trustedboot" enabled systems.
+ * These rules add the kexec kernel image and kernel modules file hashes to
+ * the IMA measurement list.
+ */
+static const char *const measure_rules[] = {
+ "measure func=KEXEC_KERNEL_CHECK",
+ "measure func=MODULE_CHECK",
+ NULL
+};
+
+/*
+ * Returns the relevant IMA arch policies based on the system secureboot
+ * and trustedboot state.
*/
const char *const *arch_get_ima_policy(void)
{
- if (is_powerpc_os_secureboot_enabled())
+ const char *const *rules;
+ int offset = 0;
+
+ for (rules = arch_rules; *rules != NULL; rules++) {
+ if (strncmp(*rules, "appraise", 8) == 0)
+ break;
+ offset++;
+ }
+
+ if (is_powerpc_os_secureboot_enabled()
+ && is_powerpc_trustedboot_enabled())
return arch_rules;
+ if (is_powerpc_os_secureboot_enabled())
+ return arch_rules + offset;
+
+ if (is_powerpc_trustedboot_enabled())
+ return measure_rules;
+
return NULL;
}
--
2.20.1
[PATCH v7 3/8] powerpc: detect the trusted boot state of the system
PowerNV systems enables the IMA measurement rules only if the
trusted boot is enabled on the system.
This patch adds the function to detect if the system has trusted
boot enabled.
Signed-off-by: Nayna Jain
---
arch/powerpc/include/asm/secure_boot.h | 6 +
arch/powerpc/kernel/secure_boot.c | 35 ++
2 files changed, 41 insertions(+)
diff --git a/arch/powerpc/include/asm/secure_boot.h
b/arch/powerpc/include/asm/secure_boot.h
index 23d2ef2f1f7b..ecd08515e301 100644
--- a/arch/powerpc/include/asm/secure_boot.h
+++ b/arch/powerpc/include/asm/secure_boot.h
@@ -12,6 +12,7 @@
bool is_powerpc_os_secureboot_enabled(void);
struct device_node *get_powerpc_os_sb_node(void);
+bool is_powerpc_trustedboot_enabled(void);
#else
@@ -25,5 +26,10 @@ static inline struct device_node
*get_powerpc_os_sb_node(void)
return NULL;
}
+static inline bool is_powerpc_os_trustedboot_enabled(void)
+{
+ return false;
+}
+
#endif
#endif
diff --git a/arch/powerpc/kernel/secure_boot.c
b/arch/powerpc/kernel/secure_boot.c
index 0488dbcab6b9..9d5ac1b39e46 100644
--- a/arch/powerpc/kernel/secure_boot.c
+++ b/arch/powerpc/kernel/secure_boot.c
@@ -7,6 +7,27 @@
#include
#include
+static const char * const fwsecureboot_compat[] = {
+ "ibm,secureboot-v1",
+ "ibm,secureboot-v2",
+ NULL,
+};
+
+static struct device_node *get_powerpc_fw_sb_node(void)
+{
+ struct device_node *node;
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(fwsecureboot_compat); ++i) {
+ node = of_find_compatible_node(NULL, NULL,
+ fwsecureboot_compat[i]);
+ if (node)
+ return node;
+ }
+
+ return NULL;
+}
+
struct device_node *get_powerpc_os_sb_node(void)
{
return of_find_compatible_node(NULL, NULL, "ibm,secvar-v1");
@@ -40,3 +61,17 @@ bool is_powerpc_os_secureboot_enabled(void)
pr_info("secureboot mode disabled\n");
return false;
}
+
+bool is_powerpc_trustedboot_enabled(void)
+{
+ struct device_node *node;
+
+ node = get_powerpc_fw_sb_node();
+ if (node && (of_find_property(node, "trusted-enabled", NULL))) {
+ pr_info("trustedboot mode enabled\n");
+ return true;
+ }
+
+ pr_info("trustedboot mode disabled\n");
+ return false;
+}
--
2.20.1
[PATCH v7 1/8] powerpc: detect the secure boot mode of the system
Secure boot on PowerNV defines different IMA policies based on the secure
boot state of the system.
This patch defines a function to detect the secure boot state of the
system.
The PPC_SECURE_BOOT config represents the base enablement of secureboot
on POWER.
Signed-off-by: Nayna Jain
---
arch/powerpc/Kconfig | 10 ++
arch/powerpc/include/asm/secure_boot.h | 29 ++
arch/powerpc/kernel/Makefile | 2 ++
arch/powerpc/kernel/secure_boot.c | 42 ++
4 files changed, 83 insertions(+)
create mode 100644 arch/powerpc/include/asm/secure_boot.h
create mode 100644 arch/powerpc/kernel/secure_boot.c
diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 3e56c9c2f16e..b4a221886fcf 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -934,6 +934,16 @@ config PPC_MEM_KEYS
If unsure, say y.
+config PPC_SECURE_BOOT
+ prompt "Enable secure boot support"
+ bool
+ depends on PPC_POWERNV
+ help
+ Systems with firmware secure boot enabled needs to define security
+ policies to extend secure boot to the OS. This config allows user
+ to enable OS secure boot on systems that have firmware support for
+ it. If in doubt say N.
+
endmenu
config ISA_DMA_API
diff --git a/arch/powerpc/include/asm/secure_boot.h
b/arch/powerpc/include/asm/secure_boot.h
new file mode 100644
index ..23d2ef2f1f7b
--- /dev/null
+++ b/arch/powerpc/include/asm/secure_boot.h
@@ -0,0 +1,29 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Secure boot definitions
+ *
+ * Copyright (C) 2019 IBM Corporation
+ * Author: Nayna Jain
+ */
+#ifndef _ASM_POWER_SECURE_BOOT_H
+#define _ASM_POWER_SECURE_BOOT_H
+
+#ifdef CONFIG_PPC_SECURE_BOOT
+
+bool is_powerpc_os_secureboot_enabled(void);
+struct device_node *get_powerpc_os_sb_node(void);
+
+#else
+
+static inline bool is_powerpc_os_secureboot_enabled(void)
+{
+ return false;
+}
+
+static inline struct device_node *get_powerpc_os_sb_node(void)
+{
+ return NULL;
+}
+
+#endif
+#endif
diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
index a7ca8fe62368..e2a54fa240ac 100644
--- a/arch/powerpc/kernel/Makefile
+++ b/arch/powerpc/kernel/Makefile
@@ -161,6 +161,8 @@ ifneq ($(CONFIG_PPC_POWERNV)$(CONFIG_PPC_SVM),)
obj-y += ucall.o
endif
+obj-$(CONFIG_PPC_SECURE_BOOT) += secure_boot.o
+
# Disable GCOV, KCOV & sanitizers in odd or sensitive code
GCOV_PROFILE_prom_init.o := n
KCOV_INSTRUMENT_prom_init.o := n
diff --git a/arch/powerpc/kernel/secure_boot.c
b/arch/powerpc/kernel/secure_boot.c
new file mode 100644
index ..0488dbcab6b9
--- /dev/null
+++ b/arch/powerpc/kernel/secure_boot.c
@@ -0,0 +1,42 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2019 IBM Corporation
+ * Author: Nayna Jain
+ */
+#include
+#include
+#include
+
+struct device_node *get_powerpc_os_sb_node(void)
+{
+ return of_find_compatible_node(NULL, NULL, "ibm,secvar-v1");
+}
+
+bool is_powerpc_os_secureboot_enabled(void)
+{
+ struct device_node *node;
+
+ node = get_powerpc_os_sb_node();
+ if (!node)
+ goto disabled;
+
+ if (!of_device_is_available(node)) {
+ pr_err("Secure variables support is in error state, fail
secure\n");
+ goto enabled;
+ }
+
+ /*
+* secureboot is enabled if os-secure-enforcing property exists,
+* else disabled.
+*/
+ if (!of_find_property(node, "os-secure-enforcing", NULL))
+ goto disabled;
+
+enabled:
+ pr_info("secureboot mode enabled\n");
+ return true;
+
+disabled:
+ pr_info("secureboot mode disabled\n");
+ return false;
+}
--
2.20.1
[PATCH v7 2/8] powerpc: add support to initialize ima policy rules
PowerNV systems uses kernel based bootloader, thus its secure boot
implementation uses kernel IMA security subsystem to verify the kernel
before kexec. Since the verification policy might differ based on the
secure boot mode of the system, the policies are defined at runtime.
This patch implements the arch-specific support to define the IMA policy
rules based on the runtime secure boot mode of the system.
This patch provides arch-specific IMA policies if PPC_SECURE_BOOT
config is enabled.
Signed-off-by: Nayna Jain
---
arch/powerpc/Kconfig | 2 ++
arch/powerpc/kernel/Makefile | 2 +-
arch/powerpc/kernel/ima_arch.c | 33 +
include/linux/ima.h| 3 ++-
4 files changed, 38 insertions(+), 2 deletions(-)
create mode 100644 arch/powerpc/kernel/ima_arch.c
diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index b4a221886fcf..deb19ec6ba3d 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -938,6 +938,8 @@ config PPC_SECURE_BOOT
prompt "Enable secure boot support"
bool
depends on PPC_POWERNV
+ depends on IMA
+ depends on IMA_ARCH_POLICY
help
Systems with firmware secure boot enabled needs to define security
policies to extend secure boot to the OS. This config allows user
diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
index e2a54fa240ac..e8eb2955b7d5 100644
--- a/arch/powerpc/kernel/Makefile
+++ b/arch/powerpc/kernel/Makefile
@@ -161,7 +161,7 @@ ifneq ($(CONFIG_PPC_POWERNV)$(CONFIG_PPC_SVM),)
obj-y += ucall.o
endif
-obj-$(CONFIG_PPC_SECURE_BOOT) += secure_boot.o
+obj-$(CONFIG_PPC_SECURE_BOOT) += secure_boot.o ima_arch.o
# Disable GCOV, KCOV & sanitizers in odd or sensitive code
GCOV_PROFILE_prom_init.o := n
diff --git a/arch/powerpc/kernel/ima_arch.c b/arch/powerpc/kernel/ima_arch.c
new file mode 100644
index ..c22d82965eb4
--- /dev/null
+++ b/arch/powerpc/kernel/ima_arch.c
@@ -0,0 +1,33 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2019 IBM Corporation
+ * Author: Nayna Jain
+ */
+
+#include
+#include
+
+bool arch_ima_get_secureboot(void)
+{
+ return is_powerpc_os_secureboot_enabled();
+}
+
+/* Defines IMA appraise rules for secureboot */
+static const char *const arch_rules[] = {
+ "appraise func=KEXEC_KERNEL_CHECK appraise_type=imasig|modsig",
+#if !IS_ENABLED(CONFIG_MODULE_SIG_FORCE)
+ "appraise func=MODULE_CHECK appraise_type=imasig|modsig",
+#endif
+ NULL
+};
+
+/*
+ * Returns the relevant IMA arch policies based on the system secureboot state.
+ */
+const char *const *arch_get_ima_policy(void)
+{
+ if (is_powerpc_os_secureboot_enabled())
+ return arch_rules;
+
+ return NULL;
+}
diff --git a/include/linux/ima.h b/include/linux/ima.h
index 1c37f17f7203..6d904754d858 100644
--- a/include/linux/ima.h
+++ b/include/linux/ima.h
@@ -29,7 +29,8 @@ extern void ima_kexec_cmdline(const void *buf, int size);
extern void ima_add_kexec_buffer(struct kimage *image);
#endif
-#if (defined(CONFIG_X86) && defined(CONFIG_EFI)) || defined(CONFIG_S390)
+#if (defined(CONFIG_X86) && defined(CONFIG_EFI)) || defined(CONFIG_S390) \
+ || defined(CONFIG_PPC_SECURE_BOOT)
extern bool arch_ima_get_secureboot(void);
extern const char * const *arch_get_ima_policy(void);
#else
--
2.20.1
[PATCH v7 0/8] powerpc: Enabling IMA arch specific secure boot policies
This patchset extends the previous version of the patchset[1] by adding the support for checking against the binary blacklisted hashes. IMA subsystem supports custom, built-in, arch-specific policies to define the files to be measured and appraised. These policies are honored based on the priority where arch-specific policies is the highest and custom is the lowest. PowerNV systems uses the linux based bootloader and kexec the Host OS. It rely on IMA for signature verification of the kernel before doing the kexec. This patchset adds support for powerpc arch specific ima policies that are defined based on system's OS secureboot and trustedboot state. The OS secureboot and trustedboot state are determined via device-tree properties. The verification needs to be done only for the binaries which are not blacklisted. The kernel currently checks against the blacklisted keys. However that results in blacklisting all the binaries that are signed by that key. In order to prevent single binary from loading, it is required to support checking against blacklisting of the binary hash. This patchset adds the support in IMA to check against blacklisted hashes for the files signed by appended signature. [1] http://patchwork.ozlabs.org/cover/1149262/ Changelog: v7: * Removes patch related to dt-bindings as per input from Rob Herring. * fixes Patch 1/8 to use new device-tree updates as per Oliver feedback to device-tree documentation in skiboot mailing list. (https://lists.ozlabs.org/pipermail/skiboot/2019-September/015329.html) * Includes feedbacks from Mimi, Thiago * moves function get_powerpc_fw_sb_node() from Patch 1 to Patch 3 * fixes Patch 2/8 to use CONFIG_MODULE_SIG_FORCE. * updates Patch description in Patch 5/8 * adds a new patch to add wrapper is_binary_blacklisted() * removes the patch that deprecated permit_directio v6: * includes feedbacks from Michael Ellerman on the patchset v5 * removed email ids from comments * add the doc for the device-tree * renames the secboot.c to secure_boot.c and secboot.h to secure_boot.h * other code specific fixes * split the patches to differentiate between secureboot and trustedboot state of the system * adds the patches to support the blacklisting of the binary hash. v5: * secureboot state is now read via device tree entry rather than OPAL secure variables * ima arch policies are updated to use policy based template for measurement rules v4: * Fixed the build issue as reported by Satheesh Rajendran. v3: * OPAL APIs in Patch 1 are updated to provide generic interface based on key/keylen. This patchset updates kernel OPAL APIs to be compatible with generic interface. * Patch 2 is cleaned up to use new OPAL APIs. * Since OPAL can support different types of backend which can vary in the variable interpretation, the Patch 2 is updated to add a check for the backend version * OPAL API now expects consumer to first check the supported backend version before calling other secvar OPAL APIs. This check is now added in patch 2. * IMA policies in Patch 3 is updated to specify appended signature and per policy template. * The patches now are free of any EFIisms. v2: * Removed Patch 1: powerpc/include: Override unneeded early ioremap functions * Updated Subject line and patch description of the Patch 1 of this series * Removed dependency of OPAL_SECVAR on EFI, CPU_BIG_ENDIAN and UCS2_STRING * Changed OPAL APIs from static to non-static. Added opal-secvar.h for the same * Removed EFI hooks from opal_secvar.c * Removed opal_secvar_get_next(), opal_secvar_enqueue() and opal_query_variable_info() function * get_powerpc_sb_mode() in secboot.c now directly calls OPAL Runtime API rather than via EFI hooks. * Fixed log messages in get_powerpc_sb_mode() function. * Added dependency for PPC_SECURE_BOOT on configs PPC64 and OPAL_SECVAR * Replaced obj-$(CONFIG_IMA) with obj-$(CONFIG_PPC_SECURE_BOOT) in arch/powerpc/kernel/Makefile *** BLURB HERE *** Nayna Jain (8): powerpc: detect the secure boot mode of the system powerpc: add support to initialize ima policy rules powerpc: detect the trusted boot state of the system powerpc/ima: add measurement rules to ima arch specific policy ima: make process_buffer_measurement() generic certs: add wrapper function to check blacklisted binary hash ima: check against blacklisted hashes for files with modsig powerpc/ima: update ima arch policy to check for blacklist Documentation/ABI/testing/ima_policy | 1 + arch/powerpc/Kconfig | 12 arch/powerpc/include/asm/secure_boot.h | 35 arch/powerpc/kernel/Makefile | 2 + arch/powerpc/kernel/ima_arch.c | 72 arch/powerpc/kernel/secure_boot.c | 77 ++ certs/blacklist.c | 9 +++ include/keys/system_keyring.h | 6 ++ include/linux/ima.h| 3 +- security/integrity/ima/ima.h | 12
Re: [PATCH] powerpc/kvm: Fix kvmppc_vcore->in_guest value in kvmhv_switch_to_host
On Friday, 4 October 2019 12:53:17 PM AEDT Jordan Niethe wrote:
> kvmhv_switch_to_host() in arch/powerpc/kvm/book3s_hv_rmhandlers.S needs
> to set kvmppc_vcore->in_guest to 0 to signal secondary CPUs to continue.
> This happens after resetting the PCR. Before commit 13c7bb3c57dc
> ("powerpc/64s: Set reserved PCR bits"), r0 would always be 0 before it
> was stored to kvmppc_vcore->in_guest. However because of this change in
> the commit:
>
> /* Reset PCR */
> ld r0, VCORE_PCR(r5)
> - cmpdi r0, 0
> + LOAD_REG_IMMEDIATE(r6, PCR_MASK)
> + cmpld r0, r6
> beq 18f
> - li r0, 0
> - mtspr SPRN_PCR, r0
> + mtspr SPRN_PCR, r6
> 18:
> /* Signal secondary CPUs to continue */
> stb r0,VCORE_IN_GUEST(r5)
Easy to understand how that was missed :-)
Reviewed-by: Alistair Popple
> We are no longer comparing r0 against 0 and loading it with 0 if it
> contains something else. Hence when we store r0 to
> kvmppc_vcore->in_guest, it might not be 0. This means that secondary
> CPUs will not be signalled to continue. Those CPUs get stuck and errors
> like the following are logged:
>
> KVM: CPU 1 seems to be stuck
> KVM: CPU 2 seems to be stuck
> KVM: CPU 3 seems to be stuck
> KVM: CPU 4 seems to be stuck
> KVM: CPU 5 seems to be stuck
> KVM: CPU 6 seems to be stuck
> KVM: CPU 7 seems to be stuck
>
> This can be reproduced with:
> $ for i in `seq 1 7` ; do chcpu -d $i ; done ;
> $ taskset -c 0 qemu-system-ppc64 -smp 8,threads=8 \
>-M pseries,accel=kvm,kvm-type=HV -m 1G -nographic -vga none \
>-kernel vmlinux -initrd initrd.cpio.xz
>
> Fix by making sure r0 is 0 before storing it to kvmppc_vcore->in_guest.
>
> Fixes: 13c7bb3c57dc ("powerpc/64s: Set reserved PCR bits")
> Reported-by: Alexey Kardashevskiy
> Signed-off-by: Jordan Niethe
> ---
> arch/powerpc/kvm/book3s_hv_rmhandlers.S | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/arch/powerpc/kvm/book3s_hv_rmhandlers.S
> b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
> index 74a9cfe84aee..faebcbb8c4db 100644
> --- a/arch/powerpc/kvm/book3s_hv_rmhandlers.S
> +++ b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
> @@ -1921,6 +1921,7 @@ END_FTR_SECTION_IFSET(CPU_FTR_ARCH_207S)
> mtspr SPRN_PCR, r6
> 18:
> /* Signal secondary CPUs to continue */
> + li r0, 0
> stb r0,VCORE_IN_GUEST(r5)
> 19: lis r8,0x7fff /* MAX_INT@h */
> mtspr SPRN_HDEC,r8
>
Re: [RESEND TRIVIAL 3/3] treewide: arch: Fix Kconfig indentation
On Fri, Oct 4, 2019 at 4:57 PM Krzysztof Kozlowski wrote:
> Adjust indentation from spaces to tab (+optional two spaces) as in
> coding style with command like:
> $ sed -e 's/^/\t/' -i */Kconfig
>
> Signed-off-by: Krzysztof Kozlowski
> arch/m68k/Kconfig.bus | 2 +-
> arch/m68k/Kconfig.debug| 16
> arch/m68k/Kconfig.machine | 8
For m68k:
Acked-by: Geert Uytterhoeven
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
[PATCH v11 06/22] powerpc: mm: Add p?d_leaf() definitions
walk_page_range() is going to be allowed to walk page tables other than
those of user space. For this it needs to know when it has reached a
'leaf' entry in the page tables. This information is provided by the
p?d_leaf() functions/macros.
For powerpc pmd_large() already exists and does what we want, so hoist
it out of the CONFIG_TRANSPARENT_HUGEPAGE condition and implement the
other levels. Macros are used to provide the generic p?d_leaf() names.
CC: Benjamin Herrenschmidt
CC: Paul Mackerras
CC: Michael Ellerman
CC: linuxppc-dev@lists.ozlabs.org
CC: kvm-...@vger.kernel.org
Signed-off-by: Steven Price
---
arch/powerpc/include/asm/book3s/64/pgtable.h | 30 ++--
1 file changed, 21 insertions(+), 9 deletions(-)
diff --git a/arch/powerpc/include/asm/book3s/64/pgtable.h
b/arch/powerpc/include/asm/book3s/64/pgtable.h
index b01624e5c467..3dd7b6f5edd0 100644
--- a/arch/powerpc/include/asm/book3s/64/pgtable.h
+++ b/arch/powerpc/include/asm/book3s/64/pgtable.h
@@ -923,6 +923,12 @@ static inline int pud_present(pud_t pud)
return !!(pud_raw(pud) & cpu_to_be64(_PAGE_PRESENT));
}
+#define pud_leaf pud_large
+static inline int pud_large(pud_t pud)
+{
+ return !!(pud_raw(pud) & cpu_to_be64(_PAGE_PTE));
+}
+
extern struct page *pud_page(pud_t pud);
extern struct page *pmd_page(pmd_t pmd);
static inline pte_t pud_pte(pud_t pud)
@@ -966,6 +972,12 @@ static inline int pgd_present(pgd_t pgd)
return !!(pgd_raw(pgd) & cpu_to_be64(_PAGE_PRESENT));
}
+#define pgd_leaf pgd_large
+static inline int pgd_large(pgd_t pgd)
+{
+ return !!(pgd_raw(pgd) & cpu_to_be64(_PAGE_PTE));
+}
+
static inline pte_t pgd_pte(pgd_t pgd)
{
return __pte_raw(pgd_raw(pgd));
@@ -1133,6 +1145,15 @@ static inline bool pmd_access_permitted(pmd_t pmd, bool
write)
return pte_access_permitted(pmd_pte(pmd), write);
}
+#define pmd_leaf pmd_large
+/*
+ * returns true for pmd migration entries, THP, devmap, hugetlb
+ */
+static inline int pmd_large(pmd_t pmd)
+{
+ return !!(pmd_raw(pmd) & cpu_to_be64(_PAGE_PTE));
+}
+
#ifdef CONFIG_TRANSPARENT_HUGEPAGE
extern pmd_t pfn_pmd(unsigned long pfn, pgprot_t pgprot);
extern pmd_t mk_pmd(struct page *page, pgprot_t pgprot);
@@ -1159,15 +1180,6 @@ pmd_hugepage_update(struct mm_struct *mm, unsigned long
addr, pmd_t *pmdp,
return hash__pmd_hugepage_update(mm, addr, pmdp, clr, set);
}
-/*
- * returns true for pmd migration entries, THP, devmap, hugetlb
- * But compile time dependent on THP config
- */
-static inline int pmd_large(pmd_t pmd)
-{
- return !!(pmd_raw(pmd) & cpu_to_be64(_PAGE_PTE));
-}
-
static inline pmd_t pmd_mknotpresent(pmd_t pmd)
{
return __pmd(pmd_val(pmd) & ~_PAGE_PRESENT);
--
2.20.1
Re: [PATCH V4 2/2] mm/pgtable/debug: Add test validating architecture page table helpers
On Mon, Oct 07, 2019 at 03:51:58PM +0200, Ingo Molnar wrote:
>
> * Kirill A. Shutemov wrote:
>
> > On Mon, Oct 07, 2019 at 03:06:17PM +0200, Ingo Molnar wrote:
> > >
> > > * Anshuman Khandual wrote:
> > >
> > > > This adds a test module which will validate architecture page table
> > > > helpers
> > > > and accessors regarding compliance with generic MM semantics
> > > > expectations.
> > > > This will help various architectures in validating changes to the
> > > > existing
> > > > page table helpers or addition of new ones.
> > > >
> > > > Test page table and memory pages creating it's entries at various level
> > > > are
> > > > all allocated from system memory with required alignments. If memory
> > > > pages
> > > > with required size and alignment could not be allocated, then all
> > > > depending
> > > > individual tests are skipped.
> > >
> > > > diff --git a/arch/x86/include/asm/pgtable_64_types.h
> > > > b/arch/x86/include/asm/pgtable_64_types.h
> > > > index 52e5f5f2240d..b882792a3999 100644
> > > > --- a/arch/x86/include/asm/pgtable_64_types.h
> > > > +++ b/arch/x86/include/asm/pgtable_64_types.h
> > > > @@ -40,6 +40,8 @@ static inline bool pgtable_l5_enabled(void)
> > > > #define pgtable_l5_enabled() 0
> > > > #endif /* CONFIG_X86_5LEVEL */
> > > >
> > > > +#define mm_p4d_folded(mm) (!pgtable_l5_enabled())
> > > > +
> > > > extern unsigned int pgdir_shift;
> > > > extern unsigned int ptrs_per_p4d;
> > >
> > > Any deep reason this has to be a macro instead of proper C?
> >
> > It's a way to override the generic mm_p4d_folded(). It can be rewritten
> > as inline function + define. Something like:
> >
> > #define mm_p4d_folded mm_p4d_folded
> > static inline bool mm_p4d_folded(struct mm_struct *mm)
> > {
> > return !pgtable_l5_enabled();
> > }
> >
> > But I don't see much reason to be more verbose here than needed.
>
> C type checking? Documentation? Yeah, I know it's just a one-liner, but
> the principle of the death by a thousand cuts applies here.
Okay, if you think it worth it. Anshuman, could you fix it up for the next
submission?
> BTW., any reason this must be in the low level pgtable_64_types.h type
> header, instead of one of the API level header files?
I defined it next pgtable_l5_enabled(). What is more appropriate place to
you? pgtable_64.h? Yeah, it makes sense.
--
Kirill A. Shutemov
Re: [PATCH V4 2/2] mm/pgtable/debug: Add test validating architecture page table helpers
* Kirill A. Shutemov wrote:
> On Mon, Oct 07, 2019 at 03:06:17PM +0200, Ingo Molnar wrote:
> >
> > * Anshuman Khandual wrote:
> >
> > > This adds a test module which will validate architecture page table
> > > helpers
> > > and accessors regarding compliance with generic MM semantics expectations.
> > > This will help various architectures in validating changes to the existing
> > > page table helpers or addition of new ones.
> > >
> > > Test page table and memory pages creating it's entries at various level
> > > are
> > > all allocated from system memory with required alignments. If memory pages
> > > with required size and alignment could not be allocated, then all
> > > depending
> > > individual tests are skipped.
> >
> > > diff --git a/arch/x86/include/asm/pgtable_64_types.h
> > > b/arch/x86/include/asm/pgtable_64_types.h
> > > index 52e5f5f2240d..b882792a3999 100644
> > > --- a/arch/x86/include/asm/pgtable_64_types.h
> > > +++ b/arch/x86/include/asm/pgtable_64_types.h
> > > @@ -40,6 +40,8 @@ static inline bool pgtable_l5_enabled(void)
> > > #define pgtable_l5_enabled() 0
> > > #endif /* CONFIG_X86_5LEVEL */
> > >
> > > +#define mm_p4d_folded(mm) (!pgtable_l5_enabled())
> > > +
> > > extern unsigned int pgdir_shift;
> > > extern unsigned int ptrs_per_p4d;
> >
> > Any deep reason this has to be a macro instead of proper C?
>
> It's a way to override the generic mm_p4d_folded(). It can be rewritten
> as inline function + define. Something like:
>
> #define mm_p4d_folded mm_p4d_folded
> static inline bool mm_p4d_folded(struct mm_struct *mm)
> {
> return !pgtable_l5_enabled();
> }
>
> But I don't see much reason to be more verbose here than needed.
C type checking? Documentation? Yeah, I know it's just a one-liner, but
the principle of the death by a thousand cuts applies here.
BTW., any reason this must be in the low level pgtable_64_types.h type
header, instead of one of the API level header files?
Thanks,
Ingo
Re: [PATCH V4 2/2] mm/pgtable/debug: Add test validating architecture page table helpers
On Mon, Oct 07, 2019 at 03:06:17PM +0200, Ingo Molnar wrote:
>
> * Anshuman Khandual wrote:
>
> > This adds a test module which will validate architecture page table helpers
> > and accessors regarding compliance with generic MM semantics expectations.
> > This will help various architectures in validating changes to the existing
> > page table helpers or addition of new ones.
> >
> > Test page table and memory pages creating it's entries at various level are
> > all allocated from system memory with required alignments. If memory pages
> > with required size and alignment could not be allocated, then all depending
> > individual tests are skipped.
>
> > diff --git a/arch/x86/include/asm/pgtable_64_types.h
> > b/arch/x86/include/asm/pgtable_64_types.h
> > index 52e5f5f2240d..b882792a3999 100644
> > --- a/arch/x86/include/asm/pgtable_64_types.h
> > +++ b/arch/x86/include/asm/pgtable_64_types.h
> > @@ -40,6 +40,8 @@ static inline bool pgtable_l5_enabled(void)
> > #define pgtable_l5_enabled() 0
> > #endif /* CONFIG_X86_5LEVEL */
> >
> > +#define mm_p4d_folded(mm) (!pgtable_l5_enabled())
> > +
> > extern unsigned int pgdir_shift;
> > extern unsigned int ptrs_per_p4d;
>
> Any deep reason this has to be a macro instead of proper C?
It's a way to override the generic mm_p4d_folded(). It can be rewritten
as inline function + define. Something like:
#define mm_p4d_folded mm_p4d_folded
static inline bool mm_p4d_folded(struct mm_struct *mm)
{
return !pgtable_l5_enabled();
}
But I don't see much reason to be more verbose here than needed.
--
Kirill A. Shutemov
Re: [PATCH V4 2/2] mm/pgtable/debug: Add test validating architecture page table helpers
* Anshuman Khandual wrote: > This adds a test module which will validate architecture page table helpers > and accessors regarding compliance with generic MM semantics expectations. > This will help various architectures in validating changes to the existing > page table helpers or addition of new ones. > > Test page table and memory pages creating it's entries at various level are > all allocated from system memory with required alignments. If memory pages > with required size and alignment could not be allocated, then all depending > individual tests are skipped. > diff --git a/arch/x86/include/asm/pgtable_64_types.h > b/arch/x86/include/asm/pgtable_64_types.h > index 52e5f5f2240d..b882792a3999 100644 > --- a/arch/x86/include/asm/pgtable_64_types.h > +++ b/arch/x86/include/asm/pgtable_64_types.h > @@ -40,6 +40,8 @@ static inline bool pgtable_l5_enabled(void) > #define pgtable_l5_enabled() 0 > #endif /* CONFIG_X86_5LEVEL */ > > +#define mm_p4d_folded(mm) (!pgtable_l5_enabled()) > + > extern unsigned int pgdir_shift; > extern unsigned int ptrs_per_p4d; Any deep reason this has to be a macro instead of proper C? > diff --git a/mm/Kconfig.debug b/mm/Kconfig.debug > index 327b3ebf23bf..683131b1ee7d 100644 > --- a/mm/Kconfig.debug > +++ b/mm/Kconfig.debug > @@ -117,3 +117,18 @@ config DEBUG_RODATA_TEST > depends on STRICT_KERNEL_RWX > ---help--- >This option enables a testcase for the setting rodata read-only. > + > +config DEBUG_ARCH_PGTABLE_TEST > + bool "Test arch page table helpers for semantics compliance" > + depends on MMU > + depends on DEBUG_KERNEL > + depends on !(ARM || IA64) Please add a proper enabling switch for architectures to opt in. Please also add it to Documentation/features/list-arch.sh so that it's listed as a 'TODO' entry on architectures where the tests are not enabled yet. > + help > + This options provides a kernel module which can be used to test > + architecture page table helper functions on various platform in > + verifying if they comply with expected generic MM semantics. This > + will help architectures code in making sure that any changes or > + new additions of these helpers will still conform to generic MM > + expected semantics. Typos and grammar fixed: help This option provides a kernel module which can be used to test architecture page table helper functions on various platforms in verifying if they comply with expected generic MM semantics. This will help architecture code in making sure that any changes or new additions of these helpers still conform to expected semantics of the generic MM. Also, more fundamentally: isn't a kernel module too late for such a debug check, should something break due to a core MM change? Have these debug checks caught any bugs or inconsistencies before? Why not call this as some earlier MM debug check, after enabling paging but before executing user-space binaries or relying on complex MM ops within the kernel, called at a stage when those primitives are all expected to work fine? It seems to me that arch_pgtable_tests_init) won't even context-switch normally, right? Finally, instead of inventing yet another randomly named .config debug switch, please fit it into the regular MM debug options which go along the CONFIG_DEBUG_VM* naming scheme. Might even make sense to enable these new debug checks by default if CONFIG_DEBUG_VM=y, that way we'll get a *lot* more debug coverage than some random module somewhere that few people will know about, let alone run. Thanks, Ingo
Applied "ASoC: fsl_mqs: Fix error handling in probe" to the asoc tree
The patch
ASoC: fsl_mqs: Fix error handling in probe
has been applied to the asoc tree at
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-5.5
All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying
to this mail.
Thanks,
Mark
>From a9d273671440c439c4f236123c59dd839c1a0eb7 Mon Sep 17 00:00:00 2001
From: Dan Carpenter
Date: Fri, 4 Oct 2019 13:22:09 +0300
Subject: [PATCH] ASoC: fsl_mqs: Fix error handling in probe
There are several problems in the error handling in fsl_mqs_probe().
1) "ret" isn't initialized on some paths. GCC has a feature which
warns about uninitialized variables but the code initializes "ret"
to zero at the start of the function so the checking is turned off.
2) "gpr_np" is a pointer so initializing it to zero is confusing and
generates a Sparse warning.
3) of_parse_phandle() doesn't return error pointers on error, it returns
NULL.
4) If devm_snd_soc_register_component() fails then the function should
free the "gpr_np".
Fixes: 9e28f6532c61 ("ASoC: fsl_mqs: Add MQS component driver")
Signed-off-by: Dan Carpenter
Link: https://lore.kernel.org/r/20191004102208.GB823@mwanda
Signed-off-by: Mark Brown
---
sound/soc/fsl/fsl_mqs.c | 27 +++
1 file changed, 15 insertions(+), 12 deletions(-)
diff --git a/sound/soc/fsl/fsl_mqs.c b/sound/soc/fsl/fsl_mqs.c
index 7b9cab3a62e7..f7fc44e8fb27 100644
--- a/sound/soc/fsl/fsl_mqs.c
+++ b/sound/soc/fsl/fsl_mqs.c
@@ -178,10 +178,10 @@ static const struct regmap_config fsl_mqs_regmap_config =
{
static int fsl_mqs_probe(struct platform_device *pdev)
{
struct device_node *np = pdev->dev.of_node;
- struct device_node *gpr_np = 0;
+ struct device_node *gpr_np = NULL;
struct fsl_mqs *mqs_priv;
void __iomem *regs;
- int ret = 0;
+ int ret;
mqs_priv = devm_kzalloc(>dev, sizeof(*mqs_priv), GFP_KERNEL);
if (!mqs_priv)
@@ -198,17 +198,16 @@ static int fsl_mqs_probe(struct platform_device *pdev)
if (mqs_priv->use_gpr) {
gpr_np = of_parse_phandle(np, "gpr", 0);
- if (IS_ERR(gpr_np)) {
+ if (!gpr_np) {
dev_err(>dev, "failed to get gpr node by
phandle\n");
- ret = PTR_ERR(gpr_np);
- goto out;
+ return -EINVAL;
}
mqs_priv->regmap = syscon_node_to_regmap(gpr_np);
if (IS_ERR(mqs_priv->regmap)) {
dev_err(>dev, "failed to get gpr regmap\n");
ret = PTR_ERR(mqs_priv->regmap);
- goto out;
+ goto err_free_gpr_np;
}
} else {
regs = devm_platform_ioremap_resource(pdev, 0);
@@ -229,7 +228,7 @@ static int fsl_mqs_probe(struct platform_device *pdev)
if (IS_ERR(mqs_priv->ipg)) {
dev_err(>dev, "failed to get the clock: %ld\n",
PTR_ERR(mqs_priv->ipg));
- goto out;
+ return PTR_ERR(mqs_priv->ipg);
}
}
@@ -237,17 +236,21 @@ static int fsl_mqs_probe(struct platform_device *pdev)
if (IS_ERR(mqs_priv->mclk)) {
dev_err(>dev, "failed to get the clock: %ld\n",
PTR_ERR(mqs_priv->mclk));
- goto out;
+ ret = PTR_ERR(mqs_priv->mclk);
+ goto err_free_gpr_np;
}
dev_set_drvdata(>dev, mqs_priv);
pm_runtime_enable(>dev);
- return devm_snd_soc_register_component(>dev, _codec_fsl_mqs,
+ ret = devm_snd_soc_register_component(>dev, _codec_fsl_mqs,
_mqs_dai, 1);
-out:
- if (!IS_ERR(gpr_np))
- of_node_put(gpr_np);
+ if (ret)
+ goto err_free_gpr_np;
+ return 0;
+
+err_free_gpr_np:
+ of_node_put(gpr_np);
return ret;
}
--
2.20.1
Applied "ASoC: fsl_mqs: remove set but not used variable 'bclk'" to the asoc tree
The patch ASoC: fsl_mqs: remove set but not used variable 'bclk' has been applied to the asoc tree at https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-5.5 All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted. You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed. If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced. Please add any relevant lists and maintainers to the CCs when replying to this mail. Thanks, Mark >From e9e8fc9ed63e7e0fb30f8612f628924fbd868467 Mon Sep 17 00:00:00 2001 From: YueHaibing Date: Sun, 6 Oct 2019 18:55:22 +0800 Subject: [PATCH] ASoC: fsl_mqs: remove set but not used variable 'bclk' Fixes gcc '-Wunused-but-set-variable' warning: sound/soc/fsl/fsl_mqs.c: In function fsl_mqs_hw_params: sound/soc/fsl/fsl_mqs.c:54:6: warning: variable bclk set but not used [-Wunused-but-set-variable] It is never used, so can be removed. Reported-by: Hulk Robot Signed-off-by: YueHaibing Link: https://lore.kernel.org/r/20191006105522.58560-1-yuehaib...@huawei.com Signed-off-by: Mark Brown --- sound/soc/fsl/fsl_mqs.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/sound/soc/fsl/fsl_mqs.c b/sound/soc/fsl/fsl_mqs.c index c1619a553514..7b9cab3a62e7 100644 --- a/sound/soc/fsl/fsl_mqs.c +++ b/sound/soc/fsl/fsl_mqs.c @@ -51,10 +51,9 @@ static int fsl_mqs_hw_params(struct snd_pcm_substream *substream, struct fsl_mqs *mqs_priv = snd_soc_component_get_drvdata(component); unsigned long mclk_rate; int div, res; - int bclk, lrclk; + int lrclk; mclk_rate = clk_get_rate(mqs_priv->mclk); - bclk = snd_soc_params_to_bclk(params); lrclk = params_rate(params); /* -- 2.20.1
Re: [PATCH v8 1/5] kasan: support backing vmalloc space with real shadow memory
> diff --git a/mm/vmalloc.c b/mm/vmalloc.c
> index a3c70e275f4e..9fb7a16f42ae 100644
> --- a/mm/vmalloc.c
> +++ b/mm/vmalloc.c
> @@ -690,8 +690,19 @@ merge_or_add_vmap_area(struct vmap_area *va,
> struct list_head *next;
> struct rb_node **link;
> struct rb_node *parent;
> + unsigned long orig_start, orig_end;
> bool merged = false;
>
> + /*
> + * To manage KASAN vmalloc memory usage, we use this opportunity to
> + * clean up the shadow memory allocated to back this allocation.
> + * Because a vmalloc shadow page covers several pages, the start or end
> + * of an allocation might not align with a shadow page. Use the merging
> + * opportunities to try to extend the region we can release.
> + */
> + orig_start = va->va_start;
> + orig_end = va->va_end;
> +
> /*
>* Find a place in the tree where VA potentially will be
>* inserted, unless it is merged with its sibling/siblings.
> @@ -741,6 +752,10 @@ merge_or_add_vmap_area(struct vmap_area *va,
> if (sibling->va_end == va->va_start) {
> sibling->va_end = va->va_end;
>
> + kasan_release_vmalloc(orig_start, orig_end,
> + sibling->va_start,
> + sibling->va_end);
> +
> /* Check and update the tree if needed. */
> augment_tree_propagate_from(sibling);
>
> @@ -754,6 +769,8 @@ merge_or_add_vmap_area(struct vmap_area *va,
> }
>
> insert:
> + kasan_release_vmalloc(orig_start, orig_end, va->va_start, va->va_end);
> +
> if (!merged) {
> link_va(va, root, parent, link, head);
> augment_tree_propagate_from(va);
Hello, Daniel.
Looking at it one more, i think above part of code is a bit wrong
and should be separated from merge_or_add_vmap_area() logic. The
reason is to keep it simple and do only what it is supposed to do:
merging or adding.
Also the kasan_release_vmalloc() gets called twice there and looks like
a duplication. Apart of that, merge_or_add_vmap_area() can be called via
recovery path when vmap/vmaps is/are not even setup. See percpu
allocator.
I guess your part could be moved directly to the __purge_vmap_area_lazy()
where all vmaps are lazily freed. To do so, we also need to modify
merge_or_add_vmap_area() to return merged area:
diff --git a/mm/vmalloc.c b/mm/vmalloc.c
index e92ff5f7dd8b..fecde4312d68 100644
--- a/mm/vmalloc.c
+++ b/mm/vmalloc.c
@@ -683,7 +683,7 @@ insert_vmap_area_augment(struct vmap_area *va,
* free area is inserted. If VA has been merged, it is
* freed.
*/
-static __always_inline void
+static __always_inline struct vmap_area *
merge_or_add_vmap_area(struct vmap_area *va,
struct rb_root *root, struct list_head *head)
{
@@ -750,7 +750,10 @@ merge_or_add_vmap_area(struct vmap_area *va,
/* Free vmap_area object. */
kmem_cache_free(vmap_area_cachep, va);
- return;
+
+ /* Point to the new merged area. */
+ va = sibling;
+ merged = true;
}
}
@@ -759,6 +762,8 @@ merge_or_add_vmap_area(struct vmap_area *va,
link_va(va, root, parent, link, head);
augment_tree_propagate_from(va);
}
+
+ return va;
}
static __always_inline bool
@@ -1172,7 +1177,7 @@ static void __free_vmap_area(struct vmap_area *va)
/*
* Merge VA with its neighbors, otherwise just add it.
*/
- merge_or_add_vmap_area(va,
+ (void) merge_or_add_vmap_area(va,
_vmap_area_root, _vmap_area_list);
}
@@ -1279,15 +1284,20 @@ static bool __purge_vmap_area_lazy(unsigned long start,
unsigned long end)
spin_lock(_area_lock);
llist_for_each_entry_safe(va, n_va, valist, purge_list) {
unsigned long nr = (va->va_end - va->va_start) >> PAGE_SHIFT;
+ unsigned long orig_start = va->va_start;
+ unsigned long orig_end = va->va_end;
/*
* Finally insert or merge lazily-freed area. It is
* detached and there is no need to "unlink" it from
* anything.
*/
- merge_or_add_vmap_area(va,
+ va = merge_or_add_vmap_area(va,
_vmap_area_root, _vmap_area_list);
+ kasan_release_vmalloc(orig_start,
+ orig_end, va->va_start, va->va_end);
+
atomic_long_sub(nr, _lazy_nr);
if (atomic_long_read(_lazy_nr) < resched_threshold)
--
Vlad Rezki
Re: [PATCH v4 0/5] Powerpc/Watchpoint: Few important fixes
On 9/25/19 9:36 AM, Ravi Bangoria wrote: v3: https://lists.ozlabs.org/pipermail/linuxppc-dev/2019-July/193339.html v3->v4: - Instead of considering exception as extraneous when dar is outside of user specified range, analyse the instruction and check for overlap between user specified range and actual load/store range. - Add selftest for the same in perf-hwbreak.c - Make ptrace-hwbreak.c selftest more strict by checking address in check_success. - Support for 8xx in ptrace-hwbreak.c selftest (Build tested only) - Rebase to powerpc/next @Christope, Can you please check Patch 5. I've just build-tested it with ep88xc_defconfig. @mpe, @mikey, Any feedback? @Christophe, Is patch5 works for you on 8xx? Thanks, Ravi
