* [PATCH] powerpc/rtas: Fix hang in race against concurrent cpu offline
From: Juliet Kim @ 2019-06-24 23:48 UTC (permalink / raw)
To: linuxppc-dev; +Cc: mmc, mwb, nathanl
From 1bd2bf7146876e099eafb292668f9a12dc22f526 Mon Sep 17 00:00:00 2001
From: Juliet Kim <julietk@linux.vnet.ibm.com>
Date: Mon, 24 Jun 2019 17:17:46 -0400
Subject: [PATCH 1/1] powerpc/rtas: Fix hang in race against concurrent cpu
offline
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
The commit
(“powerpc/rtas: Fix a potential race between CPU-Offline & Migration)
attempted to fix a hang in Live Partition Mobility(LPM) by abandoning
the LPM attempt if a race between LPM and concurrent CPU offline was
detected.
However, that fix failed to notify Hypervisor that the LPM attempted
had been abandoned which results in a system hang.
Fix this by sending a signal PHYP to cancel the migration, so that PHYP
can stop waiting, and clean up the migration.
Fixes: dfd718a2ed1f ("powerpc/rtas: Fix a potential race between CPU-Offline & Migration")
Signed-off-by: Juliet Kim <julietk@linux.vnet.ibm.com>
---
This is an alternate solution to the one Nathan proposed in:
https://lists.ozlabs.org/pipermail/linuxppc-dev/2019-June/192414.html
---
arch/powerpc/include/asm/hvcall.h | 7 +++++++
arch/powerpc/kernel/rtas.c | 8 ++++++++
2 files changed, 15 insertions(+)
diff --git a/arch/powerpc/include/asm/hvcall.h b/arch/powerpc/include/asm/hvcall.h
index 463c63a..29ca285 100644
--- a/arch/powerpc/include/asm/hvcall.h
+++ b/arch/powerpc/include/asm/hvcall.h
@@ -261,6 +261,7 @@
#define H_ADD_CONN 0x284
#define H_DEL_CONN 0x288
#define H_JOIN 0x298
+#define H_VASI_SIGNAL 0x2A0
#define H_VASI_STATE 0x2A4
#define H_VIOCTL 0x2A8
#define H_ENABLE_CRQ 0x2B0
@@ -348,6 +349,12 @@
#define H_SIGNAL_SYS_RESET_ALL_OTHERS -2
/* >= 0 values are CPU number */
+/* Values for argument to H_VASI_SIGNAL */
+#define H_SIGNAL_CANCEL_MIG 0x01
+
+/* Values for 2nd argument to H_VASI_SIGNAL */
+#define H_CPU_OFFLINE_DETECTED 0x0000000006000004
+
/* H_GET_CPU_CHARACTERISTICS return values */
#define H_CPU_CHAR_SPEC_BAR_ORI31 (1ull << 63) // IBM bit 0
#define H_CPU_CHAR_BCCTRL_SERIALISED (1ull << 62) // IBM bit 1
diff --git a/arch/powerpc/kernel/rtas.c b/arch/powerpc/kernel/rtas.c
index b824f4c..f9002b7 100644
--- a/arch/powerpc/kernel/rtas.c
+++ b/arch/powerpc/kernel/rtas.c
@@ -981,6 +981,14 @@ int rtas_ibm_suspend_me(u64 handle)
/* Check if we raced with a CPU-Offline Operation */
if (unlikely(!cpumask_equal(cpu_present_mask, cpu_online_mask))) {
+
+ /* We uses CANCEL, not ABORT to gracefully cancel migration */
+ rc = plpar_hcall_norets(H_VASI_SIGNAL, handle,
+ H_SIGNAL_CANCEL_MIG, H_CPU_OFFLINE_DETECTED);
+
+ if (rc != H_SUCCESS)
+ pr_err("%s: vasi_signal failed %ld\n", __func__, rc);
+
pr_err("%s: Raced against a concurrent CPU-Offline\n",
__func__);
atomic_set(&data.error, -EBUSY);
--
1.8.3.1
^ permalink raw reply related
* Re: [PATCH v4 1/4] lib/scatterlist: Fix mapping iterator when sg->offset is greater than PAGE_SIZE
From: Imre Deak @ 2019-06-24 17:35 UTC (permalink / raw)
To: Herbert Xu
Cc: horia.geanta, linux-kernel, linux-crypto, linuxppc-dev,
David S. Miller
In-Reply-To: <20190620060221.q4pbsqzsza3pxs42@gondor.apana.org.au>
Hi,
On Thu, Jun 20, 2019 at 02:02:21PM +0800, Herbert Xu wrote:
> On Mon, Jun 17, 2019 at 09:15:02PM +0000, Christophe Leroy wrote:
> > All mapping iterator logic is based on the assumption that sg->offset
> > is always lower than PAGE_SIZE.
> >
> > But there are situations where sg->offset is such that the SG item
> > is on the second page.
could you explain how sg->offset becomes >= PAGE_SIZE?
--Imre
> > In that case sg_copy_to_buffer() fails
> > properly copying the data into the buffer. One of the reason is
> > that the data will be outside the kmapped area used to access that
> > data.
> >
> > This patch fixes the issue by adjusting the mapping iterator
> > offset and pgoffset fields such that offset is always lower than
> > PAGE_SIZE.
> >
> > Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
> > Fixes: 4225fc8555a9 ("lib/scatterlist: use page iterator in the mapping iterator")
> > Cc: stable@vger.kernel.org
> > ---
> > lib/scatterlist.c | 9 +++++++--
> > 1 file changed, 7 insertions(+), 2 deletions(-)
>
> Good catch.
>
> > @@ -686,7 +686,12 @@ static bool sg_miter_get_next_page(struct sg_mapping_iter *miter)
> > sg = miter->piter.sg;
> > pgoffset = miter->piter.sg_pgoffset;
> >
> > - miter->__offset = pgoffset ? 0 : sg->offset;
> > + offset = pgoffset ? 0 : sg->offset;
> > + while (offset >= PAGE_SIZE) {
> > + miter->piter.sg_pgoffset = ++pgoffset;
> > + offset -= PAGE_SIZE;
> > + }
>
> How about
>
> miter->piter.sg_pgoffset += offset >> PAGE_SHIFT;
> offset &= PAGE_SIZE - 1;
>
> Thanks,
> --
> Email: Herbert Xu <herbert@gondor.apana.org.au>
> Home Page: http://gondor.apana.org.au/~herbert/
> PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [PATCH 5/5] asm-generic: remove ptrace.h
From: Paul Burton @ 2019-06-24 20:33 UTC (permalink / raw)
To: Christoph Hellwig
Cc: linux-arch@vger.kernel.org, Arnd Bergmann,
linux-sh@vger.kernel.org, linux-kernel@vger.kernel.org,
x86@kernel.org, Oleg Nesterov, linux-mips@vger.kernel.org,
linuxppc-dev@lists.ozlabs.org,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <20190624054728.30966-6-hch@lst.de>
On Mon, Jun 24, 2019 at 07:47:28AM +0200, Christoph Hellwig wrote:
> No one is using this header anymore.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> Acked-by: Arnd Bergmann <arnd@arndb.de>
> Acked-by: Oleg Nesterov <oleg@redhat.com>
> ---
> MAINTAINERS | 1 -
> arch/mips/include/asm/ptrace.h | 5 ---
> include/asm-generic/ptrace.h | 73 ----------------------------------
> 3 files changed, 79 deletions(-)
> delete mode 100644 include/asm-generic/ptrace.h
FWIW:
Acked-by: Paul Burton <paul.burton@mips.com>
Thanks,
Paul
> diff --git a/arch/mips/include/asm/ptrace.h b/arch/mips/include/asm/ptrace.h
> index b6578611dddb..1e76774b36dd 100644
> --- a/arch/mips/include/asm/ptrace.h
> +++ b/arch/mips/include/asm/ptrace.h
> @@ -56,11 +56,6 @@ static inline unsigned long kernel_stack_pointer(struct pt_regs *regs)
> return regs->regs[31];
> }
>
> -/*
> - * Don't use asm-generic/ptrace.h it defines FP accessors that don't make
> - * sense on MIPS. We rather want an error if they get invoked.
> - */
> -
> static inline void instruction_pointer_set(struct pt_regs *regs,
> unsigned long val)
> {
^ permalink raw reply
* Re: [PATCH v11 02/13] PKCS#7: Refactor verify_pkcs7_signature()
From: Thiago Jung Bauermann @ 2019-06-24 19:56 UTC (permalink / raw)
To: David Howells
Cc: Herbert Xu, linux-doc, Dmitry Kasatkin, David S. Miller,
Jonathan Corbet, linux-kernel, Mimi Zohar, James Morris,
AKASHI, Takahiro, linux-security-module, keyrings, linux-crypto,
Jessica Yu, linux-integrity, linuxppc-dev, David Woodhouse,
Serge E. Hallyn
In-Reply-To: <20190611062817.18412-3-bauerman@linux.ibm.com>
Hello David,
AFAIK Mimi is happy with this patch set, but I still need acks from
maintainers of other subsystems that my changes touch before she can
accept it.
Are this patch and the next one ("PKCS#7: Introduce pkcs7_get_digest()")
OK from your PoV?
--
Thiago Jung Bauermann
IBM Linux Technology Center
Thiago Jung Bauermann <bauerman@linux.ibm.com> writes:
> IMA will need to verify a PKCS#7 signature which has already been parsed.
> For this reason, factor out the code which does that from
> verify_pkcs7_signature() into a new function which takes a struct
> pkcs7_message instead of a data buffer.
>
> Signed-off-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
> Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
> Cc: David Howells <dhowells@redhat.com>
> Cc: David Woodhouse <dwmw2@infradead.org>
> Cc: Herbert Xu <herbert@gondor.apana.org.au>
> Cc: "David S. Miller" <davem@davemloft.net>
> ---
> certs/system_keyring.c | 61 ++++++++++++++++++++++++++----------
> include/linux/verification.h | 10 ++++++
> 2 files changed, 55 insertions(+), 16 deletions(-)
>
> diff --git a/certs/system_keyring.c b/certs/system_keyring.c
> index c05c29ae4d5d..4ba82e52e4b4 100644
> --- a/certs/system_keyring.c
> +++ b/certs/system_keyring.c
> @@ -194,33 +194,27 @@ late_initcall(load_system_certificate_list);
> #ifdef CONFIG_SYSTEM_DATA_VERIFICATION
>
> /**
> - * verify_pkcs7_signature - Verify a PKCS#7-based signature on system data.
> + * verify_pkcs7_message_sig - Verify a PKCS#7-based signature on system data.
> * @data: The data to be verified (NULL if expecting internal data).
> * @len: Size of @data.
> - * @raw_pkcs7: The PKCS#7 message that is the signature.
> - * @pkcs7_len: The size of @raw_pkcs7.
> + * @pkcs7: The PKCS#7 message that is the signature.
> * @trusted_keys: Trusted keys to use (NULL for builtin trusted keys only,
> * (void *)1UL for all trusted keys).
> * @usage: The use to which the key is being put.
> * @view_content: Callback to gain access to content.
> * @ctx: Context for callback.
> */
> -int verify_pkcs7_signature(const void *data, size_t len,
> - const void *raw_pkcs7, size_t pkcs7_len,
> - struct key *trusted_keys,
> - enum key_being_used_for usage,
> - int (*view_content)(void *ctx,
> - const void *data, size_t len,
> - size_t asn1hdrlen),
> - void *ctx)
> +int verify_pkcs7_message_sig(const void *data, size_t len,
> + struct pkcs7_message *pkcs7,
> + struct key *trusted_keys,
> + enum key_being_used_for usage,
> + int (*view_content)(void *ctx,
> + const void *data, size_t len,
> + size_t asn1hdrlen),
> + void *ctx)
> {
> - struct pkcs7_message *pkcs7;
> int ret;
>
> - pkcs7 = pkcs7_parse_message(raw_pkcs7, pkcs7_len);
> - if (IS_ERR(pkcs7))
> - return PTR_ERR(pkcs7);
> -
> /* The data should be detached - so we need to supply it. */
> if (data && pkcs7_supply_detached_data(pkcs7, data, len) < 0) {
> pr_err("PKCS#7 signature with non-detached data\n");
> @@ -273,6 +267,41 @@ int verify_pkcs7_signature(const void *data, size_t len,
> }
>
> error:
> + pr_devel("<==%s() = %d\n", __func__, ret);
> + return ret;
> +}
> +
> +/**
> + * verify_pkcs7_signature - Verify a PKCS#7-based signature on system data.
> + * @data: The data to be verified (NULL if expecting internal data).
> + * @len: Size of @data.
> + * @raw_pkcs7: The PKCS#7 message that is the signature.
> + * @pkcs7_len: The size of @raw_pkcs7.
> + * @trusted_keys: Trusted keys to use (NULL for builtin trusted keys only,
> + * (void *)1UL for all trusted keys).
> + * @usage: The use to which the key is being put.
> + * @view_content: Callback to gain access to content.
> + * @ctx: Context for callback.
> + */
> +int verify_pkcs7_signature(const void *data, size_t len,
> + const void *raw_pkcs7, size_t pkcs7_len,
> + struct key *trusted_keys,
> + enum key_being_used_for usage,
> + int (*view_content)(void *ctx,
> + const void *data, size_t len,
> + size_t asn1hdrlen),
> + void *ctx)
> +{
> + struct pkcs7_message *pkcs7;
> + int ret;
> +
> + pkcs7 = pkcs7_parse_message(raw_pkcs7, pkcs7_len);
> + if (IS_ERR(pkcs7))
> + return PTR_ERR(pkcs7);
> +
> + ret = verify_pkcs7_message_sig(data, len, pkcs7, trusted_keys, usage,
> + view_content, ctx);
> +
> pkcs7_free_message(pkcs7);
> pr_devel("<==%s() = %d\n", __func__, ret);
> return ret;
> diff --git a/include/linux/verification.h b/include/linux/verification.h
> index 018fb5f13d44..5e1d41f2b336 100644
> --- a/include/linux/verification.h
> +++ b/include/linux/verification.h
> @@ -36,6 +36,7 @@ extern const char *const key_being_used_for[NR__KEY_BEING_USED_FOR];
> #ifdef CONFIG_SYSTEM_DATA_VERIFICATION
>
> struct key;
> +struct pkcs7_message;
>
> extern int verify_pkcs7_signature(const void *data, size_t len,
> const void *raw_pkcs7, size_t pkcs7_len,
> @@ -45,6 +46,15 @@ extern int verify_pkcs7_signature(const void *data, size_t len,
> const void *data, size_t len,
> size_t asn1hdrlen),
> void *ctx);
> +extern int verify_pkcs7_message_sig(const void *data, size_t len,
> + struct pkcs7_message *pkcs7,
> + struct key *trusted_keys,
> + enum key_being_used_for usage,
> + int (*view_content)(void *ctx,
> + const void *data,
> + size_t len,
> + size_t asn1hdrlen),
> + void *ctx);
>
> #ifdef CONFIG_SIGNED_PE_FILE_VERIFICATION
> extern int verify_pefile_signature(const void *pebuf, unsigned pelen,
^ permalink raw reply
* Re: [PATCH v11 01/13] MODSIGN: Export module signature definitions
From: Thiago Jung Bauermann @ 2019-06-24 19:52 UTC (permalink / raw)
To: Jessica Yu
Cc: Herbert Xu, linux-doc, Dmitry Kasatkin, David S. Miller,
Jonathan Corbet, linux-kernel, Mimi Zohar, James Morris,
David Howells, AKASHI, Takahiro, linux-security-module, keyrings,
linux-crypto, linux-integrity, linuxppc-dev, David Woodhouse,
Serge E. Hallyn
In-Reply-To: <20190611062817.18412-2-bauerman@linux.ibm.com>
Hello Jessica,
AFAIK Mimi is happy with this patch set, but I still need acks from
maintainers of other subsystems that my changes touch before she can
accept it.
Is this patch OK from your PoV?
--
Thiago Jung Bauermann
IBM Linux Technology Center
Thiago Jung Bauermann <bauerman@linux.ibm.com> writes:
> 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 mod_check_sig() without having to depend on either
> CONFIG_MODULE_SIG or CONFIG_MODULES.
>
> Signed-off-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
> Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
> Cc: Jessica Yu <jeyu@kernel.org>
> ---
> include/linux/module.h | 3 --
> include/linux/module_signature.h | 44 +++++++++++++++++++++++++
> init/Kconfig | 6 +++-
> kernel/Makefile | 1 +
> kernel/module.c | 1 +
> kernel/module_signature.c | 46 ++++++++++++++++++++++++++
> kernel/module_signing.c | 56 +++++---------------------------
> scripts/Makefile | 2 +-
> 8 files changed, 106 insertions(+), 53 deletions(-)
>
> diff --git a/include/linux/module.h b/include/linux/module.h
> index 188998d3dca9..aa56f531cf1e 100644
> --- a/include/linux/module.h
> +++ b/include/linux/module.h
> @@ -25,9 +25,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..523617fc5b6a
> --- /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 mod_check_sig(const struct module_signature *ms, size_t file_len,
> + const char *name);
> +
> +#endif /* _LINUX_MODULE_SIGNATURE_H */
> diff --git a/init/Kconfig b/init/Kconfig
> index 8b9ffe236e4f..c2286a3c74c5 100644
> --- a/init/Kconfig
> +++ b/init/Kconfig
> @@ -1852,6 +1852,10 @@ config BASE_SMALL
> default 0 if BASE_FULL
> default 1 if !BASE_FULL
>
> +config MODULE_SIG_FORMAT
> + def_bool n
> + select SYSTEM_DATA_VERIFICATION
> +
> menuconfig MODULES
> bool "Enable loadable module support"
> option modules
> @@ -1929,7 +1933,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
> diff --git a/kernel/Makefile b/kernel/Makefile
> index 33824f0385b3..f29ae2997a43 100644
> --- a/kernel/Makefile
> +++ b/kernel/Makefile
> @@ -58,6 +58,7 @@ endif
> obj-$(CONFIG_UID16) += uid16.o
> obj-$(CONFIG_MODULES) += module.o
> obj-$(CONFIG_MODULE_SIG) += module_signing.o
> +obj-$(CONFIG_MODULE_SIG_FORMAT) += module_signature.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 6e6712b3aaf5..2712f4d217f5 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_signature.c b/kernel/module_signature.c
> new file mode 100644
> index 000000000000..4224a1086b7d
> --- /dev/null
> +++ b/kernel/module_signature.c
> @@ -0,0 +1,46 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Module signature checker
> + *
> + * Copyright (C) 2012 Red Hat, Inc. All Rights Reserved.
> + * Written by David Howells (dhowells@redhat.com)
> + */
> +
> +#include <linux/errno.h>
> +#include <linux/printk.h>
> +#include <linux/module_signature.h>
> +#include <asm/byteorder.h>
> +
> +/**
> + * mod_check_sig - check that the given signature is sane
> + *
> + * @ms: Signature to check.
> + * @file_len: Size of the file to which @ms is appended.
> + * @name: What is being checked. Used for error messages.
> + */
> +int mod_check_sig(const struct module_signature *ms, size_t file_len,
> + const char *name)
> +{
> + if (be32_to_cpu(ms->sig_len) >= file_len - sizeof(*ms))
> + return -EBADMSG;
> +
> + if (ms->id_type != PKEY_ID_PKCS7) {
> + pr_err("%s: Module is not signed with expected PKCS#7 message\n",
> + name);
> + 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("%s: PKCS#7 signature info has unexpected non-zero params\n",
> + name);
> + return -EBADMSG;
> + }
> +
> + return 0;
> +}
> diff --git a/kernel/module_signing.c b/kernel/module_signing.c
> index 6b9a926fd86b..cdd04a6b8074 100644
> --- a/kernel/module_signing.c
> +++ b/kernel/module_signing.c
> @@ -11,37 +11,13 @@
>
> #include <linux/kernel.h>
> #include <linux/errno.h>
> +#include <linux/module.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.
> - *
> - * 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 */
> -};
> -
> /*
> * Verify the signature on a module.
> */
> @@ -49,6 +25,7 @@ int mod_verify_sig(const void *mod, struct load_info *info)
> {
> struct module_signature ms;
> size_t sig_len, modlen = info->len;
> + int ret;
>
> pr_devel("==>%s(,%zu)\n", __func__, modlen);
>
> @@ -56,32 +33,15 @@ int mod_verify_sig(const void *mod, struct load_info *info)
> return -EBADMSG;
>
> memcpy(&ms, mod + (modlen - sizeof(ms)), sizeof(ms));
> - modlen -= sizeof(ms);
> +
> + ret = mod_check_sig(&ms, modlen, info->name);
> + 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);
> info->len = modlen;
>
> - if (ms.id_type != PKEY_ID_PKCS7) {
> - pr_err("%s: Module is not signed with expected PKCS#7 message\n",
> - info->name);
> - 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("%s: PKCS#7 signature info has unexpected non-zero params\n",
> - info->name);
> - return -EBADMSG;
> - }
> -
> return verify_pkcs7_signature(mod, modlen, mod + modlen, sig_len,
> VERIFY_USE_SECONDARY_KEYRING,
> VERIFYING_MODULE_SIGNATURE,
> diff --git a/scripts/Makefile b/scripts/Makefile
> index 9d442ee050bd..52098b080ab7 100644
> --- a/scripts/Makefile
> +++ b/scripts/Makefile
> @@ -17,7 +17,7 @@ hostprogs-$(CONFIG_VT) += conmakehash
> hostprogs-$(BUILD_C_RECORDMCOUNT) += recordmcount
> hostprogs-$(CONFIG_BUILDTIME_EXTABLE_SORT) += sortextable
> hostprogs-$(CONFIG_ASN1) += asn1_compiler
> -hostprogs-$(CONFIG_MODULE_SIG) += sign-file
> +hostprogs-$(CONFIG_MODULE_SIG_FORMAT) += sign-file
> hostprogs-$(CONFIG_SYSTEM_TRUSTED_KEYRING) += extract-cert
> hostprogs-$(CONFIG_SYSTEM_EXTRA_CERTIFICATE) += insert-sys-cert
>
^ permalink raw reply
* Re: [PATCH] powerpc/rtas: retry when cpu offline races with suspend/migration
From: Nathan Lynch @ 2019-06-24 17:23 UTC (permalink / raw)
To: mmc; +Cc: ego, linuxppc-dev, julietk
In-Reply-To: <f3b54ef4394bdbf4887d2185bb951c80@linux.vnet.ibm.com>
Hi Mingming,
mmc <mmc@linux.vnet.ibm.com> writes:
> On 2019-06-21 00:05, Nathan Lynch wrote:
>> So return -EAGAIN instead of -EBUSY when this race is
>> encountered. Additionally: logging this event is still appropriate but
>> use pr_info instead of pr_err; and remove use of unlikely() while here
>> as this is not a hot path at all.
>
> Looks good, since it's not a hot path anyway, so unlikely() should
> benefit from optimize compiler path, and should stay. No?
The latency of this path in rtas_ibm_suspend_me() in the best case is
around 2-3 seconds.
So I think not -- this is such a heavyweight and relatively
seldom-executed path that the unlikely() cannot yield any discernible
performance benefit, and its presence imposes a readability cost.
^ permalink raw reply
* Re: [next][PowerPC] RCU stalls while booting linux-next on PowerVM LPAR
From: Sachin Sant @ 2019-06-24 17:09 UTC (permalink / raw)
To: David Hildenbrand
Cc: Stephen Rothwell, linux-next, linuxppc-dev, linux-kernel
In-Reply-To: <83a31209-68cb-3e60-6b50-322154e6dcea@redhat.com>
> On 24-Jun-2019, at 8:12 PM, David Hildenbrand <david@redhat.com> wrote:
>
> On 24.06.19 16:09, Sachin Sant wrote:
>> Latest -next fails to boot on POWER9 PowerVM LPAR due to RCU stalls.
>>
>> This problem was introduced with next-20190620 (dc636f5d78).
>> next-20190619 was last good kernel.
>>
>> Reverting following commit allows the kernel to boot.
>> 2fd4aeea6b603 : mm/memory_hotplug: move and simplify walk_memory_blocks()
>>
>>
>> [ 0.014409] Using shared cache scheduler topology
>> [ 0.016302] devtmpfs: initialized
>> [ 0.031022] clocksource: jiffies: mask: 0xffffffff max_cycles: 0xffffffff, max_idle_ns: 19112604462750000 ns
>> [ 0.031034] futex hash table entries: 16384 (order: 5, 2097152 bytes, linear)
>> [ 0.031575] NET: Registered protocol family 16
>> [ 0.031724] audit: initializing netlink subsys (disabled)
>> [ 0.031796] audit: type=2000 audit(1561344029.030:1): state=initialized audit_enabled=0 res=1
>> [ 0.032249] cpuidle: using governor menu
>> [ 0.032403] pstore: Registered nvram as persistent store backend
>> [ 60.061246] rcu: INFO: rcu_sched self-detected stall on CPU
>> [ 60.061254] rcu: 0-....: (5999 ticks this GP) idle=1ea/1/0x4000000000000002 softirq=5/5 fqs=2999
>> [ 60.061261] (t=6000 jiffies g=-1187 q=0)
>> [ 60.061265] NMI backtrace for cpu 0
>> [ 60.061269] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.2.0-rc5-next-20190621-autotest-autotest #1
>> [ 60.061275] Call Trace:
>> [ 60.061280] [c0000018ee85f380] [c000000000b624ec] dump_stack+0xb0/0xf4 (unreliable)
>> [ 60.061287] [c0000018ee85f3c0] [c000000000b6d464] nmi_cpu_backtrace+0x144/0x150
>> [ 60.061293] [c0000018ee85f450] [c000000000b6d61c] nmi_trigger_cpumask_backtrace+0x1ac/0x1f0
>> [ 60.061300] [c0000018ee85f4f0] [c0000000000692c8] arch_trigger_cpumask_backtrace+0x28/0x40
>> [ 60.061306] [c0000018ee85f510] [c0000000001c5f90] rcu_dump_cpu_stacks+0x10c/0x16c
>> [ 60.061313] [c0000018ee85f560] [c0000000001c4fe4] rcu_sched_clock_irq+0x744/0x990
>> [ 60.061318] [c0000018ee85f630] [c0000000001d5b58] update_process_times+0x48/0x90
>> [ 60.061325] [c0000018ee85f660] [c0000000001ea03c] tick_periodic+0x4c/0x120
>> [ 60.061330] [c0000018ee85f690] [c0000000001ea150] tick_handle_periodic+0x40/0xe0
>> [ 60.061336] [c0000018ee85f6d0] [c00000000002b5cc] timer_interrupt+0x10c/0x2e0
>> [ 60.061342] [c0000018ee85f730] [c000000000009204] decrementer_common+0x134/0x140
>> [ 60.061350] --- interrupt: 901 at replay_interrupt_return+0x0/0x4
>> [ 60.061350] LR = arch_local_irq_restore+0x84/0x90
>> [ 60.061357] [c0000018ee85fa30] [c0000018ee85fbac] 0xc0000018ee85fbac (unreliable)
>> [ 60.061364] [c0000018ee85fa50] [c000000000b88300] _raw_spin_unlock_irqrestore+0x50/0x80
>> [ 60.061369] [c0000018ee85fa70] [c000000000b69da4] klist_next+0xb4/0x150
>> [ 60.061376] [c0000018ee85fac0] [c000000000766ea0] subsys_find_device_by_id+0xf0/0x1a0
>> [ 60.061382] [c0000018ee85fb20] [c000000000797a94] walk_memory_blocks+0x84/0x100
>> [ 60.061388] [c0000018ee85fb80] [c000000000795ea0] link_mem_sections+0x40/0x60
>> [ 60.061395] [c0000018ee85fbb0] [c000000000f28c28] topology_init+0xa0/0x268
>> [ 60.061400] [c0000018ee85fc10] [c000000000010448] do_one_initcall+0x68/0x2c0
>> [ 60.061406] [c0000018ee85fce0] [c000000000f247dc] kernel_init_freeable+0x318/0x47c
>> [ 60.061411] [c0000018ee85fdb0] [c0000000000107c4] kernel_init+0x24/0x150
>> [ 60.061417] [c0000018ee85fe20] [c00000000000ba54] ret_from_kernel_thread+0x5c/0x68
>> [ 88.016563] watchdog: BUG: soft lockup - CPU#0 stuck for 22s! [swapper/0:1]
>> [ 88.016569] Modules linked in:
>>
>
> Hi, thanks! Please see
>
> https://lkml.org/lkml/2019/6/21/600
>
> and especially
>
> https://lkml.org/lkml/2019/6/21/908
>
> Does this fix your problem? The fix is on its way to next.
Yes, this patch fixes the problem for me.
Thanks
-Sachin
^ permalink raw reply
* Re: [PATCH] powerpc/rtas: retry when cpu offline races with suspend/migration
From: mmc @ 2019-06-24 16:52 UTC (permalink / raw)
To: Nathan Lynch; +Cc: ego, linuxppc-dev, julietk
In-Reply-To: <20190621060518.29616-1-nathanl@linux.ibm.com>
On 2019-06-21 00:05, Nathan Lynch wrote:
> The protocol for suspending or migrating an LPAR requires all present
> processor threads to enter H_JOIN. So if we have threads offline, we
> have to temporarily bring them up. This can race with administrator
> actions such as SMT state changes. As of dfd718a2ed1f ("powerpc/rtas:
> Fix a potential race between CPU-Offline & Migration"),
> rtas_ibm_suspend_me() accounts for this, but errors out with -EBUSY
> for what almost certainly is a transient condition in any reasonable
> scenario.
>
> Callers of rtas_ibm_suspend_me() already retry when -EAGAIN is
> returned, and it is typical during a migration for that to happen
> repeatedly for several minutes polling the H_VASI_STATE hcall result
> before proceeding to the next stage.
>
> So return -EAGAIN instead of -EBUSY when this race is
> encountered. Additionally: logging this event is still appropriate but
> use pr_info instead of pr_err; and remove use of unlikely() while here
> as this is not a hot path at all.
>
Looks good, since it's not a hot path anyway, so unlikely() should
benefit from optimize compiler path, and should stay. No?
Mingming
> Fixes: dfd718a2ed1f ("powerpc/rtas: Fix a potential race between
> CPU-Offline & Migration")
> Signed-off-by: Nathan Lynch <nathanl@linux.ibm.com>
> ---
> arch/powerpc/kernel/rtas.c | 7 +++----
> 1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/arch/powerpc/kernel/rtas.c b/arch/powerpc/kernel/rtas.c
> index fbc676160adf..9b4d2a2ffb4f 100644
> --- a/arch/powerpc/kernel/rtas.c
> +++ b/arch/powerpc/kernel/rtas.c
> @@ -984,10 +984,9 @@ int rtas_ibm_suspend_me(u64 handle)
> cpu_hotplug_disable();
>
> /* Check if we raced with a CPU-Offline Operation */
> - if (unlikely(!cpumask_equal(cpu_present_mask, cpu_online_mask))) {
> - pr_err("%s: Raced against a concurrent CPU-Offline\n",
> - __func__);
> - atomic_set(&data.error, -EBUSY);
> + if (!cpumask_equal(cpu_present_mask, cpu_online_mask)) {
> + pr_info("%s: Raced against a concurrent CPU-Offline\n", __func__);
> + atomic_set(&data.error, -EAGAIN);
> goto out_hotplug_enable;
> }
^ permalink raw reply
* Re: [PATCH] ocxl: Fix concurrent AFU open and device removal
From: Greg Kurz @ 2019-06-24 15:24 UTC (permalink / raw)
To: Frederic Barrat; +Cc: clombard, linuxppc-dev, alastair, andrew.donnellan
In-Reply-To: <20190624144148.32022-1-fbarrat@linux.ibm.com>
On Mon, 24 Jun 2019 16:41:48 +0200
Frederic Barrat <fbarrat@linux.ibm.com> wrote:
> If an ocxl device is unbound through sysfs at the same time its AFU is
> being opened by a user process, the open code may dereference freed
> stuctures, which can lead to kernel oops messages. You'd have to hit a
> tiny time window, but it's possible. It's fairly easy to test by
> making the time window bigger artificially.
>
> Fix it with a combination of 2 changes:
> - when an AFU device is found in the IDR by looking for the device
> minor number, we should hold a reference on the device until after the
> context is allocated. A reference on the AFU structure is kept when
> the context is allocated, so we can release the reference on the
> device after the context allocation.
> - with the fix above, there's still another even tinier window,
> between the time the AFU device is found in the IDR and the reference
> on the device is taken. We can fix this one by removing the IDR entry
> earlier, when the device setup is removed, instead of waiting for the
> 'release' device callback. With proper locking around the IDR.
>
> Fixes: 75ca758adbaf ("ocxl: Create a clear delineation between ocxl backend & frontend")
> Signed-off-by: Frederic Barrat <fbarrat@linux.ibm.com>
> ---
> mpe: this fixes a commit merged in v5.2-rc1. It's late, and I don't think it's that important. If it's for the next merge window, I would add:
> Cc: stable@vger.kernel.org # v5.2
>
>
> drivers/misc/ocxl/file.c | 23 +++++++++++------------
> 1 file changed, 11 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/misc/ocxl/file.c b/drivers/misc/ocxl/file.c
> index 2870c25da166..4d1b44de1492 100644
> --- a/drivers/misc/ocxl/file.c
> +++ b/drivers/misc/ocxl/file.c
> @@ -18,18 +18,15 @@ static struct class *ocxl_class;
> static struct mutex minors_idr_lock;
> static struct idr minors_idr;
>
> -static struct ocxl_file_info *find_file_info(dev_t devno)
> +static struct ocxl_file_info *find_and_get_file_info(dev_t devno)
> {
> struct ocxl_file_info *info;
>
> - /*
> - * We don't declare an RCU critical section here, as our AFU
> - * is protected by a reference counter on the device. By the time the
> - * info reference is removed from the idr, the ref count of
> - * the device is already at 0, so no user API will access that AFU and
> - * this function can't return it.
> - */
> + mutex_lock(&minors_idr_lock);
> info = idr_find(&minors_idr, MINOR(devno));
> + if (info)
> + get_device(&info->dev);
> + mutex_unlock(&minors_idr_lock);
> return info;
> }
>
> @@ -58,14 +55,16 @@ static int afu_open(struct inode *inode, struct file *file)
>
> pr_debug("%s for device %x\n", __func__, inode->i_rdev);
>
> - info = find_file_info(inode->i_rdev);
> + info = find_and_get_file_info(inode->i_rdev);
> if (!info)
> return -ENODEV;
>
> rc = ocxl_context_alloc(&ctx, info->afu, inode->i_mapping);
> - if (rc)
> + if (rc) {
> + put_device(&info->dev);
You could have a single call site for put_device() since it's
needed for both branches. No big deal.
> return rc;
> -
> + }
> + put_device(&info->dev);
> file->private_data = ctx;
> return 0;
> }
> @@ -487,7 +486,6 @@ static void info_release(struct device *dev)
> {
> struct ocxl_file_info *info = container_of(dev, struct ocxl_file_info, dev);
>
> - free_minor(info);
> ocxl_afu_put(info->afu);
> kfree(info);
> }
> @@ -577,6 +575,7 @@ void ocxl_file_unregister_afu(struct ocxl_afu *afu)
>
> ocxl_file_make_invisible(info);
> ocxl_sysfs_unregister_afu(info);
> + free_minor(info);
Since the IDR entry is added by ocxl_file_register_afu(), it seems to make
sense to undo that in ocxl_file_unregister_afu(). Out of curiosity, was there
any historical reason to do this in info_release() in the first place ?
Reviewed-by: Greg Kurz <groug@kaod.org>
> device_unregister(&info->dev);
> }
>
^ permalink raw reply
* Re: [PATCH] ocxl: Fix concurrent AFU open and device removal
From: Greg Kurz @ 2019-06-24 15:50 UTC (permalink / raw)
To: Frederic Barrat; +Cc: clombard, linuxppc-dev, alastair, andrew.donnellan
In-Reply-To: <ea1295fe-d8ad-1e5f-54f7-a72a7149c5b7@linux.ibm.com>
On Mon, 24 Jun 2019 17:39:26 +0200
Frederic Barrat <fbarrat@linux.ibm.com> wrote:
> Le 24/06/2019 à 17:24, Greg Kurz a écrit :
> > On Mon, 24 Jun 2019 16:41:48 +0200
> > Frederic Barrat <fbarrat@linux.ibm.com> wrote:
> >
> >> If an ocxl device is unbound through sysfs at the same time its AFU is
> >> being opened by a user process, the open code may dereference freed
> >> stuctures, which can lead to kernel oops messages. You'd have to hit a
> >> tiny time window, but it's possible. It's fairly easy to test by
> >> making the time window bigger artificially.
> >>
> >> Fix it with a combination of 2 changes:
> >> - when an AFU device is found in the IDR by looking for the device
> >> minor number, we should hold a reference on the device until after the
> >> context is allocated. A reference on the AFU structure is kept when
> >> the context is allocated, so we can release the reference on the
> >> device after the context allocation.
> >> - with the fix above, there's still another even tinier window,
> >> between the time the AFU device is found in the IDR and the reference
> >> on the device is taken. We can fix this one by removing the IDR entry
> >> earlier, when the device setup is removed, instead of waiting for the
> >> 'release' device callback. With proper locking around the IDR.
> >>
> >> Fixes: 75ca758adbaf ("ocxl: Create a clear delineation between ocxl backend & frontend")
> >> Signed-off-by: Frederic Barrat <fbarrat@linux.ibm.com>
> >> ---
> >> mpe: this fixes a commit merged in v5.2-rc1. It's late, and I don't think it's that important. If it's for the next merge window, I would add:
> >> Cc: stable@vger.kernel.org # v5.2
> >>
> >>
> >> drivers/misc/ocxl/file.c | 23 +++++++++++------------
> >> 1 file changed, 11 insertions(+), 12 deletions(-)
> >>
> >> diff --git a/drivers/misc/ocxl/file.c b/drivers/misc/ocxl/file.c
> >> index 2870c25da166..4d1b44de1492 100644
> >> --- a/drivers/misc/ocxl/file.c
> >> +++ b/drivers/misc/ocxl/file.c
> >> @@ -18,18 +18,15 @@ static struct class *ocxl_class;
> >> static struct mutex minors_idr_lock;
> >> static struct idr minors_idr;
> >>
> >> -static struct ocxl_file_info *find_file_info(dev_t devno)
> >> +static struct ocxl_file_info *find_and_get_file_info(dev_t devno)
> >> {
> >> struct ocxl_file_info *info;
> >>
> >> - /*
> >> - * We don't declare an RCU critical section here, as our AFU
> >> - * is protected by a reference counter on the device. By the time the
> >> - * info reference is removed from the idr, the ref count of
> >> - * the device is already at 0, so no user API will access that AFU and
> >> - * this function can't return it.
> >> - */
> >> + mutex_lock(&minors_idr_lock);
> >> info = idr_find(&minors_idr, MINOR(devno));
> >> + if (info)
> >> + get_device(&info->dev);
> >> + mutex_unlock(&minors_idr_lock);
> >> return info;
> >> }
> >>
> >> @@ -58,14 +55,16 @@ static int afu_open(struct inode *inode, struct file *file)
> >>
> >> pr_debug("%s for device %x\n", __func__, inode->i_rdev);
> >>
> >> - info = find_file_info(inode->i_rdev);
> >> + info = find_and_get_file_info(inode->i_rdev);
> >> if (!info)
> >> return -ENODEV;
> >>
> >> rc = ocxl_context_alloc(&ctx, info->afu, inode->i_mapping);
> >> - if (rc)
> >> + if (rc) {
> >> + put_device(&info->dev);
> >
> > You could have a single call site for put_device() since it's
> > needed for both branches. No big deal.
>
>
> Agreed. Will fix if I end up respinning, but won't if it's the only
> complaint :-)
>
>
>
> >> return rc;
> >> -
> >> + }
> >> + put_device(&info->dev);
> >> file->private_data = ctx;
> >> return 0;
> >> }
> >> @@ -487,7 +486,6 @@ static void info_release(struct device *dev)
> >> {
> >> struct ocxl_file_info *info = container_of(dev, struct ocxl_file_info, dev);
> >>
> >> - free_minor(info);
> >> ocxl_afu_put(info->afu);
> >> kfree(info);
> >> }
> >> @@ -577,6 +575,7 @@ void ocxl_file_unregister_afu(struct ocxl_afu *afu)
> >>
> >> ocxl_file_make_invisible(info);
> >> ocxl_sysfs_unregister_afu(info);
> >> + free_minor(info);
> >
> > Since the IDR entry is added by ocxl_file_register_afu(), it seems to make
> > sense to undo that in ocxl_file_unregister_afu(). Out of curiosity, was there
> > any historical reason to do this in info_release() in the first place ?
>
>
> Yeah, it makes a lot of sense to remove the IDR entry in
> ocxl_file_unregister_afu(), that's where we undo the device. I wish I
> had noticed during the code reviews.
> I don't think there was any good reason to have it in info_release() in
> the first place. I remember the code went through many iterations to get
> the reference counting on the AFU structure and device done correctly,
> but we let that one slip.
>
> I now think the pre-5.2 ocxl code was also exposed to the 2nd window
> mentioned in the commit log (but the first window is new with the
> refactoring introduced in 5.2-rc1).
>
This calls for two separate patches then IMHO.
> Fred
>
>
>
> >
> > Reviewed-by: Greg Kurz <groug@kaod.org>
> >
> >> device_unregister(&info->dev);
> >> }
> >>
> >
>
^ permalink raw reply
* Re: [PATCH] ocxl: Fix concurrent AFU open and device removal
From: Frederic Barrat @ 2019-06-24 15:39 UTC (permalink / raw)
To: Greg Kurz; +Cc: clombard, linuxppc-dev, alastair, andrew.donnellan
In-Reply-To: <20190624172452.7e217596@bahia.lan>
Le 24/06/2019 à 17:24, Greg Kurz a écrit :
> On Mon, 24 Jun 2019 16:41:48 +0200
> Frederic Barrat <fbarrat@linux.ibm.com> wrote:
>
>> If an ocxl device is unbound through sysfs at the same time its AFU is
>> being opened by a user process, the open code may dereference freed
>> stuctures, which can lead to kernel oops messages. You'd have to hit a
>> tiny time window, but it's possible. It's fairly easy to test by
>> making the time window bigger artificially.
>>
>> Fix it with a combination of 2 changes:
>> - when an AFU device is found in the IDR by looking for the device
>> minor number, we should hold a reference on the device until after the
>> context is allocated. A reference on the AFU structure is kept when
>> the context is allocated, so we can release the reference on the
>> device after the context allocation.
>> - with the fix above, there's still another even tinier window,
>> between the time the AFU device is found in the IDR and the reference
>> on the device is taken. We can fix this one by removing the IDR entry
>> earlier, when the device setup is removed, instead of waiting for the
>> 'release' device callback. With proper locking around the IDR.
>>
>> Fixes: 75ca758adbaf ("ocxl: Create a clear delineation between ocxl backend & frontend")
>> Signed-off-by: Frederic Barrat <fbarrat@linux.ibm.com>
>> ---
>> mpe: this fixes a commit merged in v5.2-rc1. It's late, and I don't think it's that important. If it's for the next merge window, I would add:
>> Cc: stable@vger.kernel.org # v5.2
>>
>>
>> drivers/misc/ocxl/file.c | 23 +++++++++++------------
>> 1 file changed, 11 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/misc/ocxl/file.c b/drivers/misc/ocxl/file.c
>> index 2870c25da166..4d1b44de1492 100644
>> --- a/drivers/misc/ocxl/file.c
>> +++ b/drivers/misc/ocxl/file.c
>> @@ -18,18 +18,15 @@ static struct class *ocxl_class;
>> static struct mutex minors_idr_lock;
>> static struct idr minors_idr;
>>
>> -static struct ocxl_file_info *find_file_info(dev_t devno)
>> +static struct ocxl_file_info *find_and_get_file_info(dev_t devno)
>> {
>> struct ocxl_file_info *info;
>>
>> - /*
>> - * We don't declare an RCU critical section here, as our AFU
>> - * is protected by a reference counter on the device. By the time the
>> - * info reference is removed from the idr, the ref count of
>> - * the device is already at 0, so no user API will access that AFU and
>> - * this function can't return it.
>> - */
>> + mutex_lock(&minors_idr_lock);
>> info = idr_find(&minors_idr, MINOR(devno));
>> + if (info)
>> + get_device(&info->dev);
>> + mutex_unlock(&minors_idr_lock);
>> return info;
>> }
>>
>> @@ -58,14 +55,16 @@ static int afu_open(struct inode *inode, struct file *file)
>>
>> pr_debug("%s for device %x\n", __func__, inode->i_rdev);
>>
>> - info = find_file_info(inode->i_rdev);
>> + info = find_and_get_file_info(inode->i_rdev);
>> if (!info)
>> return -ENODEV;
>>
>> rc = ocxl_context_alloc(&ctx, info->afu, inode->i_mapping);
>> - if (rc)
>> + if (rc) {
>> + put_device(&info->dev);
>
> You could have a single call site for put_device() since it's
> needed for both branches. No big deal.
Agreed. Will fix if I end up respinning, but won't if it's the only
complaint :-)
>> return rc;
>> -
>> + }
>> + put_device(&info->dev);
>> file->private_data = ctx;
>> return 0;
>> }
>> @@ -487,7 +486,6 @@ static void info_release(struct device *dev)
>> {
>> struct ocxl_file_info *info = container_of(dev, struct ocxl_file_info, dev);
>>
>> - free_minor(info);
>> ocxl_afu_put(info->afu);
>> kfree(info);
>> }
>> @@ -577,6 +575,7 @@ void ocxl_file_unregister_afu(struct ocxl_afu *afu)
>>
>> ocxl_file_make_invisible(info);
>> ocxl_sysfs_unregister_afu(info);
>> + free_minor(info);
>
> Since the IDR entry is added by ocxl_file_register_afu(), it seems to make
> sense to undo that in ocxl_file_unregister_afu(). Out of curiosity, was there
> any historical reason to do this in info_release() in the first place ?
Yeah, it makes a lot of sense to remove the IDR entry in
ocxl_file_unregister_afu(), that's where we undo the device. I wish I
had noticed during the code reviews.
I don't think there was any good reason to have it in info_release() in
the first place. I remember the code went through many iterations to get
the reference counting on the AFU structure and device done correctly,
but we let that one slip.
I now think the pre-5.2 ocxl code was also exposed to the 2nd window
mentioned in the commit log (but the first window is new with the
refactoring introduced in 5.2-rc1).
Fred
>
> Reviewed-by: Greg Kurz <groug@kaod.org>
>
>> device_unregister(&info->dev);
>> }
>>
>
^ permalink raw reply
* [PATCH 2/2] powerpc/papr_scm: Force a scm-unbind if initial scm-bind fails
From: Vaibhav Jain @ 2019-06-24 14:59 UTC (permalink / raw)
To: linuxppc-dev
Cc: Aneesh Kumar K . V, Oliver O'Halloran, Vaibhav Jain,
Laurent Dufour, David Gibson
In-Reply-To: <20190624145913.20122-1-vaibhav@linux.ibm.com>
In some cases initial bind of scm memory for an lpar can fail if
previously it wasn't released using a scm-unbind hcall. This situation
can arise due to panic of the previous kernel or forced lpar reset. In
such cases the H_SCM_BIND_MEM return a H_OVERLAP error.
To mitigate such cases the patch updates drc_pmem_bind() to force a
call to drc_pmem_unbind() in case the initial bind of scm memory fails
with H_OVERLAP error. In case scm-bind operation again fails after the
forced scm-unbind then we follow the existing error path.
Signed-off-by: Vaibhav Jain <vaibhav@linux.ibm.com>
---
arch/powerpc/platforms/pseries/papr_scm.c | 23 ++++++++++++++++++++---
1 file changed, 20 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/platforms/pseries/papr_scm.c b/arch/powerpc/platforms/pseries/papr_scm.c
index d790e4e4ffb3..049d7927c0a4 100644
--- a/arch/powerpc/platforms/pseries/papr_scm.c
+++ b/arch/powerpc/platforms/pseries/papr_scm.c
@@ -44,19 +44,26 @@ struct papr_scm_priv {
struct nd_interleave_set nd_set;
};
+/* Forward declaration */
+static int drc_pmem_unbind(struct papr_scm_priv *);
+
static int drc_pmem_bind(struct papr_scm_priv *p)
{
unsigned long ret[PLPAR_HCALL_BUFSIZE];
uint64_t rc, token;
- uint64_t saved = 0;
+ uint64_t saved;
+ bool tried_unbind = false;
+ dev_dbg(&p->pdev->dev, "bind drc %x\n", p->drc_index);
/*
* When the hypervisor cannot map all the requested memory in a single
* hcall it returns H_BUSY and we call again with the token until
* we get H_SUCCESS. Aborting the retry loop before getting H_SUCCESS
* leave the system in an undefined state, so we wait.
*/
+retry:
token = 0;
+ saved = 0;
do {
rc = plpar_hcall(H_SCM_BIND_MEM, ret, p->drc_index, 0,
@@ -68,8 +75,18 @@ static int drc_pmem_bind(struct papr_scm_priv *p)
} while (rc == H_BUSY);
if (rc) {
- dev_err(&p->pdev->dev, "bind err: %lld\n", rc);
- return -ENXIO;
+ /* retry after unbinding */
+ if (rc == H_OVERLAP && !tried_unbind) {
+ dev_warn(&p->pdev->dev, "Un-binding and retrying\n");
+ /* Try unbind and ignore any errors */
+ tried_unbind = true;
+ drc_pmem_unbind(p);
+ goto retry;
+
+ } else {
+ dev_err(&p->pdev->dev, "bind err: %lld\n", rc);
+ return -ENXIO;
+ }
}
p->bound_addr = saved;
--
2.21.0
^ permalink raw reply related
* [PATCH 0/2] powerpc/papr_scm: Workaround for failure of drc bind after kexec
From: Vaibhav Jain @ 2019-06-24 14:59 UTC (permalink / raw)
To: linuxppc-dev
Cc: Aneesh Kumar K . V, Oliver O'Halloran, Vaibhav Jain,
Laurent Dufour, David Gibson
Presently an error is returned in response to hcall H_SCM_BIND_MEM when a
new kernel boots on lpar via kexec. This prevents papr_scm from registering
drc memory regions with nvdimm. The error reported is of the form below:
"papr_scm ibm,persistent-memory:ibm,pmemory@44100002: bind err: -68"
On investigation it was revealed that phyp returns this error as previous
kernel did not completely release bindings for drc scm-memory blocks and
hence phyp rejected request for re-binding these block to lpar with error
H_OVERLAP. Also support for a new H_SCM_UNBIND_ALL is recently added which
is better suited for releasing all the bound scm-memory block from an lpar.
So leveraging new hcall H_SCM_UNBIND_ALL, we can workaround H_OVERLAP issue
during kexec by forcing an unbind of all drm scm-memory blocks and issuing
H_SCM_BIND_MEM to re-bind the drc scm-memory blocks to lpar. This sequence
will also be needed when a new kernel boot on lpar after previous kernel
panicked and it never got an opportunity to call H_SCM_UNBIND_MEM/ALL.
Hence this patch-set implements following changes to papr_scm module:
* Update it to use H_SCM_UNBIND_ALL instead of H_SCM_UNBIND_MEM
* In case hcall H_SCM_BIND_MEM fails with error H_OVERLAP, force
H_SCM_UNBIND_ALL and retry the bind operation again.
With the patch-set applied re-bind of drc scm-memory to lpar succeeds after
a kexec to new kernel as illustrated below:
# Old kernel
$ sudo ndctl list -R
[
{
"dev":"region0",
<snip>
....
}
]
# kexec to new kernel
$ sudo kexec --initrd=... vmlinux
...
...
I'm in purgatory
...
papr_scm ibm,persistent-memory:ibm,pmemory@44100002: Un-binding and retrying
...
# New kernel
$ sudo ndctl list -R
$ sudo ndctl list -R
[
{
"dev":"region0",
<snip>
....
}
]
Vaibhav Jain (2):
powerpc/papr_scm: Update drc_pmem_unbind() to use H_SCM_UNBIND_ALL
powerpc/papr_scm: Force a scm-unbind if initial scm-bind fails
arch/powerpc/include/asm/hvcall.h | 2 +-
arch/powerpc/platforms/pseries/papr_scm.c | 60 ++++++++++++++++++++---
2 files changed, 53 insertions(+), 9 deletions(-)
--
2.21.0
^ permalink raw reply
* [PATCH 1/2] powerpc/papr_scm: Update drc_pmem_unbind() to use H_SCM_UNBIND_ALL
From: Vaibhav Jain @ 2019-06-24 14:59 UTC (permalink / raw)
To: linuxppc-dev
Cc: Aneesh Kumar K . V, Oliver O'Halloran, Vaibhav Jain,
Laurent Dufour, David Gibson
In-Reply-To: <20190624145913.20122-1-vaibhav@linux.ibm.com>
The new hcall named H_SCM_UNBIND_ALL has been introduce that can
unbind all the memory drc memory-blocks assigned to an lpar. This is
more efficient than using H_SCM_UNBIND_MEM as currently we don't
support partial unbind of drc memory-blocks.
Hence this patch proposes following changes to drc_pmem_unbind():
* Update drc_pmem_unbind() to replace hcall H_SCM_UNBIND_MEM to
H_SCM_UNBIND_ALL.
* Update drc_pmem_unbind() to handles cases when PHYP asks the guest
kernel to wait for specific amount of time before retrying the
hcall via the 'LONG_BUSY' return value. In case it takes more
than 1 second to unbind the memory log a warning.
* Ensure appropriate error code is returned back from the function
in case of an error.
Signed-off-by: Vaibhav Jain <vaibhav@linux.ibm.com>
---
arch/powerpc/include/asm/hvcall.h | 2 +-
arch/powerpc/platforms/pseries/papr_scm.c | 37 ++++++++++++++++++++---
2 files changed, 33 insertions(+), 6 deletions(-)
diff --git a/arch/powerpc/include/asm/hvcall.h b/arch/powerpc/include/asm/hvcall.h
index 463c63a9fcf1..bb56fa0f976c 100644
--- a/arch/powerpc/include/asm/hvcall.h
+++ b/arch/powerpc/include/asm/hvcall.h
@@ -302,7 +302,7 @@
#define H_SCM_UNBIND_MEM 0x3F0
#define H_SCM_QUERY_BLOCK_MEM_BINDING 0x3F4
#define H_SCM_QUERY_LOGICAL_MEM_BINDING 0x3F8
-#define H_SCM_MEM_QUERY 0x3FC
+#define H_SCM_UNBIND_ALL 0x3FC
#define H_SCM_BLOCK_CLEAR 0x400
#define MAX_HCALL_OPCODE H_SCM_BLOCK_CLEAR
diff --git a/arch/powerpc/platforms/pseries/papr_scm.c b/arch/powerpc/platforms/pseries/papr_scm.c
index 96c53b23e58f..d790e4e4ffb3 100644
--- a/arch/powerpc/platforms/pseries/papr_scm.c
+++ b/arch/powerpc/platforms/pseries/papr_scm.c
@@ -11,11 +11,16 @@
#include <linux/sched.h>
#include <linux/libnvdimm.h>
#include <linux/platform_device.h>
+#include <linux/delay.h>
#include <asm/plpar_wrappers.h>
#define BIND_ANY_ADDR (~0ul)
+/* Scope args for H_SCM_UNBIND_ALL */
+#define H_UNBIND_SCOPE_ALL (0x1)
+#define H_UNBIND_SCOPE_DRC (0x2)
+
#define PAPR_SCM_DIMM_CMD_MASK \
((1ul << ND_CMD_GET_CONFIG_SIZE) | \
(1ul << ND_CMD_GET_CONFIG_DATA) | \
@@ -78,21 +83,43 @@ static int drc_pmem_unbind(struct papr_scm_priv *p)
{
unsigned long ret[PLPAR_HCALL_BUFSIZE];
uint64_t rc, token;
+ unsigned long starttime;
token = 0;
- /* NB: unbind has the same retry requirements mentioned above */
+ dev_dbg(&p->pdev->dev, "unbind drc %x\n", p->drc_index);
+
+ /* NB: unbind_all has the same retry requirements as drc_pmem_bind() */
+ starttime = HZ;
do {
- rc = plpar_hcall(H_SCM_UNBIND_MEM, ret, p->drc_index,
- p->bound_addr, p->blocks, token);
+ /* If this is taking too much time then log warning */
+ if (jiffies_to_msecs(HZ - starttime) > 1000) {
+ dev_warn(&p->pdev->dev,
+ "unbind taking > 1s to complete\n");
+ starttime = HZ;
+ }
+
+ /* Unbind of all SCM resources associated with drcIndex */
+ rc = plpar_hcall(H_SCM_UNBIND_ALL, ret, H_UNBIND_SCOPE_DRC,
+ p->drc_index, token);
token = ret[0];
- cond_resched();
+
+ /* Check if we are stalled for some time */
+ if (H_IS_LONG_BUSY(rc)) {
+ msleep(get_longbusy_msecs(rc));
+ rc = H_BUSY;
+ token = 0;
+
+ } else if (rc == H_BUSY) {
+ cond_resched();
+ }
+
} while (rc == H_BUSY);
if (rc)
dev_err(&p->pdev->dev, "unbind error: %lld\n", rc);
- return !!rc;
+ return rc == H_SUCCESS ? 0 : -ENXIO;
}
static int papr_scm_meta_get(struct papr_scm_priv *p,
--
2.21.0
^ permalink raw reply related
* CVE-2019-12817: Linux kernel: powerpc: Unrelated processes may be able to read/write to each other's virtual memory
From: Michael Ellerman @ 2019-06-24 14:44 UTC (permalink / raw)
To: oss-security; +Cc: linuxppc-dev, linux-kernel, linuxppc-users
[-- Attachment #1: Type: text/plain, Size: 4251 bytes --]
The Linux kernel for powerpc since 4.17 has a bug where unrelated processes may
be able to read/write to each other's virtual memory under certain conditions.
This bug only affects machines using 64-bit CPUs with the hash page table MMU,
see below for more detail on affected CPUs.
To trigger the bug a process must allocate memory above 512TB. That only happens
if userspace explicitly requests it with mmap(). That process must then fork(),
at this point the child incorrectly inherits the "context id" of the parent
associated with the mapping above 512TB. It may then be possible for the
parent/child to write to each other's mappings above 512TB, which should not be
possible, and constitutes memory corruption.
If instead the child process exits, all its context ids are freed, including the
context id that is still in use by the parent for the mapping above 512TB. That
id can then be reallocated to a third process, that process can then read/write
to the parent's mapping above 512TB. Additionally if the freed id is used for
the third process's primary context id, then the parent is able to read/write to
the third process's mappings *below* 512TB.
If the parent and child both exit before another process is allocated the freed
context id, the kernel will notice the double free of the id and print a warning
such as:
ida_free called for id=103 which is not allocated.
WARNING: CPU: 8 PID: 7293 at lib/idr.c:520 ida_free_rc+0x1b4/0x1d0
The bug was introduced in commit:
f384796c40dc ("powerpc/mm: Add support for handling > 512TB address in SLB miss")
Which was originally merged in v4.17.
Only machines using the hash page table (HPT) MMU are affected, eg. PowerPC 970
(G5), PA6T, Power5/6/7/8/9. By default Power9 bare metal machines (powernv) use
the Radix MMU and are not affected, unless the machine has been explicitly
booted in HPT mode (using disable_radix on the kernel command line). KVM guests
on Power9 may be affected if the host or guest is configured to use the HPT MMU.
LPARs under PowerVM on Power9 are affected as they always use the HPT MMU.
Kernels built with PAGE_SIZE=4K are not affected.
The upstream fix is here:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=ca72d88378b2f2444d3ec145dd442d449d3fefbc
There's also a kernel selftest to verify the fix:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=16391bfc862342f285195013b73c1394fab28b97
Or a similar standalone version is included below.
cheers
cat > test.c <<EOF
#undef NDEBUG
#include <assert.h>
#include <stdio.h>
#include <stdlib.h>
#include <sys/mman.h>
#include <sys/types.h>
#include <sys/wait.h>
#include <unistd.h>
#ifndef MAP_FIXED_NOREPLACE
#define MAP_FIXED_NOREPLACE MAP_FIXED // "Should be safe" above 512TB
#endif
int main(void)
{
int p2c[2], c2p[2], rc, status, c, *p;
unsigned long page_size;
pid_t pid;
page_size = sysconf(_SC_PAGESIZE);
if (page_size != 65536) {
printf("Unsupported page size - not affected\n");
return 1;
}
// Create a mapping at 512TB to allocate an extended_id
p = mmap((void *)(512ul << 40), page_size, PROT_READ | PROT_WRITE,
MAP_PRIVATE | MAP_ANONYMOUS | MAP_FIXED_NOREPLACE, -1, 0);
if (p == MAP_FAILED) {
perror("mmap");
printf("Error: couldn't mmap(), confirm kernel has 4TB support\n");
return 1;
}
printf("parent writing %p = 1\n", p);
*p = 1;
assert(pipe(p2c) != -1 && pipe(c2p) != -1);
pid = fork();
if (pid == 0) {
close(p2c[1]);
close(c2p[0]);
assert(read(p2c[0], &c, 1) == 1);
pid = getpid();
printf("child writing %p = %d\n", p, pid);
*p = pid;
assert(write(c2p[1], &c, 1) == 1);
assert(read(p2c[0], &c, 1) == 1);
exit(0);
}
close(p2c[0]);
close(c2p[1]);
c = 0;
assert(write(p2c[1], &c, 1) == 1);
assert(read(c2p[0], &c, 1) == 1);
// Prevent compiler optimisation
asm volatile("" : : : "memory");
rc = 0;
printf("parent reading %p = %d\n", p, *p);
if (*p != 1) {
printf("Error: BUG! parent saw child's write! *p = %d\n", *p);
rc = 1;
}
assert(write(p2c[1], &c, 1) == 1);
assert(waitpid(pid, &status, 0) != -1);
assert(WIFEXITED(status) && WEXITSTATUS(status) == 0);
if (rc == 0)
printf("success: test completed OK\n");
return rc;
}
EOF
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]
^ permalink raw reply
* Re: [next][PowerPC] RCU stalls while booting linux-next on PowerVM LPAR
From: David Hildenbrand @ 2019-06-24 14:42 UTC (permalink / raw)
To: Sachin Sant, linuxppc-dev, Stephen Rothwell; +Cc: linux-next, linux-kernel
In-Reply-To: <0F3EAC10-0EA8-4097-99F2-BC42D92291FE@linux.vnet.ibm.com>
On 24.06.19 16:09, Sachin Sant wrote:
> Latest -next fails to boot on POWER9 PowerVM LPAR due to RCU stalls.
>
> This problem was introduced with next-20190620 (dc636f5d78).
> next-20190619 was last good kernel.
>
> Reverting following commit allows the kernel to boot.
> 2fd4aeea6b603 : mm/memory_hotplug: move and simplify walk_memory_blocks()
>
>
> [ 0.014409] Using shared cache scheduler topology
> [ 0.016302] devtmpfs: initialized
> [ 0.031022] clocksource: jiffies: mask: 0xffffffff max_cycles: 0xffffffff, max_idle_ns: 19112604462750000 ns
> [ 0.031034] futex hash table entries: 16384 (order: 5, 2097152 bytes, linear)
> [ 0.031575] NET: Registered protocol family 16
> [ 0.031724] audit: initializing netlink subsys (disabled)
> [ 0.031796] audit: type=2000 audit(1561344029.030:1): state=initialized audit_enabled=0 res=1
> [ 0.032249] cpuidle: using governor menu
> [ 0.032403] pstore: Registered nvram as persistent store backend
> [ 60.061246] rcu: INFO: rcu_sched self-detected stall on CPU
> [ 60.061254] rcu: 0-....: (5999 ticks this GP) idle=1ea/1/0x4000000000000002 softirq=5/5 fqs=2999
> [ 60.061261] (t=6000 jiffies g=-1187 q=0)
> [ 60.061265] NMI backtrace for cpu 0
> [ 60.061269] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.2.0-rc5-next-20190621-autotest-autotest #1
> [ 60.061275] Call Trace:
> [ 60.061280] [c0000018ee85f380] [c000000000b624ec] dump_stack+0xb0/0xf4 (unreliable)
> [ 60.061287] [c0000018ee85f3c0] [c000000000b6d464] nmi_cpu_backtrace+0x144/0x150
> [ 60.061293] [c0000018ee85f450] [c000000000b6d61c] nmi_trigger_cpumask_backtrace+0x1ac/0x1f0
> [ 60.061300] [c0000018ee85f4f0] [c0000000000692c8] arch_trigger_cpumask_backtrace+0x28/0x40
> [ 60.061306] [c0000018ee85f510] [c0000000001c5f90] rcu_dump_cpu_stacks+0x10c/0x16c
> [ 60.061313] [c0000018ee85f560] [c0000000001c4fe4] rcu_sched_clock_irq+0x744/0x990
> [ 60.061318] [c0000018ee85f630] [c0000000001d5b58] update_process_times+0x48/0x90
> [ 60.061325] [c0000018ee85f660] [c0000000001ea03c] tick_periodic+0x4c/0x120
> [ 60.061330] [c0000018ee85f690] [c0000000001ea150] tick_handle_periodic+0x40/0xe0
> [ 60.061336] [c0000018ee85f6d0] [c00000000002b5cc] timer_interrupt+0x10c/0x2e0
> [ 60.061342] [c0000018ee85f730] [c000000000009204] decrementer_common+0x134/0x140
> [ 60.061350] --- interrupt: 901 at replay_interrupt_return+0x0/0x4
> [ 60.061350] LR = arch_local_irq_restore+0x84/0x90
> [ 60.061357] [c0000018ee85fa30] [c0000018ee85fbac] 0xc0000018ee85fbac (unreliable)
> [ 60.061364] [c0000018ee85fa50] [c000000000b88300] _raw_spin_unlock_irqrestore+0x50/0x80
> [ 60.061369] [c0000018ee85fa70] [c000000000b69da4] klist_next+0xb4/0x150
> [ 60.061376] [c0000018ee85fac0] [c000000000766ea0] subsys_find_device_by_id+0xf0/0x1a0
> [ 60.061382] [c0000018ee85fb20] [c000000000797a94] walk_memory_blocks+0x84/0x100
> [ 60.061388] [c0000018ee85fb80] [c000000000795ea0] link_mem_sections+0x40/0x60
> [ 60.061395] [c0000018ee85fbb0] [c000000000f28c28] topology_init+0xa0/0x268
> [ 60.061400] [c0000018ee85fc10] [c000000000010448] do_one_initcall+0x68/0x2c0
> [ 60.061406] [c0000018ee85fce0] [c000000000f247dc] kernel_init_freeable+0x318/0x47c
> [ 60.061411] [c0000018ee85fdb0] [c0000000000107c4] kernel_init+0x24/0x150
> [ 60.061417] [c0000018ee85fe20] [c00000000000ba54] ret_from_kernel_thread+0x5c/0x68
> [ 88.016563] watchdog: BUG: soft lockup - CPU#0 stuck for 22s! [swapper/0:1]
> [ 88.016569] Modules linked in:
>
Hi, thanks! Please see
https://lkml.org/lkml/2019/6/21/600
and especially
https://lkml.org/lkml/2019/6/21/908
Does this fix your problem? The fix is on its way to next.
Cheers!
--
Thanks,
David / dhildenb
^ permalink raw reply
* [PATCH] ocxl: Fix concurrent AFU open and device removal
From: Frederic Barrat @ 2019-06-24 14:41 UTC (permalink / raw)
To: linuxppc-dev, andrew.donnellan, clombard, groug, alastair
If an ocxl device is unbound through sysfs at the same time its AFU is
being opened by a user process, the open code may dereference freed
stuctures, which can lead to kernel oops messages. You'd have to hit a
tiny time window, but it's possible. It's fairly easy to test by
making the time window bigger artificially.
Fix it with a combination of 2 changes:
- when an AFU device is found in the IDR by looking for the device
minor number, we should hold a reference on the device until after the
context is allocated. A reference on the AFU structure is kept when
the context is allocated, so we can release the reference on the
device after the context allocation.
- with the fix above, there's still another even tinier window,
between the time the AFU device is found in the IDR and the reference
on the device is taken. We can fix this one by removing the IDR entry
earlier, when the device setup is removed, instead of waiting for the
'release' device callback. With proper locking around the IDR.
Fixes: 75ca758adbaf ("ocxl: Create a clear delineation between ocxl backend & frontend")
Signed-off-by: Frederic Barrat <fbarrat@linux.ibm.com>
---
mpe: this fixes a commit merged in v5.2-rc1. It's late, and I don't think it's that important. If it's for the next merge window, I would add:
Cc: stable@vger.kernel.org # v5.2
drivers/misc/ocxl/file.c | 23 +++++++++++------------
1 file changed, 11 insertions(+), 12 deletions(-)
diff --git a/drivers/misc/ocxl/file.c b/drivers/misc/ocxl/file.c
index 2870c25da166..4d1b44de1492 100644
--- a/drivers/misc/ocxl/file.c
+++ b/drivers/misc/ocxl/file.c
@@ -18,18 +18,15 @@ static struct class *ocxl_class;
static struct mutex minors_idr_lock;
static struct idr minors_idr;
-static struct ocxl_file_info *find_file_info(dev_t devno)
+static struct ocxl_file_info *find_and_get_file_info(dev_t devno)
{
struct ocxl_file_info *info;
- /*
- * We don't declare an RCU critical section here, as our AFU
- * is protected by a reference counter on the device. By the time the
- * info reference is removed from the idr, the ref count of
- * the device is already at 0, so no user API will access that AFU and
- * this function can't return it.
- */
+ mutex_lock(&minors_idr_lock);
info = idr_find(&minors_idr, MINOR(devno));
+ if (info)
+ get_device(&info->dev);
+ mutex_unlock(&minors_idr_lock);
return info;
}
@@ -58,14 +55,16 @@ static int afu_open(struct inode *inode, struct file *file)
pr_debug("%s for device %x\n", __func__, inode->i_rdev);
- info = find_file_info(inode->i_rdev);
+ info = find_and_get_file_info(inode->i_rdev);
if (!info)
return -ENODEV;
rc = ocxl_context_alloc(&ctx, info->afu, inode->i_mapping);
- if (rc)
+ if (rc) {
+ put_device(&info->dev);
return rc;
-
+ }
+ put_device(&info->dev);
file->private_data = ctx;
return 0;
}
@@ -487,7 +486,6 @@ static void info_release(struct device *dev)
{
struct ocxl_file_info *info = container_of(dev, struct ocxl_file_info, dev);
- free_minor(info);
ocxl_afu_put(info->afu);
kfree(info);
}
@@ -577,6 +575,7 @@ void ocxl_file_unregister_afu(struct ocxl_afu *afu)
ocxl_file_make_invisible(info);
ocxl_sysfs_unregister_afu(info);
+ free_minor(info);
device_unregister(&info->dev);
}
--
2.21.0
^ permalink raw reply related
* [next][PowerPC] RCU stalls while booting linux-next on PowerVM LPAR
From: Sachin Sant @ 2019-06-24 14:09 UTC (permalink / raw)
To: linuxppc-dev, david, Stephen Rothwell; +Cc: linux-next, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 3443 bytes --]
Latest -next fails to boot on POWER9 PowerVM LPAR due to RCU stalls.
This problem was introduced with next-20190620 (dc636f5d78).
next-20190619 was last good kernel.
Reverting following commit allows the kernel to boot.
2fd4aeea6b603 : mm/memory_hotplug: move and simplify walk_memory_blocks()
[ 0.014409] Using shared cache scheduler topology
[ 0.016302] devtmpfs: initialized
[ 0.031022] clocksource: jiffies: mask: 0xffffffff max_cycles: 0xffffffff, max_idle_ns: 19112604462750000 ns
[ 0.031034] futex hash table entries: 16384 (order: 5, 2097152 bytes, linear)
[ 0.031575] NET: Registered protocol family 16
[ 0.031724] audit: initializing netlink subsys (disabled)
[ 0.031796] audit: type=2000 audit(1561344029.030:1): state=initialized audit_enabled=0 res=1
[ 0.032249] cpuidle: using governor menu
[ 0.032403] pstore: Registered nvram as persistent store backend
[ 60.061246] rcu: INFO: rcu_sched self-detected stall on CPU
[ 60.061254] rcu: 0-....: (5999 ticks this GP) idle=1ea/1/0x4000000000000002 softirq=5/5 fqs=2999
[ 60.061261] (t=6000 jiffies g=-1187 q=0)
[ 60.061265] NMI backtrace for cpu 0
[ 60.061269] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.2.0-rc5-next-20190621-autotest-autotest #1
[ 60.061275] Call Trace:
[ 60.061280] [c0000018ee85f380] [c000000000b624ec] dump_stack+0xb0/0xf4 (unreliable)
[ 60.061287] [c0000018ee85f3c0] [c000000000b6d464] nmi_cpu_backtrace+0x144/0x150
[ 60.061293] [c0000018ee85f450] [c000000000b6d61c] nmi_trigger_cpumask_backtrace+0x1ac/0x1f0
[ 60.061300] [c0000018ee85f4f0] [c0000000000692c8] arch_trigger_cpumask_backtrace+0x28/0x40
[ 60.061306] [c0000018ee85f510] [c0000000001c5f90] rcu_dump_cpu_stacks+0x10c/0x16c
[ 60.061313] [c0000018ee85f560] [c0000000001c4fe4] rcu_sched_clock_irq+0x744/0x990
[ 60.061318] [c0000018ee85f630] [c0000000001d5b58] update_process_times+0x48/0x90
[ 60.061325] [c0000018ee85f660] [c0000000001ea03c] tick_periodic+0x4c/0x120
[ 60.061330] [c0000018ee85f690] [c0000000001ea150] tick_handle_periodic+0x40/0xe0
[ 60.061336] [c0000018ee85f6d0] [c00000000002b5cc] timer_interrupt+0x10c/0x2e0
[ 60.061342] [c0000018ee85f730] [c000000000009204] decrementer_common+0x134/0x140
[ 60.061350] --- interrupt: 901 at replay_interrupt_return+0x0/0x4
[ 60.061350] LR = arch_local_irq_restore+0x84/0x90
[ 60.061357] [c0000018ee85fa30] [c0000018ee85fbac] 0xc0000018ee85fbac (unreliable)
[ 60.061364] [c0000018ee85fa50] [c000000000b88300] _raw_spin_unlock_irqrestore+0x50/0x80
[ 60.061369] [c0000018ee85fa70] [c000000000b69da4] klist_next+0xb4/0x150
[ 60.061376] [c0000018ee85fac0] [c000000000766ea0] subsys_find_device_by_id+0xf0/0x1a0
[ 60.061382] [c0000018ee85fb20] [c000000000797a94] walk_memory_blocks+0x84/0x100
[ 60.061388] [c0000018ee85fb80] [c000000000795ea0] link_mem_sections+0x40/0x60
[ 60.061395] [c0000018ee85fbb0] [c000000000f28c28] topology_init+0xa0/0x268
[ 60.061400] [c0000018ee85fc10] [c000000000010448] do_one_initcall+0x68/0x2c0
[ 60.061406] [c0000018ee85fce0] [c000000000f247dc] kernel_init_freeable+0x318/0x47c
[ 60.061411] [c0000018ee85fdb0] [c0000000000107c4] kernel_init+0x24/0x150
[ 60.061417] [c0000018ee85fe20] [c00000000000ba54] ret_from_kernel_thread+0x5c/0x68
[ 88.016563] watchdog: BUG: soft lockup - CPU#0 stuck for 22s! [swapper/0:1]
[ 88.016569] Modules linked in:
Thanks
-Sachin
[-- Attachment #2: boot.log --]
[-- Type: application/octet-stream, Size: 11416 bytes --]
[ 0.000000] hash-mmu: Page sizes from device-tree:
[ 0.000000] hash-mmu: base_shift=12: shift=12, sllp=0x0000, avpnm=0x00000000, tlbiel=1, penc=0
[ 0.000000] hash-mmu: base_shift=12: shift=16, sllp=0x0000, avpnm=0x00000000, tlbiel=1, penc=7
[ 0.000000] hash-mmu: base_shift=12: shift=24, sllp=0x0000, avpnm=0x00000000, tlbiel=1, penc=56
[ 0.000000] hash-mmu: base_shift=16: shift=16, sllp=0x0110, avpnm=0x00000000, tlbiel=1, penc=1
[ 0.000000] hash-mmu: base_shift=16: shift=24, sllp=0x0110, avpnm=0x00000000, tlbiel=1, penc=8
[ 0.000000] hash-mmu: base_shift=24: shift=24, sllp=0x0100, avpnm=0x00000001, tlbiel=0, penc=0
[ 0.000000] hash-mmu: base_shift=34: shift=34, sllp=0x0120, avpnm=0x000007ff, tlbiel=0, penc=3
[ 0.000000] Using 1TB segments
[ 0.000000] hash-mmu: Initializing hash mmu with SLB
[ 0.000000] Linux version 5.2.0-rc5-next-20190620-autotest (root@ltczep10-lp3.aus.stglabs.ibm.com) (gcc version 4.8.5 20150623 (Red Hat 4.8.5-36) (GCC)) #1 SMP Mon Jun 24 06:38:12 PDT 2019
[ 0.000000] Found initrd at 0xc0000000032b0000:0xc0000000044c4efc
[ 0.000000] Using pSeries machine description
[ 0.000000] printk: bootconsole [udbg0] enabled
[ 0.000000] Partition configured for 40 cpus.
[ 0.000000] CPU maps initialized for 8 threads per core
[ 0.000000] -----------------------------------------------------
[ 0.000000] phys_mem_size = 0x1900000000
[ 0.000000] dcache_bsize = 0x80
[ 0.000000] icache_bsize = 0x80
[ 0.000000] cpu_features = 0x0000c07f8f5f91a7
[ 0.000000] possible = 0x0000fbffcf5fb1a7
[ 0.000000] always = 0x00000003800081a1
[ 0.000000] cpu_user_features = 0xdc0065c2 0xefe00000
[ 0.000000] mmu_features = 0x7c006001
[ 0.000000] firmware_features = 0x00000013c45bfc57
[ 0.000000] hash-mmu: ppc64_pft_size = 0x1e
[ 0.000000] hash-mmu: htab_hash_mask = 0x7fffff
[ 0.000000] hash-mmu: kernel vmalloc start = 0xc008000000000000
[ 0.000000] hash-mmu: kernel IO start = 0xc00a000000000000
[ 0.000000] hash-mmu: kernel vmemmap start = 0xc00c000000000000
[ 0.000000] -----------------------------------------------------
[ 0.000000] numa: NODE_DATA [mem 0x18ffe98980-0x18ffe9ffff]
[ 0.000000] numa: NODE_DATA(0) on node 3
[ 0.000000] numa: NODE_DATA [mem 0x18ffe91300-0x18ffe9897f]
[ 0.000000] rfi-flush: fallback displacement flush available
[ 0.000000] rfi-flush: mttrig type flush available
[ 0.000000] count-cache-flush: full software flush sequence enabled.
[ 0.000000] stf-barrier: eieio barrier available
[ 0.000000] PPC64 nvram contains 15360 bytes
[ 0.000000] barrier-nospec: using ORI speculation barrier
[ 0.000000] Zone ranges:
[ 0.000000] Normal [mem 0x0000000000000000-0x00000018ffffffff]
[ 0.000000] Device empty
[ 0.000000] Movable zone start for each node
[ 0.000000] Early memory node ranges
[ 0.000000] node 3: [mem 0x0000000000000000-0x00000018ffffffff]
[ 0.000000] Initmem setup node 0 [mem 0x0000000000000000-0x0000000000000000]
[ 0.000000] Initmem setup node 3 [mem 0x0000000000000000-0x00000018ffffffff]
[ 0.000000] percpu: Embedded 11 pages/cpu s631960 r0 d88936 u1048576
[ 0.000000] node[0] zonelist: 3:Normal
[ 0.000000] node[3] zonelist: 3:Normal
[ 0.000000] Built 2 zonelists, mobility grouping on. Total pages: 1636800
[ 0.000000] Policy zone: Normal
[ 0.000000] Kernel command line: root=UUID=5b503143-3264-4a73-bd1b-2b1cd7d250f6
[ 0.000000] Dentry cache hash table entries: 8388608 (order: 10, 67108864 bytes, linear)
[ 0.000000] Inode-cache hash table entries: 4194304 (order: 9, 33554432 bytes, linear)
[ 0.000000] mem auto-init: stack:off, heap alloc:off, heap free:off
[ 0.000000] Memory: 104547200K/104857600K available (11840K kernel code, 1792K rwdata, 3584K rodata, 4992K init, 3850K bss, 310400K reserved, 0K cma-reserved)
[ 0.000000] random: get_random_u64 called from cache_random_seq_create+0xa4/0x1c0 with crng_init=0
[ 0.000000] SLUB: HWalign=128, Order=0-3, MinObjects=0, CPUs=40, Nodes=32
[ 0.000000] ftrace: allocating 31746 entries in 12 pages
[ 0.000000] rcu: Hierarchical RCU implementation.
[ 0.000000] rcu: RCU restricting CPUs from NR_CPUS=2048 to nr_cpu_ids=40.
[ 0.000000] rcu: RCU calculated value of scheduler-enlistment delay is 10 jiffies.
[ 0.000000] rcu: Adjusting geometry for rcu_fanout_leaf=16, nr_cpu_ids=40
[ 0.000000] NR_IRQS: 512, nr_irqs: 512, preallocated irqs: 16
[ 0.000000] rcu: Offload RCU callbacks from CPUs: (none).
[ 0.000002] time_init: 56 bit decrementer (max: 7fffffffffffff)
[ 0.000067] clocksource: timebase: mask: 0xffffffffffffffff max_cycles: 0x761537d007, max_idle_ns: 440795202126 ns
[ 0.000178] clocksource: timebase mult[1f40000] shift[24] registered
[ 0.000312] Console: colour dummy device 80x25
[ 0.000363] printk: console [hvc0] enabled
[ 0.000363] printk: console [hvc0] enabled
[ 0.000411] printk: bootconsole [udbg0] disabled
[ 0.000411] printk: bootconsole [udbg0] disabled
[ 0.000508] mempolicy: Enabling automatic NUMA balancing. Configure with numa_balancing= or the kernel.numa_balancing sysctl
[ 0.000521] pid_max: default: 40960 minimum: 320
[ 0.000673] LSM: Security Framework initializing
[ 0.000759] Yama: becoming mindful.
[ 0.000781] SELinux: Initializing.
[ 0.000944] *** VALIDATE SELinux ***
[ 0.001035] Mount-cache hash table entries: 131072 (order: 4, 1048576 bytes, linear)
[ 0.001073] Mountpoint-cache hash table entries: 131072 (order: 4, 1048576 bytes, linear)
[ 0.001370] *** VALIDATE proc ***
[ 0.001623] *** VALIDATE cgroup1 ***
[ 0.001629] *** VALIDATE cgroup2 ***
[ 0.002225] EEH: pSeries platform initialized
[ 0.002233] POWER9 performance monitor hardware support registered
[ 0.002272] rcu: Hierarchical SRCU implementation.
[ 0.003078] smp: Bringing up secondary CPUs ...
[ 0.014555] smp: Brought up 2 nodes, 40 CPUs
[ 0.014564] numa: Node 0 CPUs:
[ 0.014567] numa: Node 3 CPUs: 0-39
[ 0.014571] Using small cores at SMT level
[ 0.014575] Using shared cache scheduler topology
[ 0.016465] devtmpfs: initialized
[ 0.031046] clocksource: jiffies: mask: 0xffffffff max_cycles: 0xffffffff, max_idle_ns: 19112604462750000 ns
[ 0.031058] futex hash table entries: 16384 (order: 5, 2097152 bytes, linear)
[ 0.031602] NET: Registered protocol family 16
[ 0.031751] audit: initializing netlink subsys (disabled)
[ 0.031822] audit: type=2000 audit(1561384091.030:1): state=initialized audit_enabled=0 res=1
[ 0.032275] cpuidle: using governor menu
[ 0.032429] pstore: Registered nvram as persistent store backend
[ 60.061325] rcu: INFO: rcu_sched self-detected stall on CPU
[ 60.061333] rcu: 0-....: (5999 ticks this GP) idle=1ca/1/0x4000000000000002 softirq=5/5 fqs=2999
[ 60.061340] (t=6000 jiffies g=-1187 q=0)
[ 60.061343] NMI backtrace for cpu 0
[ 60.061349] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.2.0-rc5-next-20190620-autotest #1
[ 60.061354] Call Trace:
[ 60.061358] [c0000018ee813360] [c000000000b625cc] dump_stack+0xb0/0xf4 (unreliable)
[ 60.061365] [c0000018ee8133a0] [c000000000b6d544] nmi_cpu_backtrace+0x144/0x150
[ 60.061371] [c0000018ee813430] [c000000000b6d6fc] nmi_trigger_cpumask_backtrace+0x1ac/0x1f0
[ 60.061378] [c0000018ee8134d0] [c0000000000692c8] arch_trigger_cpumask_backtrace+0x28/0x40
[ 60.061385] [c0000018ee8134f0] [c0000000001c6210] rcu_dump_cpu_stacks+0x10c/0x16c
[ 60.061391] [c0000018ee813540] [c0000000001c5264] rcu_sched_clock_irq+0x744/0x990
[ 60.061397] [c0000018ee813610] [c0000000001d5e38] update_process_times+0x48/0x90
[ 60.061403] [c0000018ee813640] [c0000000001ea31c] tick_periodic+0x4c/0x120
[ 60.061408] [c0000018ee813670] [c0000000001ea430] tick_handle_periodic+0x40/0xe0
[ 60.061414] [c0000018ee8136b0] [c00000000002b5cc] timer_interrupt+0x10c/0x2e0
[ 60.061420] [c0000018ee813710] [c000000000009204] decrementer_common+0x134/0x140
[ 60.061427] --- interrupt: 901 at replay_interrupt_return+0x0/0x4
[ 60.061427] LR = arch_local_irq_restore+0x84/0x90
[ 60.061434] [c0000018ee813a10] [c0000018ee813bac] 0xc0000018ee813bac (unreliable)
[ 60.061441] [c0000018ee813a30] [c000000000b883e0] _raw_spin_unlock_irqrestore+0x50/0x80
[ 60.061447] [c0000018ee813a50] [c000000000b69e84] klist_next+0xb4/0x150
[ 60.061453] [c0000018ee813aa0] [c000000000766f60] subsys_find_device_by_id+0xf0/0x1a0
[ 60.061459] [c0000018ee813b00] [c000000000796464] find_memory_block_by_id+0x94/0xb0
[ 60.061466] [c0000018ee813b30] [c000000000797b1c] walk_memory_blocks+0x7c/0xf0
[ 60.061472] [c0000018ee813b80] [c000000000795f60] link_mem_sections+0x40/0x60
[ 60.061478] [c0000018ee813bb0] [c000000000f28c28] topology_init+0xa0/0x268
[ 60.061483] [c0000018ee813c10] [c000000000010448] do_one_initcall+0x68/0x2c0
[ 60.061489] [c0000018ee813ce0] [c000000000f247dc] kernel_init_freeable+0x318/0x47c
[ 60.061495] [c0000018ee813db0] [c0000000000107c4] kernel_init+0x24/0x150
[ 60.061500] [c0000018ee813e20] [c00000000000ba54] ret_from_kernel_thread+0x5c/0x68
[ 88.016703] watchdog: BUG: soft lockup - CPU#0 stuck for 22s! [swapper/0:1]
[ 88.016709] Modules linked in:
[ 88.016713] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.2.0-rc5-next-20190620-autotest #1
[ 88.016718] NIP: c00000000000ab8c LR: c00000000001b434 CTR: 0000000000000000
[ 88.016724] REGS: c0000018ee813780 TRAP: 0901 Not tainted (5.2.0-rc5-next-20190620-autotest)
[ 88.016730] MSR: 8000000002009033 <SF,VEC,EE,ME,IR,DR,RI,LE> CR: 44008484 XER: 00000005
[ 88.016738] CFAR: c000000000b69e30 IRQMASK: 0
[ 88.016738] GPR00: c000000000b883e0 c0000018ee813a10 c000000001544d00 0000000000000900
[ 88.016738] GPR04: 0000000000000000 0000000000000000 0000000000000001 0000000000000004
[ 88.016738] GPR08: 0000000000000000 c0000018eb436d80 0000000000000001 0000000000000220
[ 88.016738] GPR12: 0000000000000000 c000000001990000
[ 88.016761] NIP [c00000000000ab8c] replay_interrupt_return+0x0/0x4
[ 88.016767] LR [c00000000001b434] arch_local_irq_restore+0x84/0x90
[ 88.016771] Call Trace:
[ 88.016774] [c0000018ee813a10] [c0000018ee813bac] 0xc0000018ee813bac (unreliable)
[ 88.016780] [c0000018ee813a30] [c000000000b883e0] _raw_spin_unlock_irqrestore+0x50/0x80
[ 88.016786] [c0000018ee813a50] [c000000000b69e84] klist_next+0xb4/0x150
[ 88.016792] [c0000018ee813aa0] [c000000000766f60] subsys_find_device_by_id+0xf0/0x1a0
[ 88.016798] [c0000018ee813b00] [c000000000796464] find_memory_block_by_id+0x94/0xb0
[ 88.016804] [c0000018ee813b30] [c000000000797b1c] walk_memory_blocks+0x7c/0xf0
[ 88.016810] [c0000018ee813b80] [c000000000795f60] link_mem_sections+0x40/0x60
[ 88.016816] [c0000018ee813bb0] [c000000000f28c28] topology_init+0xa0/0x268
[ 88.016821] [c0000018ee813c10] [c000000000010448] do_one_initcall+0x68/0x2c0
[ 88.016827] [c0000018ee813ce0] [c000000000f247dc] kernel_init_freeable+0x318/0x47c
[ 88.016833] [c0000018ee813db0] [c0000000000107c4] kernel_init+0x24/0x150
[ 88.016838] [c0000018ee813e20] [c00000000000ba54] ret_from_kernel_thread+0x5c/0x68
[ 88.016843] Instruction dump:
[ 88.016847] 7d200026 618c8000 2c030900 4182e568 2c030500 4182e010 2c030f00 4182f2d8
[ 88.016854] 2c030a00 4182ffb8 60000000 60000000 <4e800020> 7c781b78 480003e1 480003f9
^ permalink raw reply
* Re: [PATCH] KVM: PPC: Book3S HV: Fix CR0 setting in TM emulation
From: Michael Ellerman @ 2019-06-24 11:48 UTC (permalink / raw)
To: Michael Neuling; +Cc: mikey, linuxppc-dev, sjitindarsingh, kvm-ppc
In-Reply-To: <20190620060040.26945-1-mikey@neuling.org>
Michael Neuling <mikey@neuling.org> writes:
> When emulating tsr, treclaim and trechkpt, we incorrectly set CR0. The
> code currently sets:
> CR0 <- 00 || MSR[TS]
> but according to the ISA it should be:
> CR0 <- 0 || MSR[TS] || 0
Seems bad, what's the worst case impact?
Do we have a test case for this?
> This fixes the bit shift to put the bits in the correct location.
Fixes: ?
cheers
> diff --git a/arch/powerpc/kvm/book3s_hv_tm.c b/arch/powerpc/kvm/book3s_hv_tm.c
> index 888e2609e3..31cd0f327c 100644
> --- a/arch/powerpc/kvm/book3s_hv_tm.c
> +++ b/arch/powerpc/kvm/book3s_hv_tm.c
> @@ -131,7 +131,7 @@ int kvmhv_p9_tm_emulation(struct kvm_vcpu *vcpu)
> }
> /* Set CR0 to indicate previous transactional state */
> vcpu->arch.regs.ccr = (vcpu->arch.regs.ccr & 0x0fffffff) |
> - (((msr & MSR_TS_MASK) >> MSR_TS_S_LG) << 28);
> + (((msr & MSR_TS_MASK) >> MSR_TS_S_LG) << 29);
> /* L=1 => tresume, L=0 => tsuspend */
> if (instr & (1 << 21)) {
> if (MSR_TM_SUSPENDED(msr))
> @@ -175,7 +175,7 @@ int kvmhv_p9_tm_emulation(struct kvm_vcpu *vcpu)
>
> /* Set CR0 to indicate previous transactional state */
> vcpu->arch.regs.ccr = (vcpu->arch.regs.ccr & 0x0fffffff) |
> - (((msr & MSR_TS_MASK) >> MSR_TS_S_LG) << 28);
> + (((msr & MSR_TS_MASK) >> MSR_TS_S_LG) << 29);
> vcpu->arch.shregs.msr &= ~MSR_TS_MASK;
> return RESUME_GUEST;
>
> @@ -205,7 +205,7 @@ int kvmhv_p9_tm_emulation(struct kvm_vcpu *vcpu)
>
> /* Set CR0 to indicate previous transactional state */
> vcpu->arch.regs.ccr = (vcpu->arch.regs.ccr & 0x0fffffff) |
> - (((msr & MSR_TS_MASK) >> MSR_TS_S_LG) << 28);
> + (((msr & MSR_TS_MASK) >> MSR_TS_S_LG) << 29);
> vcpu->arch.shregs.msr = msr | MSR_TS_S;
> return RESUME_GUEST;
> }
> --
> 2.21.0
^ permalink raw reply
* Re: [PATCH 1/2] powerpc/64s: Rename PPC_INVALIDATE_ERAT to PPC_ARCH_300_INVALIDATE_ERAT
From: Michael Ellerman @ 2019-06-24 11:39 UTC (permalink / raw)
To: Nicholas Piggin, Segher Boessenkool; +Cc: linuxppc-dev, kvm-ppc
In-Reply-To: <1561297021.pyb7y0yjt7.astroid@bobo.none>
Nicholas Piggin <npiggin@gmail.com> writes:
> Segher Boessenkool's on June 23, 2019 10:03 pm:
>> On Sun, Jun 23, 2019 at 08:41:51PM +1000, Nicholas Piggin wrote:
>>> This makes it clear to the caller that it can only be used on POWER9
>>> and later CPUs.
>>
>>> -#define PPC_INVALIDATE_ERAT PPC_SLBIA(7)
>>> +#define PPC_ARCH_300_INVALIDATE_ERAT PPC_SLBIA(7)
>>
>> The architecture version is 3.0 (or 3.0B), not "300". This will work on
>> implementations of later architecture versions as well, so maybe the name
>> isn't so great anyway?
>
> Yeah... this is kernel convention for better or worse. ISA v3.0B
> feature support is called CPU_FTR_ARCH_300, and later architectures
> will advertise that support. For the most part we can use architected
> features (incompatible changes would require additional code).
I'd rather we used 3_0B or something inside the kernel, but I'm not sure
it's worth the churn to rename the existing feature everywhere. And we
can't rename the user visible feature.
But if you're adding a new usage then I'd prefer: PPC_ISA_3_0B_INVALIDATE_ERAT
I dislike "300" because it implies we support ISA v3.0 but we actually
don't, we only support v3.0B.
cheers
^ permalink raw reply
* Re: [PATCH 4/5] x86: don't use asm-generic/ptrace.h
From: Thomas Gleixner @ 2019-06-24 7:22 UTC (permalink / raw)
To: Christoph Hellwig
Cc: linux-arch, Arnd Bergmann, linux-sh, linux-kernel, x86,
Oleg Nesterov, linux-mips, linuxppc-dev, Ingo Molnar,
linux-arm-kernel
In-Reply-To: <20190624054728.30966-5-hch@lst.de>
On Mon, 24 Jun 2019, Christoph Hellwig wrote:
> Doing the indirection through macros for the regs accessors just
> makes them harder to read, so implement the helpers directly.
>
> Note that only the helpers actually used are implemented now.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> Acked-by: Ingo Molnar <mingo@kernel.org>
> Acked-by: Oleg Nesterov <oleg@redhat.com>
Reviewed-by: Thomas Gleixner <tglx@linutronix.de>
^ permalink raw reply
* Re: remove asm-generic/ptrace.h v3
From: Thomas Gleixner @ 2019-06-24 7:23 UTC (permalink / raw)
To: Christoph Hellwig
Cc: linux-arch, Arnd Bergmann, linux-sh, linux-kernel, x86,
Oleg Nesterov, linux-mips, linuxppc-dev, linux-arm-kernel
In-Reply-To: <20190624054728.30966-1-hch@lst.de>
On Mon, 24 Jun 2019, Christoph Hellwig wrote:
>
> asm-generic/ptrace.h is a little weird in that it doesn't actually
> implement any functionality, but it provided multiple layers of macros
> that just implement trivial inline functions. We implement those
> directly in the few architectures and be off with a much simpler
> design.
>
> I'm not sure which tree is the right place, but may this can go through
> the asm-generic tree since it removes an asm-generic header?
Makes sense.
Thanks,
tglx
^ permalink raw reply
* [PATCH v5 0/4] Additional fixes on Talitos driver
From: Christophe Leroy @ 2019-06-24 7:21 UTC (permalink / raw)
To: Herbert Xu, David S. Miller, horia.geanta
Cc: linuxppc-dev, linux-crypto, linux-kernel
This series is the last set of fixes for the Talitos driver.
We now get a fully clean boot on both SEC1 (SEC1.2 on mpc885) and
SEC2 (SEC2.2 on mpc8321E) with CONFIG_CRYPTO_MANAGER_EXTRA_TESTS:
[ 3.385197] bus: 'platform': really_probe: probing driver talitos with device ff020000.crypto
[ 3.450982] random: fast init done
[ 12.252548] alg: No test for authenc(hmac(md5),cbc(aes)) (authenc-hmac-md5-cbc-aes-talitos-hsna)
[ 12.262226] alg: No test for authenc(hmac(md5),cbc(des3_ede)) (authenc-hmac-md5-cbc-3des-talitos-hsna)
[ 43.310737] Bug in SEC1, padding ourself
[ 45.603318] random: crng init done
[ 54.612333] talitos ff020000.crypto: fsl,sec1.2 algorithms registered in /proc/crypto
[ 54.620232] driver: 'talitos': driver_bound: bound to device 'ff020000.crypto'
[ 1.193721] bus: 'platform': really_probe: probing driver talitos with device b0030000.crypto
[ 1.229197] random: fast init done
[ 2.714920] alg: No test for authenc(hmac(sha224),cbc(aes)) (authenc-hmac-sha224-cbc-aes-talitos)
[ 2.724312] alg: No test for authenc(hmac(sha224),cbc(aes)) (authenc-hmac-sha224-cbc-aes-talitos-hsna)
[ 4.482045] alg: No test for authenc(hmac(md5),cbc(aes)) (authenc-hmac-md5-cbc-aes-talitos)
[ 4.490940] alg: No test for authenc(hmac(md5),cbc(aes)) (authenc-hmac-md5-cbc-aes-talitos-hsna)
[ 4.500280] alg: No test for authenc(hmac(md5),cbc(des3_ede)) (authenc-hmac-md5-cbc-3des-talitos)
[ 4.509727] alg: No test for authenc(hmac(md5),cbc(des3_ede)) (authenc-hmac-md5-cbc-3des-talitos-hsna)
[ 6.631781] random: crng init done
[ 11.521795] talitos b0030000.crypto: fsl,sec2.2 algorithms registered in /proc/crypto
[ 11.529803] driver: 'talitos': driver_bound: bound to device 'b0030000.crypto'
v2: dropped patch 1 which was irrelevant due to a rebase weirdness. Added Cc to stable on the 2 first patches.
v3:
- removed stable reference in patch 1
- reworded patch 1 to include name of patch 2 for the dependency.
- mentionned this dependency in patch 2 as well.
- corrected the Fixes: sha1 in patch 4
v4:
- using scatterwalk_ffwd() instead of opencodying SG list forwarding.
- Added a patch to fix sg_copy_to_buffer() when sg->offset() is greater than PAGE_SIZE,
otherwise sg_copy_to_buffer() fails when the list has been forwarded with scatterwalk_ffwd().
- taken the patch "crypto: talitos - eliminate unneeded 'done' functions at build time"
out of the series because it is independent.
- added a helper to find the header field associated to a request in flush_channe()
v5:
- Replacing the while loop by a direct shift/mask operation, as suggested by Herbert in patch 1.
Christophe Leroy (4):
lib/scatterlist: Fix mapping iterator when sg->offset is greater than
PAGE_SIZE
crypto: talitos - move struct talitos_edesc into talitos.h
crypto: talitos - fix hash on SEC1.
crypto: talitos - drop icv_ool
drivers/crypto/talitos.c | 102 +++++++++++++++++++----------------------------
drivers/crypto/talitos.h | 28 +++++++++++++
lib/scatterlist.c | 9 +++--
3 files changed, 74 insertions(+), 65 deletions(-)
--
2.13.3
^ permalink raw reply
* [PATCH v5 3/4] crypto: talitos - fix hash on SEC1.
From: Christophe Leroy @ 2019-06-24 7:20 UTC (permalink / raw)
To: Herbert Xu, David S. Miller, horia.geanta
Cc: linuxppc-dev, linux-crypto, linux-kernel
In-Reply-To: <cover.1561360551.git.christophe.leroy@c-s.fr>
On SEC1, hash provides wrong result when performing hashing in several
steps with input data SG list has more than one element. This was
detected with CONFIG_CRYPTO_MANAGER_EXTRA_TESTS:
[ 44.185947] alg: hash: md5-talitos test failed (wrong result) on test vector 6, cfg="random: may_sleep use_finup src_divs=[<reimport>25.88%@+8063, <flush>24.19%@+9588, 28.63%@+16333, <reimport>4.60%@+6756, 16.70%@+16281] dst_divs=[71.61%@alignmask+16361, 14.36%@+7756, 14.3%@+"
[ 44.325122] alg: hash: sha1-talitos test failed (wrong result) on test vector 3, cfg="random: inplace use_final src_divs=[<flush,nosimd>16.56%@+16378, <reimport>52.0%@+16329, 21.42%@alignmask+16380, 10.2%@alignmask+16380] iv_offset=39"
[ 44.493500] alg: hash: sha224-talitos test failed (wrong result) on test vector 4, cfg="random: use_final nosimd src_divs=[<reimport>52.27%@+7401, <reimport>17.34%@+16285, <flush>17.71%@+26, 12.68%@+10644] iv_offset=43"
[ 44.673262] alg: hash: sha256-talitos test failed (wrong result) on test vector 4, cfg="random: may_sleep use_finup src_divs=[<reimport>60.6%@+12790, 17.86%@+1329, <reimport>12.64%@alignmask+16300, 8.29%@+15, 0.40%@+13506, <reimport>0.51%@+16322, <reimport>0.24%@+16339] dst_divs"
This is due to two issues:
- We have an overlap between the buffer used for copying the input
data (SEC1 doesn't do scatter/gather) and the chained descriptor.
- Data copy is wrong when the previous hash left less than one
blocksize of data to hash, implying a complement of the previous
block with a few bytes from the new request.
Fix it by:
- Moving the second descriptor after the buffer, as moving the buffer
after the descriptor would make it more complex for other cipher
operations (AEAD, ABLKCIPHER)
- Skip the bytes taken from the new request to complete the previous
one by moving the SG list forward.
Fixes: 37b5e8897eb5 ("crypto: talitos - chain in buffered data for ahash on SEC1")
Cc: stable@vger.kernel.org
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
drivers/crypto/talitos.c | 69 ++++++++++++++++++++++++++++--------------------
1 file changed, 41 insertions(+), 28 deletions(-)
diff --git a/drivers/crypto/talitos.c b/drivers/crypto/talitos.c
index 95dc3957f358..ab6bd45addf7 100644
--- a/drivers/crypto/talitos.c
+++ b/drivers/crypto/talitos.c
@@ -320,6 +320,21 @@ static int talitos_submit(struct device *dev, int ch, struct talitos_desc *desc,
return -EINPROGRESS;
}
+static __be32 get_request_hdr(struct talitos_request *request, bool is_sec1)
+{
+ struct talitos_edesc *edesc;
+
+ if (!is_sec1)
+ return request->desc->hdr;
+
+ if (!request->desc->next_desc)
+ return request->desc->hdr1;
+
+ edesc = container_of(request->desc, struct talitos_edesc, desc);
+
+ return ((struct talitos_desc *)(edesc->buf + edesc->dma_len))->hdr1;
+}
+
/*
* process what was done, notify callback of error if not
*/
@@ -341,12 +356,7 @@ static void flush_channel(struct device *dev, int ch, int error, int reset_ch)
/* descriptors with their done bits set don't get the error */
rmb();
- if (!is_sec1)
- hdr = request->desc->hdr;
- else if (request->desc->next_desc)
- hdr = (request->desc + 1)->hdr1;
- else
- hdr = request->desc->hdr1;
+ hdr = get_request_hdr(request, is_sec1);
if ((hdr & DESC_HDR_DONE) == DESC_HDR_DONE)
status = 0;
@@ -476,8 +486,14 @@ static u32 current_desc_hdr(struct device *dev, int ch)
}
}
- if (priv->chan[ch].fifo[iter].desc->next_desc == cur_desc)
- return (priv->chan[ch].fifo[iter].desc + 1)->hdr;
+ if (priv->chan[ch].fifo[iter].desc->next_desc == cur_desc) {
+ struct talitos_edesc *edesc;
+
+ edesc = container_of(priv->chan[ch].fifo[iter].desc,
+ struct talitos_edesc, desc);
+ return ((struct talitos_desc *)
+ (edesc->buf + edesc->dma_len))->hdr;
+ }
return priv->chan[ch].fifo[iter].desc->hdr;
}
@@ -1402,15 +1418,11 @@ static struct talitos_edesc *talitos_edesc_alloc(struct device *dev,
edesc->dst_nents = dst_nents;
edesc->iv_dma = iv_dma;
edesc->dma_len = dma_len;
- if (dma_len) {
- void *addr = &edesc->link_tbl[0];
-
- if (is_sec1 && !dst)
- addr += sizeof(struct talitos_desc);
- edesc->dma_link_tbl = dma_map_single(dev, addr,
+ if (dma_len)
+ edesc->dma_link_tbl = dma_map_single(dev, &edesc->link_tbl[0],
edesc->dma_len,
DMA_BIDIRECTIONAL);
- }
+
return edesc;
}
@@ -1722,14 +1734,16 @@ static void common_nonsnoop_hash_unmap(struct device *dev,
struct talitos_private *priv = dev_get_drvdata(dev);
bool is_sec1 = has_ftr_sec1(priv);
struct talitos_desc *desc = &edesc->desc;
- struct talitos_desc *desc2 = desc + 1;
+ struct talitos_desc *desc2 = (struct talitos_desc *)
+ (edesc->buf + edesc->dma_len);
unmap_single_talitos_ptr(dev, &edesc->desc.ptr[5], DMA_FROM_DEVICE);
if (desc->next_desc &&
desc->ptr[5].ptr != desc2->ptr[5].ptr)
unmap_single_talitos_ptr(dev, &desc2->ptr[5], DMA_FROM_DEVICE);
- talitos_sg_unmap(dev, edesc, req_ctx->psrc, NULL, 0, 0);
+ if (req_ctx->psrc)
+ talitos_sg_unmap(dev, edesc, req_ctx->psrc, NULL, 0, 0);
/* When using hashctx-in, must unmap it. */
if (from_talitos_ptr_len(&edesc->desc.ptr[1], is_sec1))
@@ -1796,7 +1810,6 @@ static void talitos_handle_buggy_hash(struct talitos_ctx *ctx,
static int common_nonsnoop_hash(struct talitos_edesc *edesc,
struct ahash_request *areq, unsigned int length,
- unsigned int offset,
void (*callback) (struct device *dev,
struct talitos_desc *desc,
void *context, int error))
@@ -1835,9 +1848,7 @@ static int common_nonsnoop_hash(struct talitos_edesc *edesc,
sg_count = edesc->src_nents ?: 1;
if (is_sec1 && sg_count > 1)
- sg_pcopy_to_buffer(req_ctx->psrc, sg_count,
- edesc->buf + sizeof(struct talitos_desc),
- length, req_ctx->nbuf);
+ sg_copy_to_buffer(req_ctx->psrc, sg_count, edesc->buf, length);
else if (length)
sg_count = dma_map_sg(dev, req_ctx->psrc, sg_count,
DMA_TO_DEVICE);
@@ -1850,7 +1861,7 @@ static int common_nonsnoop_hash(struct talitos_edesc *edesc,
DMA_TO_DEVICE);
} else {
sg_count = talitos_sg_map(dev, req_ctx->psrc, length, edesc,
- &desc->ptr[3], sg_count, offset, 0);
+ &desc->ptr[3], sg_count, 0, 0);
if (sg_count > 1)
sync_needed = true;
}
@@ -1874,7 +1885,8 @@ static int common_nonsnoop_hash(struct talitos_edesc *edesc,
talitos_handle_buggy_hash(ctx, edesc, &desc->ptr[3]);
if (is_sec1 && req_ctx->nbuf && length) {
- struct talitos_desc *desc2 = desc + 1;
+ struct talitos_desc *desc2 = (struct talitos_desc *)
+ (edesc->buf + edesc->dma_len);
dma_addr_t next_desc;
memset(desc2, 0, sizeof(*desc2));
@@ -1895,7 +1907,7 @@ static int common_nonsnoop_hash(struct talitos_edesc *edesc,
DMA_TO_DEVICE);
copy_talitos_ptr(&desc2->ptr[2], &desc->ptr[2], is_sec1);
sg_count = talitos_sg_map(dev, req_ctx->psrc, length, edesc,
- &desc2->ptr[3], sg_count, offset, 0);
+ &desc2->ptr[3], sg_count, 0, 0);
if (sg_count > 1)
sync_needed = true;
copy_talitos_ptr(&desc2->ptr[5], &desc->ptr[5], is_sec1);
@@ -2006,7 +2018,6 @@ static int ahash_process_req(struct ahash_request *areq, unsigned int nbytes)
struct device *dev = ctx->dev;
struct talitos_private *priv = dev_get_drvdata(dev);
bool is_sec1 = has_ftr_sec1(priv);
- int offset = 0;
u8 *ctx_buf = req_ctx->buf[req_ctx->buf_idx];
if (!req_ctx->last && (nbytes + req_ctx->nbuf <= blocksize)) {
@@ -2046,6 +2057,8 @@ static int ahash_process_req(struct ahash_request *areq, unsigned int nbytes)
sg_chain(req_ctx->bufsl, 2, areq->src);
req_ctx->psrc = req_ctx->bufsl;
} else if (is_sec1 && req_ctx->nbuf && req_ctx->nbuf < blocksize) {
+ int offset;
+
if (nbytes_to_hash > blocksize)
offset = blocksize - req_ctx->nbuf;
else
@@ -2058,7 +2071,8 @@ static int ahash_process_req(struct ahash_request *areq, unsigned int nbytes)
sg_copy_to_buffer(areq->src, nents,
ctx_buf + req_ctx->nbuf, offset);
req_ctx->nbuf += offset;
- req_ctx->psrc = areq->src;
+ req_ctx->psrc = scatterwalk_ffwd(req_ctx->bufsl, areq->src,
+ offset);
} else
req_ctx->psrc = areq->src;
@@ -2098,8 +2112,7 @@ static int ahash_process_req(struct ahash_request *areq, unsigned int nbytes)
if (ctx->keylen && (req_ctx->first || req_ctx->last))
edesc->desc.hdr |= DESC_HDR_MODE0_MDEU_HMAC;
- return common_nonsnoop_hash(edesc, areq, nbytes_to_hash, offset,
- ahash_done);
+ return common_nonsnoop_hash(edesc, areq, nbytes_to_hash, ahash_done);
}
static int ahash_update(struct ahash_request *areq)
--
2.13.3
^ permalink raw reply related
* [PATCH v5 0/4] *** SUBJECT HERE ***
From: Christophe Leroy @ 2019-06-24 7:20 UTC (permalink / raw)
To: Herbert Xu, David S. Miller, horia.geanta
Cc: linuxppc-dev, linux-crypto, linux-kernel
This series is the last set of fixes for the Talitos driver.
We now get a fully clean boot on both SEC1 (SEC1.2 on mpc885) and
SEC2 (SEC2.2 on mpc8321E) with CONFIG_CRYPTO_MANAGER_EXTRA_TESTS:
[ 3.385197] bus: 'platform': really_probe: probing driver talitos with device ff020000.crypto
[ 3.450982] random: fast init done
[ 12.252548] alg: No test for authenc(hmac(md5),cbc(aes)) (authenc-hmac-md5-cbc-aes-talitos-hsna)
[ 12.262226] alg: No test for authenc(hmac(md5),cbc(des3_ede)) (authenc-hmac-md5-cbc-3des-talitos-hsna)
[ 43.310737] Bug in SEC1, padding ourself
[ 45.603318] random: crng init done
[ 54.612333] talitos ff020000.crypto: fsl,sec1.2 algorithms registered in /proc/crypto
[ 54.620232] driver: 'talitos': driver_bound: bound to device 'ff020000.crypto'
[ 1.193721] bus: 'platform': really_probe: probing driver talitos with device b0030000.crypto
[ 1.229197] random: fast init done
[ 2.714920] alg: No test for authenc(hmac(sha224),cbc(aes)) (authenc-hmac-sha224-cbc-aes-talitos)
[ 2.724312] alg: No test for authenc(hmac(sha224),cbc(aes)) (authenc-hmac-sha224-cbc-aes-talitos-hsna)
[ 4.482045] alg: No test for authenc(hmac(md5),cbc(aes)) (authenc-hmac-md5-cbc-aes-talitos)
[ 4.490940] alg: No test for authenc(hmac(md5),cbc(aes)) (authenc-hmac-md5-cbc-aes-talitos-hsna)
[ 4.500280] alg: No test for authenc(hmac(md5),cbc(des3_ede)) (authenc-hmac-md5-cbc-3des-talitos)
[ 4.509727] alg: No test for authenc(hmac(md5),cbc(des3_ede)) (authenc-hmac-md5-cbc-3des-talitos-hsna)
[ 6.631781] random: crng init done
[ 11.521795] talitos b0030000.crypto: fsl,sec2.2 algorithms registered in /proc/crypto
[ 11.529803] driver: 'talitos': driver_bound: bound to device 'b0030000.crypto'
v2: dropped patch 1 which was irrelevant due to a rebase weirdness. Added Cc to stable on the 2 first patches.
v3:
- removed stable reference in patch 1
- reworded patch 1 to include name of patch 2 for the dependency.
- mentionned this dependency in patch 2 as well.
- corrected the Fixes: sha1 in patch 4
v4:
- using scatterwalk_ffwd() instead of opencodying SG list forwarding.
- Added a patch to fix sg_copy_to_buffer() when sg->offset() is greater than PAGE_SIZE,
otherwise sg_copy_to_buffer() fails when the list has been forwarded with scatterwalk_ffwd().
- taken the patch "crypto: talitos - eliminate unneeded 'done' functions at build time"
out of the series because it is independent.
- added a helper to find the header field associated to a request in flush_channe()
v5:
- Replacing the while loop by a direct shift/mask operation, as suggested by Herbert in patch 1.
Christophe Leroy (4):
lib/scatterlist: Fix mapping iterator when sg->offset is greater than
PAGE_SIZE
crypto: talitos - move struct talitos_edesc into talitos.h
crypto: talitos - fix hash on SEC1.
crypto: talitos - drop icv_ool
drivers/crypto/talitos.c | 102 +++++++++++++++++++----------------------------
drivers/crypto/talitos.h | 28 +++++++++++++
lib/scatterlist.c | 9 +++--
3 files changed, 74 insertions(+), 65 deletions(-)
--
2.13.3
^ 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