LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH v3 6/6] KVM: PPC: mmio: Reject instructions that access more than mmio.data size
From: Fabiano Rosas @ 2022-01-11 14:32 UTC (permalink / raw)
  To: Nicholas Piggin, kvm-ppc; +Cc: aik, linuxppc-dev
In-Reply-To: <1641800177.nr6ngd1fot.astroid@bobo.none>

Nicholas Piggin <npiggin@gmail.com> writes:

> Excerpts from Fabiano Rosas's message of January 8, 2022 7:00 am:
>> The MMIO interface between the kernel and userspace uses a structure
>> that supports a maximum of 8-bytes of data. Instructions that access
>> more than that need to be emulated in parts.
>> 
>> We currently don't have generic support for splitting the emulation in
>> parts and each set of instructions needs to be explicitly included.
>> 
>> There's already an error message being printed when a load or store
>> exceeds the mmio.data buffer but we don't fail the emulation until
>> later at kvmppc_complete_mmio_load and even then we allow userspace to
>> make a partial copy of the data, which ends up overwriting some fields
>> of the mmio structure.
>> 
>> This patch makes the emulation fail earlier at kvmppc_handle_load|store,
>> which will send a Program interrupt to the guest. This is better than
>> allowing the guest to proceed with partial data.
>> 
>> Note that this was caught in a somewhat artificial scenario using
>> quadword instructions (lq/stq), there's no account of an actual guest
>> in the wild running instructions that are not properly emulated.
>> 
>> (While here, fix the error message to check against 'bytes' and not
>> 'run->mmio.len' which at this point has an old value.)
>
> This looks good to me
>
> Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
>
>> 
>> Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com>
>> Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>> ---
>>  arch/powerpc/kvm/powerpc.c | 6 ++++--
>>  1 file changed, 4 insertions(+), 2 deletions(-)
>> 
>> diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
>> index 56b0faab7a5f..a1643ca988e0 100644
>> --- a/arch/powerpc/kvm/powerpc.c
>> +++ b/arch/powerpc/kvm/powerpc.c
>> @@ -1246,7 +1246,8 @@ static int __kvmppc_handle_load(struct kvm_vcpu *vcpu,
>>  
>>  	if (bytes > sizeof(run->mmio.data)) {
>>  		printk(KERN_ERR "%s: bad MMIO length: %d\n", __func__,
>> -		       run->mmio.len);
>> +		       bytes);
>
> I wonder though this should probably be ratelimited, informational (or 
> at least warning because it's a host message), and perhaps a bit more
> explanatory that it's a guest problem (or at least lack of host support
> for particular guest MMIO sizes).

Yes, I'll ratelimit it an try to make it clear that this is something
that happened inside the guest but it lacks support in KVM. Then
hopefully people will tell to us if they ever need that support.

^ permalink raw reply

* Re: [PATCH] powerpc: dts: t1040rdb: fix ports names for Seville Ethernet switch
From: Vladimir Oltean @ 2022-01-11 13:13 UTC (permalink / raw)
  To: Maxim Kiselev
  Cc: Andrew Lunn, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org, fido_max@inbox.ru, Rob Herring,
	Paul Mackerras, linuxppc-dev@lists.ozlabs.org, David S. Miller
In-Reply-To: <CALHCpMgO2bqxPcaxwg29gEGF4te1HCgCa7SdNFVoxa6JDzrCrA@mail.gmail.com>

Hi Maxim,

On Mon, Jan 10, 2022 at 07:40:38AM +0000, Maxim Kiselev wrote:
> Here are photos of my boards.

Your patch is OK to change t1040rdb.dts, but please preserve the existing
port mappings in a new arch/powerpc/boot/dts/fsl/t1040rdb-rev-a.dts file.

You will also need to modify the /model and /compatible nodes of the new
device tree for Rev A, something like "fsl,T1040RDB-REV-A". Take a look
at arch/arm64/boot/dts/freescale/fsl-lx2160a-bluebox3-rev-a.dts to see
an example of what I'd like to be done.

Thanks.

^ permalink raw reply

* [PATCH v5 0/6] KEXEC_SIG with appended signature
From: Michal Suchanek @ 2022-01-11 11:37 UTC (permalink / raw)
  To: keyrings, linux-crypto, linux-integrity
  Cc: Nayna, Mimi Zohar, David Howells, Paul Mackerras,
	Alexander Gordeev, Rob Herring, Herbert Xu, Baoquan He,
	Christian Borntraeger, James Morris, Lakshmi Ramasubramanian,
	Michal Suchanek, Serge E. Hallyn, Vasily Gorbik, linux-s390,
	Heiko Carstens, Dmitry Kasatkin, Hari Bathini, Daniel Axtens,
	Christian Borntraeger, Philipp Rudo, Frank van der Linden, kexec,
	linux-kernel, Luis Chamberlain, Sven Schnelle,
	linux-security-module, Jessica Yu, linuxppc-dev, David S. Miller,
	Thiago Jung Bauermann, buendgen

Hello,

This is a refresh of the KEXEC_SIG series.

This adds KEXEC_SIG support on powerpc and deduplicates the code dealing
with appended signatures in the kernel.

powerpc supports IMA_KEXEC but that's an exception rather than the norm.
On the other hand, KEXEC_SIG is portable across platforms.

For distributions to have uniform security features across platforms one
option should be used on all platforms.

Thanks

Michal

Previous revision: https://lore.kernel.org/linuxppc-dev/cover.1637862358.git.msuchanek@suse.de/
Patched kernel tree: https://github.com/hramrach/kernel/tree/kexec_sig

Michal Suchanek (6):
  s390/kexec_file: Don't opencode appended signature check.
  powerpc/kexec_file: Add KEXEC_SIG support.
  kexec_file: Don't opencode appended signature verification.
  module: strip the signature marker in the verification function.
  module: Use key_being_used_for for log messages in
    verify_appended_signature
  module: Move duplicate mod_check_sig users code to mod_parse_sig

 arch/powerpc/Kconfig                     | 16 +++++++
 arch/powerpc/kexec/elf_64.c              | 12 +++++
 arch/s390/Kconfig                        |  2 +-
 arch/s390/kernel/machine_kexec_file.c    | 41 +----------------
 crypto/asymmetric_keys/asymmetric_type.c |  1 +
 include/linux/module_signature.h         |  4 +-
 include/linux/verification.h             |  5 ++
 kernel/module-internal.h                 |  2 -
 kernel/module.c                          | 12 ++---
 kernel/module_signature.c                | 58 +++++++++++++++++++++++-
 kernel/module_signing.c                  | 34 ++++++--------
 security/integrity/ima/ima_modsig.c      | 22 ++-------
 12 files changed, 119 insertions(+), 90 deletions(-)

-- 
2.31.1


^ permalink raw reply

* [PATCH v5 6/6] module: Move duplicate mod_check_sig users code to mod_parse_sig
From: Michal Suchanek @ 2022-01-11 11:37 UTC (permalink / raw)
  To: keyrings, linux-crypto, linux-integrity
  Cc: Nayna, Mimi Zohar, David Howells, Paul Mackerras,
	Alexander Gordeev, Rob Herring, Herbert Xu, Baoquan He,
	Christian Borntraeger, James Morris, Lakshmi Ramasubramanian,
	Michal Suchanek, Serge E. Hallyn, Vasily Gorbik, linux-s390,
	Heiko Carstens, Dmitry Kasatkin, Hari Bathini, Daniel Axtens,
	Christian Borntraeger, Philipp Rudo, Frank van der Linden, kexec,
	linux-kernel, Luis Chamberlain, Sven Schnelle,
	linux-security-module, Jessica Yu, linuxppc-dev, David S. Miller,
	Thiago Jung Bauermann, buendgen
In-Reply-To: <cover.1641900831.git.msuchanek@suse.de>

Multiple users of mod_check_sig check for the marker, then call
mod_check_sig, extract signature length, and remove the signature.

Put this code in one place together with mod_check_sig.

This changes the error from ENOENT to ENODATA for ima_read_modsig in the
case the signature marker is missing.

This also changes the buffer length in ima_read_modsig from size_t to
unsigned long. This reduces the possible value range on 32bit but the
length refers to kernel in-memory buffer which cannot be longer than
ULONG_MAX.

Also change mod_check_sig to unsigned long while at it.

Signed-off-by: Michal Suchanek <msuchanek@suse.de>
---
v3: - Philipp Rudo <prudo@redhat.com>: Update the commit with note about
      change of raturn value
    - Preserve the EBADMSG error code while moving code araound
v4: - remove unused variable ms in module_signing.c
    - note about buffer length
v5: - also change the functions in module_signature.c to unsigned long
---
 include/linux/module_signature.h    |  4 +-
 kernel/module_signature.c           | 58 ++++++++++++++++++++++++++++-
 kernel/module_signing.c             | 27 ++------------
 security/integrity/ima/ima_modsig.c | 22 ++---------
 4 files changed, 66 insertions(+), 45 deletions(-)

diff --git a/include/linux/module_signature.h b/include/linux/module_signature.h
index 7eb4b00381ac..e5fb157c085c 100644
--- a/include/linux/module_signature.h
+++ b/include/linux/module_signature.h
@@ -40,7 +40,9 @@ struct module_signature {
 	__be32	sig_len;	/* Length of signature data */
 };
 
-int mod_check_sig(const struct module_signature *ms, size_t file_len,
+int mod_check_sig(const struct module_signature *ms, unsigned long file_len,
+		  const char *name);
+int mod_parse_sig(const void *data, unsigned long *len, unsigned long *sig_len,
 		  const char *name);
 
 #endif /* _LINUX_MODULE_SIGNATURE_H */
diff --git a/kernel/module_signature.c b/kernel/module_signature.c
index 00132d12487c..4a36405ecd08 100644
--- a/kernel/module_signature.c
+++ b/kernel/module_signature.c
@@ -8,17 +8,39 @@
 
 #include <linux/errno.h>
 #include <linux/printk.h>
+#include <linux/string.h>
 #include <linux/module_signature.h>
 #include <asm/byteorder.h>
 
+/**
+ * mod_check_sig_marker - check that the given data has signature marker at the end
+ *
+ * @data:	Data with appended signature
+ * @len:	Length of data. Signature marker length is subtracted on success.
+ */
+static inline int mod_check_sig_marker(const void *data, unsigned long *len)
+{
+	const unsigned long markerlen = sizeof(MODULE_SIG_STRING) - 1;
+
+	if (markerlen > *len)
+		return -ENODATA;
+
+	if (memcmp(data + *len - markerlen, MODULE_SIG_STRING,
+		   markerlen))
+		return -ENODATA;
+
+	*len -= markerlen;
+	return 0;
+}
+
 /**
  * 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.
+ * @file_len:	Size of the file to which @ms is appended (without the marker).
  * @name:	What is being checked. Used for error messages.
  */
-int mod_check_sig(const struct module_signature *ms, size_t file_len,
+int mod_check_sig(const struct module_signature *ms, unsigned long file_len,
 		  const char *name)
 {
 	if (be32_to_cpu(ms->sig_len) >= file_len - sizeof(*ms))
@@ -44,3 +66,35 @@ int mod_check_sig(const struct module_signature *ms, size_t file_len,
 
 	return 0;
 }
+
+/**
+ * mod_parse_sig - check that the given signature is sane and determine signature length
+ *
+ * @data:	Data with appended signature.
+ * @len:	Length of data. Signature and marker length is subtracted on success.
+ * @sig_len:	Length of signature. Filled on success.
+ * @name:	What is being checked. Used for error messages.
+ */
+int mod_parse_sig(const void *data, unsigned long *len, unsigned long *sig_len, const char *name)
+{
+	const struct module_signature *sig;
+	int rc;
+
+	rc = mod_check_sig_marker(data, len);
+	if (rc)
+		return rc;
+
+	if (*len < sizeof(*sig))
+		return -EBADMSG;
+
+	sig = data + (*len - sizeof(*sig));
+
+	rc = mod_check_sig(sig, *len, name);
+	if (rc)
+		return rc;
+
+	*sig_len = be32_to_cpu(sig->sig_len);
+	*len -= *sig_len + sizeof(*sig);
+
+	return 0;
+}
diff --git a/kernel/module_signing.c b/kernel/module_signing.c
index 20857d2a15ca..1d4cb03cce21 100644
--- a/kernel/module_signing.c
+++ b/kernel/module_signing.c
@@ -25,35 +25,16 @@ int verify_appended_signature(const void *data, unsigned long *len,
 			      struct key *trusted_keys,
 			      enum key_being_used_for purpose)
 {
-	const unsigned long markerlen = sizeof(MODULE_SIG_STRING) - 1;
-	struct module_signature *ms;
-	unsigned long sig_len, modlen = *len;
+	unsigned long sig_len;
 	int ret;
 
-	pr_devel("==>%s %s(,%lu)\n", __func__, key_being_used_for[purpose], modlen);
+	pr_devel("==>%s %s(,%lu)\n", __func__, key_being_used_for[purpose], *len);
 
-	if (markerlen > modlen)
-		return -ENODATA;
-
-	if (memcmp(data + modlen - markerlen, MODULE_SIG_STRING,
-		   markerlen))
-		return -ENODATA;
-	modlen -= markerlen;
-
-	if (modlen <= sizeof(*ms))
-		return -EBADMSG;
-
-	ms = data + modlen - sizeof(*ms);
-
-	ret = mod_check_sig(ms, modlen, key_being_used_for[purpose]);
+	ret = mod_parse_sig(data, len, &sig_len, key_being_used_for[purpose]);
 	if (ret)
 		return ret;
 
-	sig_len = be32_to_cpu(ms->sig_len);
-	modlen -= sig_len + sizeof(*ms);
-	*len = modlen;
-
-	return verify_pkcs7_signature(data, modlen, data + modlen, sig_len,
+	return verify_pkcs7_signature(data, *len, data + *len, sig_len,
 				      trusted_keys,
 				      purpose,
 				      NULL, NULL);
diff --git a/security/integrity/ima/ima_modsig.c b/security/integrity/ima/ima_modsig.c
index fb25723c65bc..b40c8fdf6139 100644
--- a/security/integrity/ima/ima_modsig.c
+++ b/security/integrity/ima/ima_modsig.c
@@ -37,33 +37,17 @@ struct modsig {
  *
  * Return: 0 on success, error code otherwise.
  */
-int ima_read_modsig(enum ima_hooks func, const void *buf, loff_t buf_len,
+int ima_read_modsig(enum ima_hooks func, const void *buf, loff_t len,
 		    struct modsig **modsig)
 {
-	const size_t marker_len = strlen(MODULE_SIG_STRING);
-	const struct module_signature *sig;
 	struct modsig *hdr;
-	size_t sig_len;
-	const void *p;
+	unsigned long sig_len, buf_len = len;
 	int rc;
 
-	if (buf_len <= marker_len + sizeof(*sig))
-		return -ENOENT;
-
-	p = buf + buf_len - marker_len;
-	if (memcmp(p, MODULE_SIG_STRING, marker_len))
-		return -ENOENT;
-
-	buf_len -= marker_len;
-	sig = (const struct module_signature *)(p - sizeof(*sig));
-
-	rc = mod_check_sig(sig, buf_len, func_tokens[func]);
+	rc = mod_parse_sig(buf, &buf_len, &sig_len, func_tokens[func]);
 	if (rc)
 		return rc;
 
-	sig_len = be32_to_cpu(sig->sig_len);
-	buf_len -= sig_len + sizeof(*sig);
-
 	/* Allocate sig_len additional bytes to hold the raw PKCS#7 data. */
 	hdr = kzalloc(sizeof(*hdr) + sig_len, GFP_KERNEL);
 	if (!hdr)
-- 
2.31.1


^ permalink raw reply related

* [PATCH v5 1/6] s390/kexec_file: Don't opencode appended signature check.
From: Michal Suchanek @ 2022-01-11 11:37 UTC (permalink / raw)
  To: keyrings, linux-crypto, linux-integrity
  Cc: Nayna, Mimi Zohar, David Howells, Paul Mackerras,
	Alexander Gordeev, Rob Herring, Herbert Xu, Baoquan He,
	Christian Borntraeger, James Morris, Lakshmi Ramasubramanian,
	Michal Suchanek, Serge E. Hallyn, Vasily Gorbik, linux-s390,
	Heiko Carstens, Dmitry Kasatkin, Hari Bathini, Daniel Axtens,
	Christian Borntraeger, Philipp Rudo, Frank van der Linden, kexec,
	linux-kernel, Luis Chamberlain, Sven Schnelle,
	linux-security-module, Jessica Yu, linuxppc-dev, David S. Miller,
	Thiago Jung Bauermann, buendgen
In-Reply-To: <cover.1641900831.git.msuchanek@suse.de>

Module verification already implements appeded signature check.

Reuse it for kexec_file.

The kexec_file implementation uses EKEYREJECTED error in some cases when
there is no key and the common implementation uses ENOPKG or EBADMSG
instead.

Signed-off-by: Michal Suchanek <msuchanek@suse.de>
Acked-by: Heiko Carstens <hca@linux.ibm.com>
---
v3: Philipp Rudo <prudo@redhat.com>: Update the commit with note about
change of return value
---
 arch/s390/kernel/machine_kexec_file.c | 22 +++++-----------------
 1 file changed, 5 insertions(+), 17 deletions(-)

diff --git a/arch/s390/kernel/machine_kexec_file.c b/arch/s390/kernel/machine_kexec_file.c
index 8f43575a4dd3..c944d71316c7 100644
--- a/arch/s390/kernel/machine_kexec_file.c
+++ b/arch/s390/kernel/machine_kexec_file.c
@@ -31,6 +31,7 @@ int s390_verify_sig(const char *kernel, unsigned long kernel_len)
 	const unsigned long marker_len = sizeof(MODULE_SIG_STRING) - 1;
 	struct module_signature *ms;
 	unsigned long sig_len;
+	int ret;
 
 	/* Skip signature verification when not secure IPLed. */
 	if (!ipl_secure_flag)
@@ -45,25 +46,12 @@ int s390_verify_sig(const char *kernel, unsigned long kernel_len)
 	kernel_len -= marker_len;
 
 	ms = (void *)kernel + kernel_len - sizeof(*ms);
-	kernel_len -= sizeof(*ms);
+	ret = mod_check_sig(ms, kernel_len, "kexec");
+	if (ret)
+		return ret;
 
 	sig_len = be32_to_cpu(ms->sig_len);
-	if (sig_len >= kernel_len)
-		return -EKEYREJECTED;
-	kernel_len -= sig_len;
-
-	if (ms->id_type != PKEY_ID_PKCS7)
-		return -EKEYREJECTED;
-
-	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) {
-		return -EBADMSG;
-	}
+	kernel_len -= sizeof(*ms) + sig_len;
 
 	return verify_pkcs7_signature(kernel, kernel_len,
 				      kernel + kernel_len, sig_len,
-- 
2.31.1


^ permalink raw reply related

* [PATCH v5 2/6] powerpc/kexec_file: Add KEXEC_SIG support.
From: Michal Suchanek @ 2022-01-11 11:37 UTC (permalink / raw)
  To: keyrings, linux-crypto, linux-integrity
  Cc: Nayna, Mimi Zohar, David Howells, Paul Mackerras,
	Alexander Gordeev, Rob Herring, Herbert Xu, Baoquan He,
	Christian Borntraeger, James Morris, Lakshmi Ramasubramanian,
	Michal Suchanek, Serge E. Hallyn, Vasily Gorbik, linux-s390,
	Heiko Carstens, Dmitry Kasatkin, Hari Bathini, Daniel Axtens,
	Christian Borntraeger, Philipp Rudo, Frank van der Linden, kexec,
	linux-kernel, Luis Chamberlain, Sven Schnelle,
	linux-security-module, Jessica Yu, linuxppc-dev, David S. Miller,
	Thiago Jung Bauermann, buendgen
In-Reply-To: <cover.1641900831.git.msuchanek@suse.de>

Copy the code from s390x

Both powerpc and s390x use appended signature format (as opposed to EFI
based patforms using PE format).

Signed-off-by: Michal Suchanek <msuchanek@suse.de>
---
v3: - Philipp Rudo <prudo@redhat.com>: Update the comit message with
      explanation why the s390 code is usable on powerpc.
    - Include correct header for mod_check_sig
    - Nayna <nayna@linux.vnet.ibm.com>: Mention additional IMA features
      in kconfig text
---
 arch/powerpc/Kconfig        | 16 ++++++++++++++++
 arch/powerpc/kexec/elf_64.c | 36 ++++++++++++++++++++++++++++++++++++
 2 files changed, 52 insertions(+)

diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index dea74d7717c0..1cde9b6c5987 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -560,6 +560,22 @@ config KEXEC_FILE
 config ARCH_HAS_KEXEC_PURGATORY
 	def_bool KEXEC_FILE
 
+config KEXEC_SIG
+	bool "Verify kernel signature during kexec_file_load() syscall"
+	depends on KEXEC_FILE && MODULE_SIG_FORMAT
+	help
+	  This option makes kernel signature verification mandatory for
+	  the kexec_file_load() syscall.
+
+	  In addition to that option, you need to enable signature
+	  verification for the corresponding kernel image type being
+	  loaded in order for this to work.
+
+	  Note: on powerpc IMA_ARCH_POLICY also implements kexec'ed kernel
+	  verification. In addition IMA adds kernel hashes to the measurement
+	  list, extends IMA PCR in the TPM, and implements kernel image
+	  blacklist by hash.
+
 config RELOCATABLE
 	bool "Build a relocatable kernel"
 	depends on PPC64 || (FLATMEM && (44x || FSL_BOOKE))
diff --git a/arch/powerpc/kexec/elf_64.c b/arch/powerpc/kexec/elf_64.c
index eeb258002d1e..98d1cb5135b4 100644
--- a/arch/powerpc/kexec/elf_64.c
+++ b/arch/powerpc/kexec/elf_64.c
@@ -23,6 +23,7 @@
 #include <linux/of_fdt.h>
 #include <linux/slab.h>
 #include <linux/types.h>
+#include <linux/module_signature.h>
 
 static void *elf64_load(struct kimage *image, char *kernel_buf,
 			unsigned long kernel_len, char *initrd,
@@ -151,7 +152,42 @@ static void *elf64_load(struct kimage *image, char *kernel_buf,
 	return ret ? ERR_PTR(ret) : NULL;
 }
 
+#ifdef CONFIG_KEXEC_SIG
+int elf64_verify_sig(const char *kernel, unsigned long kernel_len)
+{
+	const unsigned long marker_len = sizeof(MODULE_SIG_STRING) - 1;
+	struct module_signature *ms;
+	unsigned long sig_len;
+	int ret;
+
+	if (marker_len > kernel_len)
+		return -EKEYREJECTED;
+
+	if (memcmp(kernel + kernel_len - marker_len, MODULE_SIG_STRING,
+		   marker_len))
+		return -EKEYREJECTED;
+	kernel_len -= marker_len;
+
+	ms = (void *)kernel + kernel_len - sizeof(*ms);
+	ret = mod_check_sig(ms, kernel_len, "kexec");
+	if (ret)
+		return ret;
+
+	sig_len = be32_to_cpu(ms->sig_len);
+	kernel_len -= sizeof(*ms) + sig_len;
+
+	return verify_pkcs7_signature(kernel, kernel_len,
+				      kernel + kernel_len, sig_len,
+				      VERIFY_USE_PLATFORM_KEYRING,
+				      VERIFYING_MODULE_SIGNATURE,
+				      NULL, NULL);
+}
+#endif /* CONFIG_KEXEC_SIG */
+
 const struct kexec_file_ops kexec_elf64_ops = {
 	.probe = kexec_elf_probe,
 	.load = elf64_load,
+#ifdef CONFIG_KEXEC_SIG
+	.verify_sig = elf64_verify_sig,
+#endif
 };
-- 
2.31.1


^ permalink raw reply related

* [PATCH v5 3/6] kexec_file: Don't opencode appended signature verification.
From: Michal Suchanek @ 2022-01-11 11:37 UTC (permalink / raw)
  To: keyrings, linux-crypto, linux-integrity
  Cc: Nayna, Mimi Zohar, David Howells, Paul Mackerras,
	Alexander Gordeev, Rob Herring, Herbert Xu, Baoquan He,
	Christian Borntraeger, James Morris, Lakshmi Ramasubramanian,
	Michal Suchanek, Serge E. Hallyn, Vasily Gorbik, linux-s390,
	Heiko Carstens, Dmitry Kasatkin, Hari Bathini, Daniel Axtens,
	Christian Borntraeger, Philipp Rudo, Frank van der Linden, kexec,
	linux-kernel, Luis Chamberlain, Sven Schnelle,
	linux-security-module, Jessica Yu, linuxppc-dev, David S. Miller,
	Thiago Jung Bauermann, buendgen
In-Reply-To: <cover.1641900831.git.msuchanek@suse.de>

Module verification already implements appeded signature verification.

Reuse it for kexec_file.

Signed-off-by: Michal Suchanek <msuchanek@suse.de>
---
v3: - Philipp Rudo <prudo@redhat.com>: Update the dependency on
      MODULE_SIG_FORMAT to MODULE_SIG
    - Include linux/verification.h - previously added in earlier patch
v4: - kernel test robot <lkp@intel.com>: Use unsigned long rather than size_t for data length
    - Update the code in kernel/module_signing.c to use pointer rather
      than memcpy as the kexec and IMA code does
---
 arch/powerpc/Kconfig                  |  2 +-
 arch/powerpc/kexec/elf_64.c           | 19 +++------------
 arch/s390/Kconfig                     |  2 +-
 arch/s390/kernel/machine_kexec_file.c | 18 ++------------
 include/linux/verification.h          |  3 +++
 kernel/module-internal.h              |  2 --
 kernel/module.c                       |  4 +++-
 kernel/module_signing.c               | 34 ++++++++++++++++-----------
 8 files changed, 33 insertions(+), 51 deletions(-)

diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 1cde9b6c5987..4092187474ff 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -562,7 +562,7 @@ config ARCH_HAS_KEXEC_PURGATORY
 
 config KEXEC_SIG
 	bool "Verify kernel signature during kexec_file_load() syscall"
-	depends on KEXEC_FILE && MODULE_SIG_FORMAT
+	depends on KEXEC_FILE && MODULE_SIG
 	help
 	  This option makes kernel signature verification mandatory for
 	  the kexec_file_load() syscall.
diff --git a/arch/powerpc/kexec/elf_64.c b/arch/powerpc/kexec/elf_64.c
index 98d1cb5135b4..64cd314cce0d 100644
--- a/arch/powerpc/kexec/elf_64.c
+++ b/arch/powerpc/kexec/elf_64.c
@@ -23,6 +23,7 @@
 #include <linux/of_fdt.h>
 #include <linux/slab.h>
 #include <linux/types.h>
+#include <linux/verification.h>
 #include <linux/module_signature.h>
 
 static void *elf64_load(struct kimage *image, char *kernel_buf,
@@ -156,9 +157,6 @@ static void *elf64_load(struct kimage *image, char *kernel_buf,
 int elf64_verify_sig(const char *kernel, unsigned long kernel_len)
 {
 	const unsigned long marker_len = sizeof(MODULE_SIG_STRING) - 1;
-	struct module_signature *ms;
-	unsigned long sig_len;
-	int ret;
 
 	if (marker_len > kernel_len)
 		return -EKEYREJECTED;
@@ -168,19 +166,8 @@ int elf64_verify_sig(const char *kernel, unsigned long kernel_len)
 		return -EKEYREJECTED;
 	kernel_len -= marker_len;
 
-	ms = (void *)kernel + kernel_len - sizeof(*ms);
-	ret = mod_check_sig(ms, kernel_len, "kexec");
-	if (ret)
-		return ret;
-
-	sig_len = be32_to_cpu(ms->sig_len);
-	kernel_len -= sizeof(*ms) + sig_len;
-
-	return verify_pkcs7_signature(kernel, kernel_len,
-				      kernel + kernel_len, sig_len,
-				      VERIFY_USE_PLATFORM_KEYRING,
-				      VERIFYING_MODULE_SIGNATURE,
-				      NULL, NULL);
+	return verify_appended_signature(kernel, &kernel_len, VERIFY_USE_PLATFORM_KEYRING,
+					 "kexec_file");
 }
 #endif /* CONFIG_KEXEC_SIG */
 
diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
index 2a5bb4f29cfe..cece7152ea35 100644
--- a/arch/s390/Kconfig
+++ b/arch/s390/Kconfig
@@ -544,7 +544,7 @@ config ARCH_HAS_KEXEC_PURGATORY
 
 config KEXEC_SIG
 	bool "Verify kernel signature during kexec_file_load() syscall"
-	depends on KEXEC_FILE && MODULE_SIG_FORMAT
+	depends on KEXEC_FILE && MODULE_SIG
 	help
 	  This option makes kernel signature verification mandatory for
 	  the kexec_file_load() syscall.
diff --git a/arch/s390/kernel/machine_kexec_file.c b/arch/s390/kernel/machine_kexec_file.c
index c944d71316c7..345f2eab6e04 100644
--- a/arch/s390/kernel/machine_kexec_file.c
+++ b/arch/s390/kernel/machine_kexec_file.c
@@ -29,9 +29,6 @@ const struct kexec_file_ops * const kexec_file_loaders[] = {
 int s390_verify_sig(const char *kernel, unsigned long kernel_len)
 {
 	const unsigned long marker_len = sizeof(MODULE_SIG_STRING) - 1;
-	struct module_signature *ms;
-	unsigned long sig_len;
-	int ret;
 
 	/* Skip signature verification when not secure IPLed. */
 	if (!ipl_secure_flag)
@@ -45,19 +42,8 @@ int s390_verify_sig(const char *kernel, unsigned long kernel_len)
 		return -EKEYREJECTED;
 	kernel_len -= marker_len;
 
-	ms = (void *)kernel + kernel_len - sizeof(*ms);
-	ret = mod_check_sig(ms, kernel_len, "kexec");
-	if (ret)
-		return ret;
-
-	sig_len = be32_to_cpu(ms->sig_len);
-	kernel_len -= sizeof(*ms) + sig_len;
-
-	return verify_pkcs7_signature(kernel, kernel_len,
-				      kernel + kernel_len, sig_len,
-				      VERIFY_USE_PLATFORM_KEYRING,
-				      VERIFYING_MODULE_SIGNATURE,
-				      NULL, NULL);
+	return verify_appended_signature(kernel, &kernel_len, VERIFY_USE_PLATFORM_KEYRING,
+					"kexec_file");
 }
 #endif /* CONFIG_KEXEC_SIG */
 
diff --git a/include/linux/verification.h b/include/linux/verification.h
index a655923335ae..32db9287a7b0 100644
--- a/include/linux/verification.h
+++ b/include/linux/verification.h
@@ -60,5 +60,8 @@ extern int verify_pefile_signature(const void *pebuf, unsigned pelen,
 				   enum key_being_used_for usage);
 #endif
 
+int verify_appended_signature(const void *data, unsigned long *len,
+			      struct key *trusted_keys, const char *what);
+
 #endif /* CONFIG_SYSTEM_DATA_VERIFICATION */
 #endif /* _LINUX_VERIFY_PEFILE_H */
diff --git a/kernel/module-internal.h b/kernel/module-internal.h
index 33783abc377b..80461e14bf29 100644
--- a/kernel/module-internal.h
+++ b/kernel/module-internal.h
@@ -27,5 +27,3 @@ struct load_info {
 		unsigned int sym, str, mod, vers, info, pcpu;
 	} index;
 };
-
-extern int mod_verify_sig(const void *mod, struct load_info *info);
diff --git a/kernel/module.c b/kernel/module.c
index 84a9141a5e15..8481933dfa92 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -57,6 +57,7 @@
 #include <linux/bsearch.h>
 #include <linux/dynamic_debug.h>
 #include <linux/audit.h>
+#include <linux/verification.h>
 #include <uapi/linux/module.h>
 #include "module-internal.h"
 
@@ -2894,7 +2895,8 @@ static int module_sig_check(struct load_info *info, int flags)
 	    memcmp(mod + info->len - markerlen, MODULE_SIG_STRING, markerlen) == 0) {
 		/* We truncate the module to discard the signature */
 		info->len -= markerlen;
-		err = mod_verify_sig(mod, info);
+		err = verify_appended_signature(mod, &info->len,
+						VERIFY_USE_SECONDARY_KEYRING, "module");
 		if (!err) {
 			info->sig_ok = true;
 			return 0;
diff --git a/kernel/module_signing.c b/kernel/module_signing.c
index 8723ae70ea1f..30149969f21f 100644
--- a/kernel/module_signing.c
+++ b/kernel/module_signing.c
@@ -14,32 +14,38 @@
 #include <crypto/public_key.h>
 #include "module-internal.h"
 
-/*
- * Verify the signature on a module.
+/**
+ * verify_appended_signature - Verify the signature on a module with the
+ * signature marker stripped.
+ * @data: The data to be verified
+ * @len: Size of @data.
+ * @trusted_keys: Keyring to use for verification
+ * @what: Informational string for log messages
  */
-int mod_verify_sig(const void *mod, struct load_info *info)
+int verify_appended_signature(const void *data, unsigned long *len,
+			      struct key *trusted_keys, const char *what)
 {
-	struct module_signature ms;
-	size_t sig_len, modlen = info->len;
+	struct module_signature *ms;
+	unsigned long sig_len, modlen = *len;
 	int ret;
 
-	pr_devel("==>%s(,%zu)\n", __func__, modlen);
+	pr_devel("==>%s(,%lu)\n", __func__, modlen);
 
-	if (modlen <= sizeof(ms))
+	if (modlen <= sizeof(*ms))
 		return -EBADMSG;
 
-	memcpy(&ms, mod + (modlen - sizeof(ms)), sizeof(ms));
+	ms = data + modlen - sizeof(*ms);
 
-	ret = mod_check_sig(&ms, modlen, "module");
+	ret = mod_check_sig(ms, modlen, what);
 	if (ret)
 		return ret;
 
-	sig_len = be32_to_cpu(ms.sig_len);
-	modlen -= sig_len + sizeof(ms);
-	info->len = modlen;
+	sig_len = be32_to_cpu(ms->sig_len);
+	modlen -= sig_len + sizeof(*ms);
+	*len = modlen;
 
-	return verify_pkcs7_signature(mod, modlen, mod + modlen, sig_len,
-				      VERIFY_USE_SECONDARY_KEYRING,
+	return verify_pkcs7_signature(data, modlen, data + modlen, sig_len,
+				      trusted_keys,
 				      VERIFYING_MODULE_SIGNATURE,
 				      NULL, NULL);
 }
-- 
2.31.1


^ permalink raw reply related

* [PATCH v5 5/6] module: Use key_being_used_for for log messages in verify_appended_signature
From: Michal Suchanek @ 2022-01-11 11:37 UTC (permalink / raw)
  To: keyrings, linux-crypto, linux-integrity
  Cc: Nayna, Mimi Zohar, David Howells, Paul Mackerras,
	Alexander Gordeev, Rob Herring, Herbert Xu, Baoquan He,
	Christian Borntraeger, James Morris, Lakshmi Ramasubramanian,
	Michal Suchanek, Serge E. Hallyn, Vasily Gorbik, linux-s390,
	Heiko Carstens, Dmitry Kasatkin, Hari Bathini, Daniel Axtens,
	Christian Borntraeger, Philipp Rudo, Frank van der Linden, kexec,
	linux-kernel, Luis Chamberlain, Sven Schnelle,
	linux-security-module, Jessica Yu, linuxppc-dev, David S. Miller,
	Thiago Jung Bauermann, buendgen
In-Reply-To: <cover.1641900831.git.msuchanek@suse.de>

Add value for kexec appended signature and pass in key_being_used_for
enum rather than a string to verify_appended_signature to produce log
messages about the signature.

Signed-off-by: Michal Suchanek <msuchanek@suse.de>
---
 arch/powerpc/kexec/elf_64.c              |  2 +-
 arch/s390/kernel/machine_kexec_file.c    |  2 +-
 crypto/asymmetric_keys/asymmetric_type.c |  1 +
 include/linux/verification.h             |  4 +++-
 kernel/module.c                          |  3 ++-
 kernel/module_signing.c                  | 11 ++++++-----
 6 files changed, 14 insertions(+), 9 deletions(-)

diff --git a/arch/powerpc/kexec/elf_64.c b/arch/powerpc/kexec/elf_64.c
index 6dec8151ef73..c50869195d51 100644
--- a/arch/powerpc/kexec/elf_64.c
+++ b/arch/powerpc/kexec/elf_64.c
@@ -156,7 +156,7 @@ static void *elf64_load(struct kimage *image, char *kernel_buf,
 int elf64_verify_sig(const char *kernel, unsigned long kernel_len)
 {
 	return verify_appended_signature(kernel, &kernel_len, VERIFY_USE_PLATFORM_KEYRING,
-					 "kexec_file");
+					 VERIFYING_KEXEC_APPENDED_SIGNATURE);
 }
 #endif /* CONFIG_KEXEC_SIG */
 
diff --git a/arch/s390/kernel/machine_kexec_file.c b/arch/s390/kernel/machine_kexec_file.c
index c3deccf1da83..63eec38e3137 100644
--- a/arch/s390/kernel/machine_kexec_file.c
+++ b/arch/s390/kernel/machine_kexec_file.c
@@ -32,7 +32,7 @@ int s390_verify_sig(const char *kernel, unsigned long kernel_len)
 		return 0;
 
 	return verify_appended_signature(kernel, &kernel_len, VERIFY_USE_PLATFORM_KEYRING,
-					"kexec_file");
+					VERIFYING_KEXEC_APPENDED_SIGNATURE);
 }
 #endif /* CONFIG_KEXEC_SIG */
 
diff --git a/crypto/asymmetric_keys/asymmetric_type.c b/crypto/asymmetric_keys/asymmetric_type.c
index ad8af3d70ac0..6fd20eec3882 100644
--- a/crypto/asymmetric_keys/asymmetric_type.c
+++ b/crypto/asymmetric_keys/asymmetric_type.c
@@ -25,6 +25,7 @@ const char *const key_being_used_for[NR__KEY_BEING_USED_FOR] = {
 	[VERIFYING_KEXEC_PE_SIGNATURE]		= "kexec PE sig",
 	[VERIFYING_KEY_SIGNATURE]		= "key sig",
 	[VERIFYING_KEY_SELF_SIGNATURE]		= "key self sig",
+	[VERIFYING_KEXEC_APPENDED_SIGNATURE]	= "kexec appended sig",
 	[VERIFYING_UNSPECIFIED_SIGNATURE]	= "unspec sig",
 };
 EXPORT_SYMBOL_GPL(key_being_used_for);
diff --git a/include/linux/verification.h b/include/linux/verification.h
index 32db9287a7b0..f92c49443b4f 100644
--- a/include/linux/verification.h
+++ b/include/linux/verification.h
@@ -26,6 +26,7 @@ enum key_being_used_for {
 	VERIFYING_KEXEC_PE_SIGNATURE,
 	VERIFYING_KEY_SIGNATURE,
 	VERIFYING_KEY_SELF_SIGNATURE,
+	VERIFYING_KEXEC_APPENDED_SIGNATURE,
 	VERIFYING_UNSPECIFIED_SIGNATURE,
 	NR__KEY_BEING_USED_FOR
 };
@@ -61,7 +62,8 @@ extern int verify_pefile_signature(const void *pebuf, unsigned pelen,
 #endif
 
 int verify_appended_signature(const void *data, unsigned long *len,
-			      struct key *trusted_keys, const char *what);
+			      struct key *trusted_keys,
+			      enum key_being_used_for purpose);
 
 #endif /* CONFIG_SYSTEM_DATA_VERIFICATION */
 #endif /* _LINUX_VERIFY_PEFILE_H */
diff --git a/kernel/module.c b/kernel/module.c
index d91ca0f93a40..0a359dc6b690 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -2891,7 +2891,8 @@ static int module_sig_check(struct load_info *info, int flags)
 	 */
 	if (flags == 0) {
 		err = verify_appended_signature(mod, &info->len,
-						VERIFY_USE_SECONDARY_KEYRING, "module");
+						VERIFY_USE_SECONDARY_KEYRING,
+						VERIFYING_MODULE_SIGNATURE);
 		if (!err) {
 			info->sig_ok = true;
 			return 0;
diff --git a/kernel/module_signing.c b/kernel/module_signing.c
index 39a6dd7c6dd2..20857d2a15ca 100644
--- a/kernel/module_signing.c
+++ b/kernel/module_signing.c
@@ -19,17 +19,18 @@
  * @data: The data to be verified
  * @len: Size of @data.
  * @trusted_keys: Keyring to use for verification
- * @what: Informational string for log messages
+ * @purpose: The use to which the key is being put
  */
 int verify_appended_signature(const void *data, unsigned long *len,
-			      struct key *trusted_keys, const char *what)
+			      struct key *trusted_keys,
+			      enum key_being_used_for purpose)
 {
 	const unsigned long markerlen = sizeof(MODULE_SIG_STRING) - 1;
 	struct module_signature *ms;
 	unsigned long sig_len, modlen = *len;
 	int ret;
 
-	pr_devel("==>%s(,%lu)\n", __func__, modlen);
+	pr_devel("==>%s %s(,%lu)\n", __func__, key_being_used_for[purpose], modlen);
 
 	if (markerlen > modlen)
 		return -ENODATA;
@@ -44,7 +45,7 @@ int verify_appended_signature(const void *data, unsigned long *len,
 
 	ms = data + modlen - sizeof(*ms);
 
-	ret = mod_check_sig(ms, modlen, what);
+	ret = mod_check_sig(ms, modlen, key_being_used_for[purpose]);
 	if (ret)
 		return ret;
 
@@ -54,6 +55,6 @@ int verify_appended_signature(const void *data, unsigned long *len,
 
 	return verify_pkcs7_signature(data, modlen, data + modlen, sig_len,
 				      trusted_keys,
-				      VERIFYING_MODULE_SIGNATURE,
+				      purpose,
 				      NULL, NULL);
 }
-- 
2.31.1


^ permalink raw reply related

* [PATCH v5 4/6] module: strip the signature marker in the verification function.
From: Michal Suchanek @ 2022-01-11 11:37 UTC (permalink / raw)
  To: keyrings, linux-crypto, linux-integrity
  Cc: Nayna, Mimi Zohar, David Howells, Paul Mackerras,
	Alexander Gordeev, Rob Herring, Herbert Xu, Baoquan He,
	Christian Borntraeger, James Morris, Lakshmi Ramasubramanian,
	Michal Suchanek, Serge E. Hallyn, Vasily Gorbik, linux-s390,
	Heiko Carstens, Dmitry Kasatkin, Hari Bathini, Daniel Axtens,
	Christian Borntraeger, Philipp Rudo, Frank van der Linden, kexec,
	linux-kernel, Luis Chamberlain, Sven Schnelle,
	linux-security-module, Jessica Yu, linuxppc-dev, David S. Miller,
	Thiago Jung Bauermann, buendgen
In-Reply-To: <cover.1641900831.git.msuchanek@suse.de>

It is stripped by each caller separately.

Note: this changes the error for kexec_file from EKEYREJECTED to ENODATA
when the signature marker is missing.

Signed-off-by: Michal Suchanek <msuchanek@suse.de>
---
v3: - Philipp Rudo <prudo@redhat.com>: Update the commit with note about
      change of raturn value
    - the module_signature.h is now no longer needed for kexec_file
---
 arch/powerpc/kexec/elf_64.c           | 11 -----------
 arch/s390/kernel/machine_kexec_file.c | 11 -----------
 kernel/module.c                       |  7 +------
 kernel/module_signing.c               | 12 ++++++++++--
 4 files changed, 11 insertions(+), 30 deletions(-)

diff --git a/arch/powerpc/kexec/elf_64.c b/arch/powerpc/kexec/elf_64.c
index 64cd314cce0d..6dec8151ef73 100644
--- a/arch/powerpc/kexec/elf_64.c
+++ b/arch/powerpc/kexec/elf_64.c
@@ -24,7 +24,6 @@
 #include <linux/slab.h>
 #include <linux/types.h>
 #include <linux/verification.h>
-#include <linux/module_signature.h>
 
 static void *elf64_load(struct kimage *image, char *kernel_buf,
 			unsigned long kernel_len, char *initrd,
@@ -156,16 +155,6 @@ static void *elf64_load(struct kimage *image, char *kernel_buf,
 #ifdef CONFIG_KEXEC_SIG
 int elf64_verify_sig(const char *kernel, unsigned long kernel_len)
 {
-	const unsigned long marker_len = sizeof(MODULE_SIG_STRING) - 1;
-
-	if (marker_len > kernel_len)
-		return -EKEYREJECTED;
-
-	if (memcmp(kernel + kernel_len - marker_len, MODULE_SIG_STRING,
-		   marker_len))
-		return -EKEYREJECTED;
-	kernel_len -= marker_len;
-
 	return verify_appended_signature(kernel, &kernel_len, VERIFY_USE_PLATFORM_KEYRING,
 					 "kexec_file");
 }
diff --git a/arch/s390/kernel/machine_kexec_file.c b/arch/s390/kernel/machine_kexec_file.c
index 345f2eab6e04..c3deccf1da83 100644
--- a/arch/s390/kernel/machine_kexec_file.c
+++ b/arch/s390/kernel/machine_kexec_file.c
@@ -12,7 +12,6 @@
 #include <linux/elf.h>
 #include <linux/errno.h>
 #include <linux/kexec.h>
-#include <linux/module_signature.h>
 #include <linux/verification.h>
 #include <linux/vmalloc.h>
 #include <asm/boot_data.h>
@@ -28,20 +27,10 @@ const struct kexec_file_ops * const kexec_file_loaders[] = {
 #ifdef CONFIG_KEXEC_SIG
 int s390_verify_sig(const char *kernel, unsigned long kernel_len)
 {
-	const unsigned long marker_len = sizeof(MODULE_SIG_STRING) - 1;
-
 	/* Skip signature verification when not secure IPLed. */
 	if (!ipl_secure_flag)
 		return 0;
 
-	if (marker_len > kernel_len)
-		return -EKEYREJECTED;
-
-	if (memcmp(kernel + kernel_len - marker_len, MODULE_SIG_STRING,
-		   marker_len))
-		return -EKEYREJECTED;
-	kernel_len -= marker_len;
-
 	return verify_appended_signature(kernel, &kernel_len, VERIFY_USE_PLATFORM_KEYRING,
 					"kexec_file");
 }
diff --git a/kernel/module.c b/kernel/module.c
index 8481933dfa92..d91ca0f93a40 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -2882,7 +2882,6 @@ static inline void kmemleak_load_module(const struct module *mod,
 static int module_sig_check(struct load_info *info, int flags)
 {
 	int err = -ENODATA;
-	const unsigned long markerlen = sizeof(MODULE_SIG_STRING) - 1;
 	const char *reason;
 	const void *mod = info->hdr;
 
@@ -2890,11 +2889,7 @@ static int module_sig_check(struct load_info *info, int flags)
 	 * Require flags == 0, as a module with version information
 	 * removed is no longer the module that was signed
 	 */
-	if (flags == 0 &&
-	    info->len > markerlen &&
-	    memcmp(mod + info->len - markerlen, MODULE_SIG_STRING, markerlen) == 0) {
-		/* We truncate the module to discard the signature */
-		info->len -= markerlen;
+	if (flags == 0) {
 		err = verify_appended_signature(mod, &info->len,
 						VERIFY_USE_SECONDARY_KEYRING, "module");
 		if (!err) {
diff --git a/kernel/module_signing.c b/kernel/module_signing.c
index 30149969f21f..39a6dd7c6dd2 100644
--- a/kernel/module_signing.c
+++ b/kernel/module_signing.c
@@ -15,8 +15,7 @@
 #include "module-internal.h"
 
 /**
- * verify_appended_signature - Verify the signature on a module with the
- * signature marker stripped.
+ * verify_appended_signature - Verify the signature on a module
  * @data: The data to be verified
  * @len: Size of @data.
  * @trusted_keys: Keyring to use for verification
@@ -25,12 +24,21 @@
 int verify_appended_signature(const void *data, unsigned long *len,
 			      struct key *trusted_keys, const char *what)
 {
+	const unsigned long markerlen = sizeof(MODULE_SIG_STRING) - 1;
 	struct module_signature *ms;
 	unsigned long sig_len, modlen = *len;
 	int ret;
 
 	pr_devel("==>%s(,%lu)\n", __func__, modlen);
 
+	if (markerlen > modlen)
+		return -ENODATA;
+
+	if (memcmp(data + modlen - markerlen, MODULE_SIG_STRING,
+		   markerlen))
+		return -ENODATA;
+	modlen -= markerlen;
+
 	if (modlen <= sizeof(*ms))
 		return -EBADMSG;
 
-- 
2.31.1


^ permalink raw reply related

* Re: [PATCH 11/13] powerpc64/bpf elfv2: Setup kernel TOC in r2 on entry
From: Naveen N. Rao @ 2022-01-11 10:31 UTC (permalink / raw)
  To: Alexei Starovoitov, Christophe Leroy, Daniel Borkmann,
	Michael Ellerman
  Cc: ykaliuta@redhat.com, johan.almbladh@anyfinetworks.com, Jiri Olsa,
	song@kernel.org, bpf@vger.kernel.org,
	linuxppc-dev@lists.ozlabs.org, Hari Bathini
In-Reply-To: <d0e28f07-c24c-200d-de04-5d27c651a5e6@csgroup.eu>

Christophe Leroy wrote:
> 
> 
> Le 06/01/2022 à 12:45, Naveen N. Rao a écrit :
>> In preparation for using kernel TOC, load the same in r2 on entry. With
>> elfv1, the kernel TOC is already setup by our caller so we just emit a
>> nop. We adjust the number of instructions to skip on a tail call
>> accordingly.
>> 
>> Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
>> ---
>>   arch/powerpc/net/bpf_jit_comp64.c | 8 +++++++-
>>   1 file changed, 7 insertions(+), 1 deletion(-)
>> 
>> diff --git a/arch/powerpc/net/bpf_jit_comp64.c b/arch/powerpc/net/bpf_jit_comp64.c
>> index ce4fc59bbd6a92..e05b577d95bf11 100644
>> --- a/arch/powerpc/net/bpf_jit_comp64.c
>> +++ b/arch/powerpc/net/bpf_jit_comp64.c
>> @@ -73,6 +73,12 @@ void bpf_jit_build_prologue(u32 *image, struct codegen_context *ctx)
>>   {
>>   	int i;
>>   
>> +#ifdef PPC64_ELF_ABI_v2
>> +	PPC_BPF_LL(_R2, _R13, offsetof(struct paca_struct, kernel_toc));
>> +#else
>> +	EMIT(PPC_RAW_NOP());
>> +#endif
> 
> Can we avoid the #ifdef, using
> 
> 	if (__is_defined(PPC64_ELF_ABI_v2))
> 		PPC_BPF_LL(_R2, _R13, offsetof(struct paca_struct, kernel_toc));
> 	else
> 		EMIT(PPC_RAW_NOP());

Hmm... that doesn't work for me. Is __is_defined() expected to work with 
macros other than CONFIG options?

> 
>> +
>>   	/*
>>   	 * Initialize tail_call_cnt if we do tail calls.
>>   	 * Otherwise, put in NOPs so that it can be skipped when we are
>> @@ -87,7 +93,7 @@ void bpf_jit_build_prologue(u32 *image, struct codegen_context *ctx)
>>   		EMIT(PPC_RAW_NOP());
>>   	}
>>   
>> -#define BPF_TAILCALL_PROLOGUE_SIZE	8
>> +#define BPF_TAILCALL_PROLOGUE_SIZE	12
> 
> Why not change that for v2 ABI only instead of adding a NOP ? ABI won't 
> change during runtime AFAIU

Yeah, I wanted to keep this simple and I felt an additional nop 
shouldn't matter too much. But, I guess we can get rid of 
BPF_TAILCALL_PROLOGUE_SIZE since the only user is the function emitting 
a tail call. I will submit that as a separate cleanup unless I need to 
redo this series.

Thanks for the reviews!
- Naveen


^ permalink raw reply

* RE: [PATCH 5/5] compat: consolidate the compat_flock{,64} definition
From: David Laight @ 2022-01-11  9:52 UTC (permalink / raw)
  To: 'Christoph Hellwig', Arnd Bergmann
  Cc: linux-arch@vger.kernel.org, linux-s390@vger.kernel.org,
	linux-parisc@vger.kernel.org, x86@kernel.org,
	linux-kernel@vger.kernel.org, linux-mips@vger.kernel.org, Guo Ren,
	sparclinux@vger.kernel.org, linuxppc-dev@lists.ozlabs.org,
	linux-arm-kernel@lists.infradead.org
In-Reply-To: <20220111083515.502308-6-hch@lst.de>

From: Christoph Hellwig
> Sent: 11 January 2022 08:35
> 
> Provide a single common definition for the compat_flock and
> compat_flock64 structures using the same tricks as for the native
> variants.  Another extra define is added for the packing required on
> x86.
> 
...
>  /*
> - * IA32 uses 4 byte alignment for 64 bit quantities,
> - * so we need to pack this structure.
> + * IA32 uses 4 byte alignment for 64 bit quantities, so we need to pack the
> + * compat flock64 structure.
>   */
...
> +#define __ARCH_NEED_COMPAT_FLOCK64_PACKED

Maybe:
#define __ARCH_COMPAT_FLOCK64_ATTR (packed),(aligned(4)

...
Delete this bit:
> +#ifdef __ARCH_NEED_COMPAT_FLOCK64_PACKED
> +#define __ARCH_COMPAT_FLOCK64_PACK	__attribute__((packed))
> +#else
> +#define __ARCH_COMPAT_FLOCK64_PACK
> +#endif
...
> +struct compat_flock64 {
> +	short		l_type;
> +	short		l_whence;
> +	compat_loff_t	l_start;
> +	compat_loff_t	l_len;
> +	compat_pid_t	l_pid;
> +#ifdef __ARCH_COMPAT_FLOCK64_PAD
> +	__ARCH_COMPAT_FLOCK64_PAD
> +#endif
> +} __ARCH_COMPAT_FLOCK64_PACK;

then:
#ifdef __ARCH_COMPAT_FLOCK64_ATTR
} __attribute__(__ARCH_COMPAT_FLOCK64_ATTR);
#else
};
#endif

Makes it a bit more like the xxx_PAD bits.
Although the trailing ; does cause a bit of grief.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)


^ permalink raw reply

* Re: [5.16.0] build error on unrecognized opcode: ptesync
From: Christophe Leroy @ 2022-01-11  9:51 UTC (permalink / raw)
  To: Mike; +Cc: linuxppc-dev@lists.ozlabs.org
In-Reply-To: <CANtoAtOVjnvvti1rnrwpR9ybvpQJzjq48tdQbhsqJWncgoTmbw@mail.gmail.com>



Le 11/01/2022 à 10:32, Mike a écrit :
> I managed to fix it in the end, patch attached, though i should have
> done a $(call cc-option-, -maltivec, -mabi=altivec) in the
> arch/powerpc/mm/Makefile
>   I wrongly assumed that the manual i had downloaded at 4.44am was for
> 32bit ppc only and found ptesync to be ppc64 only.
> 
> binutils-2.37.50 - GNU assembler version 2.37.50 (powerpc-linux-gnu)
> using BFD version (GNU Binutils for Debian) 2.37.50.20220106
> gcc version 11.2.0 (Debian 11.2.0-13)
> ld.lld is missing but with LLVM/CLANG and LD=ld.bfd
> arch/powerpc/kernel/vdso32/gettimeofday.S:72:8:
> error: unsupported directive '.stabs'    .stabs
> "_restgpr_31_x:F-1",36,0,0,_restgpr_31_x; .globl _restgpr_31_x;
> _restgpr_31_x:
> 
> Attached the config i'm using, and the debian config 5.15.0-2. It's
> still building.

Ok, I tried with your config on my Fedora Core 35 where I have:

powerpc64-linux-gnu-gcc (GCC) 11.2.1 20210728 (Red Hat Cross 11.2.1-1)
GNU ld version 2.37-3.fc35

 From packages:
- binutils-powerpc64-linux-gnu-2.37-3.fc35.x86_64
- gcc-powerpc64-linux-gnu-11.2.1-1.fc35.x86_64

And I don't have the problems you mention, so it must be something 
special with Debian GCC.


Your change regarding ptesync is probably OK but is fragile I think, 
because for instance there is also a 'ptesync' in 
arch/powerpc/mm/pageattr.c and probably many other places.

Also please prefer CONFIG_PPC64 to __powerpc64__

Regarding the DSSALL issue, the following commit will probably help:

d51f86cfd8e3 ("powerpc/mm: Switch obsolete dssall to .long")

Regarding the .stabs with LLVM there is a patch at 
https://patchwork.ozlabs.org/project/linuxppc-dev/patch/68932ec2ba6b868d35006b96e90f0890f3da3c05.1638273868.git.christophe.leroy@csgroup.eu/

Thanks
Christophe



> 
> Cheers
> Michael
> 
> On Tue, 11 Jan 2022 at 07:20, Christophe Leroy
> <christophe.leroy@csgroup.eu> wrote:
>>
>>
>>
>> Le 10/01/2022 à 13:32, Mike a écrit :
>>> Hey, so I originally sat down to compile the fast headers V2 patch, but
>>> quickly discovered other things at play, and grabbed 5.16.0 a few hours
>>> after it lifted off,  arch/powerpc/mm/mmu_context.c I had to
>>> specifically say had to include -maltivec or it barfed on a 'dssall',
>>> I'm fine with that, I've spent years in kernel land, I can deal with
>>> that, then came arch/powerpc/lib/step.c with the ptesync. This seems
>>> like a totally normal instruction that shouldn't need any extra flags or
>>> anything, yet the assembler throws up, and no flag I can think of fixes
>>> it. This is a G4 7447. I reverted back to the Debian 5.15. defconfig
>>> before dropping this mail as I had tweaked my config to be more G4.
>>>
>>
>> Hi Mike,
>>
>> Can you provide a bit more details about your setup and config ?
>>
>> Are you using GCC or LLVM ?
>> What version of GCC and BINUTILS or what version of LLVM ?
>>
>> What is DEBIAN defconfig ? Does it correspond to one of the standard
>> mainline kernel defconfigs ? If not can you provide it ?
>>
>> Thanks
>> Christophe

^ permalink raw reply

* Re: [powerpc/merge] PMU: Kernel warning while running pmu/ebb selftests
From: Sachin Sant @ 2022-01-11  8:55 UTC (permalink / raw)
  To: Madhavan Srinivasan, Athira Rajeev; +Cc: linuxppc-dev
In-Reply-To: <4645BEDB-75D8-4178-A5AE-C2AE8693BE96@linux.vnet.ibm.com>



> On 18-Dec-2021, at 3:38 PM, Sachin Sant <sachinp@linux.vnet.ibm.com> wrote:
> 
> While running kernel selftests (lost_exception_test) against latest
> powerpc merge/next branch code (5.16.0-rc5-03218-g798527287598)
> following warning is seen:
> 
Ping. I continue to see this problem.

> [  172.851380] ------------[ cut here ]------------
> [  172.851391] WARNING: CPU: 8 PID: 2901 at arch/powerpc/include/asm/hw_irq.h:246 power_pmu_disable+0x270/0x280
> [  172.851402] Modules linked in: dm_mod bonding nft_ct nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 ip_set nf_tables rfkill nfnetlink sunrpc xfs libcrc32c pseries_rng xts vmx_crypto uio_pdrv_genirq uio sch_fq_codel ip_tables ext4 mbcache jbd2 sd_mod t10_pi sg ibmvscsi ibmveth scsi_transport_srp fuse
> [  172.851442] CPU: 8 PID: 2901 Comm: lost_exception_ Not tainted 5.16.0-rc5-03218-g798527287598 #2
> [  172.851451] NIP:  c00000000013d600 LR: c00000000013d5a4 CTR: c00000000013b180
> [  172.851458] REGS: c000000017687860 TRAP: 0700   Not tainted  (5.16.0-rc5-03218-g798527287598)
> [  172.851465] MSR:  8000000000029033 <SF,EE,ME,IR,DR,RI,LE>  CR: 48004884  XER: 20040000
> [  172.851482] CFAR: c00000000013d5b4 IRQMASK: 1 
> [  172.851482] GPR00: c00000000013d5a4 c000000017687b00 c000000002a10600 0000000000000004 
> [  172.851482] GPR04: 0000000082004000 c0000008ba08f0a8 0000000000000000 00000008b7ed0000 
> [  172.851482] GPR08: 00000000446194f6 0000000000008000 c00000000013b118 c000000000d58e68 
> [  172.851482] GPR12: c00000000013d390 c00000001ec54a80 0000000000000000 0000000000000000 
> [  172.851482] GPR16: 0000000000000000 0000000000000000 c000000015d5c708 c0000000025396d0 
> [  172.851482] GPR20: 0000000000000000 0000000000000000 c00000000a3bbf40 0000000000000003 
> [  172.851482] GPR24: 0000000000000000 c0000008ba097400 c0000000161e0d00 c00000000a3bb600 
> [  172.851482] GPR28: c000000015d5c700 0000000000000001 0000000082384090 c0000008ba0020d8 
> [  172.851549] NIP [c00000000013d600] power_pmu_disable+0x270/0x280
> [  172.851557] LR [c00000000013d5a4] power_pmu_disable+0x214/0x280
> [  172.851565] Call Trace:
> [  172.851568] [c000000017687b00] [c00000000013d5a4] power_pmu_disable+0x214/0x280 (unreliable)
> [  172.851579] [c000000017687b40] [c0000000003403ac] perf_pmu_disable+0x4c/0x60
> [  172.851588] [c000000017687b60] [c0000000003445e4] __perf_event_task_sched_out+0x1d4/0x660
> [  172.851596] [c000000017687c50] [c000000000d1175c] __schedule+0xbcc/0x12a0
> [  172.851602] [c000000017687d60] [c000000000d11ea8] schedule+0x78/0x140
> [  172.851608] [c000000017687d90] [c0000000001a8080] sys_sched_yield+0x20/0x40
> [  172.851615] [c000000017687db0] [c0000000000334dc] system_call_exception+0x18c/0x380
> [  172.851622] [c000000017687e10] [c00000000000c74c] system_call_common+0xec/0x268
> [  172.851629] --- interrupt: c00 at 0x7fffa9d0d2fc
> [  172.851633] NIP:  00007fffa9d0d2fc LR: 0000000010001914 CTR: 0000000000000000
> [  172.851638] REGS: c000000017687e80 TRAP: 0c00   Not tainted  (5.16.0-rc5-03218-g798527287598)
> [  172.851643] MSR:  800000000280f033 <SF,VEC,VSX,EE,PR,FP,ME,IR,DR,RI,LE>  CR: 28000288  XER: 00000000
> [  172.851657] IRQMASK: 0 
> [  172.851657] GPR00: 000000000000009e 00007fffe44c0b40 00007fffa9e07300 0000000000000000 
> [  172.851657] GPR04: 00007fffe44c0c28 0000000000000018 000000000000000a 0000000000000008 
> [  172.851657] GPR08: 0000000000000007 0000000000000000 0000000000000000 0000000000000000 
> [  172.851657] GPR12: 0000000000000000 00007fffa9eaa270 0000000000000000 0000000000000000 
> [  172.851657] GPR16: 0000000000000000 0000000000000000 0000000000000000 0000000000000000 
> [  172.851657] GPR20: 0000000000000000 0000000000000000 0000000000000000 0000000000000000 
> [  172.851657] GPR24: 0000000000000190 0000000000000000 00000000000186a0 00000000000f423f 
> [  172.851657] GPR28: 0000000010021650 0000000000000198 0000000010020238 000000000000d446 
> [  172.851711] NIP [00007fffa9d0d2fc] 0x7fffa9d0d2fc
> [  172.851715] LR [0000000010001914] 0x10001914
> [  172.851719] --- interrupt: c00
> [  172.851722] Instruction dump:
> [  172.851725] 71490001 81280078 552905ac 79290020 2fa90000 4182fe9c 4bffff88 60000000 
> [  172.851735] 4bee4729 60000000 9bdf0014 4bfffe00 <0fe00000> 60000000 60000000 60000000 
> [  172.851745] ---[ end trace 10a1b687c9c436f7 ]—
> 
> CONFIG_PPC_IRQ_SOFT_MASK_DEBUG is enabled for this kernel.
> 
> The code in question was last changed by following commit:
> 
> commit 2c9ac51b850d 
> powerpc/perf: Fix PMU callbacks to clear pending PMI before resetting an overflown PMC
> 
> Reverting this commit helps.
> 
> Thanks
> -Sachin


^ permalink raw reply

* [PATCH 5/5] compat: consolidate the compat_flock{,64} definition
From: Christoph Hellwig @ 2022-01-11  8:35 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: linux-arch, linux-s390, linux-parisc, x86, linux-kernel,
	linux-mips, Guo Ren, sparclinux, linuxppc-dev, linux-arm-kernel
In-Reply-To: <20220111083515.502308-1-hch@lst.de>

Provide a single common definition for the compat_flock and
compat_flock64 structures using the same tricks as for the native
variants.  Another extra define is added for the packing required on
x86.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 arch/arm64/include/asm/compat.h   | 16 ----------------
 arch/mips/include/asm/compat.h    | 19 ++-----------------
 arch/parisc/include/asm/compat.h  | 16 ----------------
 arch/powerpc/include/asm/compat.h | 16 ----------------
 arch/s390/include/asm/compat.h    | 16 ----------------
 arch/sparc/include/asm/compat.h   | 18 +-----------------
 arch/x86/include/asm/compat.h     | 20 +++-----------------
 include/linux/compat.h            | 31 +++++++++++++++++++++++++++++++
 8 files changed, 37 insertions(+), 115 deletions(-)

diff --git a/arch/arm64/include/asm/compat.h b/arch/arm64/include/asm/compat.h
index 2763287654081..e0faec1984a1c 100644
--- a/arch/arm64/include/asm/compat.h
+++ b/arch/arm64/include/asm/compat.h
@@ -65,22 +65,6 @@ struct compat_stat {
 	compat_ulong_t	__unused4[2];
 };
 
-struct compat_flock {
-	short		l_type;
-	short		l_whence;
-	compat_off_t	l_start;
-	compat_off_t	l_len;
-	compat_pid_t	l_pid;
-};
-
-struct compat_flock64 {
-	short		l_type;
-	short		l_whence;
-	compat_loff_t	l_start;
-	compat_loff_t	l_len;
-	compat_pid_t	l_pid;
-};
-
 struct compat_statfs {
 	int		f_type;
 	int		f_bsize;
diff --git a/arch/mips/include/asm/compat.h b/arch/mips/include/asm/compat.h
index 6a350c1f70d7e..6d6e5a451f4d9 100644
--- a/arch/mips/include/asm/compat.h
+++ b/arch/mips/include/asm/compat.h
@@ -55,23 +55,8 @@ struct compat_stat {
 	s32		st_pad4[14];
 };
 
-struct compat_flock {
-	short		l_type;
-	short		l_whence;
-	compat_off_t	l_start;
-	compat_off_t	l_len;
-	s32		l_sysid;
-	compat_pid_t	l_pid;
-	s32		pad[4];
-};
-
-struct compat_flock64 {
-	short		l_type;
-	short		l_whence;
-	compat_loff_t	l_start;
-	compat_loff_t	l_len;
-	compat_pid_t	l_pid;
-};
+#define __ARCH_COMPAT_FLOCK_EXTRA_SYSID		s32 l_sysid;
+#define __ARCH_COMPAT_FLOCK_PAD			s32 pad[4];
 
 struct compat_statfs {
 	int		f_type;
diff --git a/arch/parisc/include/asm/compat.h b/arch/parisc/include/asm/compat.h
index c04f5a637c390..a1e4534d80509 100644
--- a/arch/parisc/include/asm/compat.h
+++ b/arch/parisc/include/asm/compat.h
@@ -53,22 +53,6 @@ struct compat_stat {
 	u32			st_spare4[3];
 };
 
-struct compat_flock {
-	short			l_type;
-	short			l_whence;
-	compat_off_t		l_start;
-	compat_off_t		l_len;
-	compat_pid_t		l_pid;
-};
-
-struct compat_flock64 {
-	short			l_type;
-	short			l_whence;
-	compat_loff_t		l_start;
-	compat_loff_t		l_len;
-	compat_pid_t		l_pid;
-};
-
 struct compat_statfs {
 	s32		f_type;
 	s32		f_bsize;
diff --git a/arch/powerpc/include/asm/compat.h b/arch/powerpc/include/asm/compat.h
index 83d8f70779cbc..5ef3c7c83c343 100644
--- a/arch/powerpc/include/asm/compat.h
+++ b/arch/powerpc/include/asm/compat.h
@@ -44,22 +44,6 @@ struct compat_stat {
 	u32		__unused4[2];
 };
 
-struct compat_flock {
-	short		l_type;
-	short		l_whence;
-	compat_off_t	l_start;
-	compat_off_t	l_len;
-	compat_pid_t	l_pid;
-};
-
-struct compat_flock64 {
-	short		l_type;
-	short		l_whence;
-	compat_loff_t	l_start;
-	compat_loff_t	l_len;
-	compat_pid_t	l_pid;
-};
-
 struct compat_statfs {
 	int		f_type;
 	int		f_bsize;
diff --git a/arch/s390/include/asm/compat.h b/arch/s390/include/asm/compat.h
index 0f14b3188b1bb..07f04d37068b6 100644
--- a/arch/s390/include/asm/compat.h
+++ b/arch/s390/include/asm/compat.h
@@ -102,22 +102,6 @@ struct compat_stat {
 	u32		__unused5;
 };
 
-struct compat_flock {
-	short		l_type;
-	short		l_whence;
-	compat_off_t	l_start;
-	compat_off_t	l_len;
-	compat_pid_t	l_pid;
-};
-
-struct compat_flock64 {
-	short		l_type;
-	short		l_whence;
-	compat_loff_t	l_start;
-	compat_loff_t	l_len;
-	compat_pid_t	l_pid;
-};
-
 struct compat_statfs {
 	u32		f_type;
 	u32		f_bsize;
diff --git a/arch/sparc/include/asm/compat.h b/arch/sparc/include/asm/compat.h
index 108078751bb5a..d78fb44942e0f 100644
--- a/arch/sparc/include/asm/compat.h
+++ b/arch/sparc/include/asm/compat.h
@@ -75,23 +75,7 @@ struct compat_stat64 {
 	unsigned int	__unused5;
 };
 
-struct compat_flock {
-	short		l_type;
-	short		l_whence;
-	compat_off_t	l_start;
-	compat_off_t	l_len;
-	compat_pid_t	l_pid;
-	short		__unused;
-};
-
-struct compat_flock64 {
-	short		l_type;
-	short		l_whence;
-	compat_loff_t	l_start;
-	compat_loff_t	l_len;
-	compat_pid_t	l_pid;
-	short		__unused;
-};
+#define __ARCH_COMPAT_FLOCK_PAD		short __unused;
 
 struct compat_statfs {
 	int		f_type;
diff --git a/arch/x86/include/asm/compat.h b/arch/x86/include/asm/compat.h
index 8d19a212f4f26..de794d8958663 100644
--- a/arch/x86/include/asm/compat.h
+++ b/arch/x86/include/asm/compat.h
@@ -50,25 +50,11 @@ struct compat_stat {
 	u32		__unused5;
 };
 
-struct compat_flock {
-	short		l_type;
-	short		l_whence;
-	compat_off_t	l_start;
-	compat_off_t	l_len;
-	compat_pid_t	l_pid;
-};
-
 /*
- * IA32 uses 4 byte alignment for 64 bit quantities,
- * so we need to pack this structure.
+ * IA32 uses 4 byte alignment for 64 bit quantities, so we need to pack the
+ * compat flock64 structure.
  */
-struct compat_flock64 {
-	short		l_type;
-	short		l_whence;
-	compat_loff_t	l_start;
-	compat_loff_t	l_len;
-	compat_pid_t	l_pid;
-} __attribute__((packed));
+#define __ARCH_NEED_COMPAT_FLOCK64_PACKED
 
 struct compat_statfs {
 	int		f_type;
diff --git a/include/linux/compat.h b/include/linux/compat.h
index 1c758b0e03598..a0481fe6c5d51 100644
--- a/include/linux/compat.h
+++ b/include/linux/compat.h
@@ -258,6 +258,37 @@ struct compat_rlimit {
 	compat_ulong_t	rlim_max;
 };
 
+#ifdef __ARCH_NEED_COMPAT_FLOCK64_PACKED
+#define __ARCH_COMPAT_FLOCK64_PACK	__attribute__((packed))
+#else
+#define __ARCH_COMPAT_FLOCK64_PACK
+#endif
+
+struct compat_flock {
+	short			l_type;
+	short			l_whence;
+	compat_off_t		l_start;
+	compat_off_t		l_len;
+#ifdef __ARCH_COMPAT_FLOCK_EXTRA_SYSID
+	__ARCH_COMPAT_FLOCK_EXTRA_SYSID
+#endif
+	compat_pid_t		l_pid;
+#ifdef __ARCH_COMPAT_FLOCK_PAD
+	__ARCH_COMPAT_FLOCK_PAD
+#endif
+};
+
+struct compat_flock64 {
+	short		l_type;
+	short		l_whence;
+	compat_loff_t	l_start;
+	compat_loff_t	l_len;
+	compat_pid_t	l_pid;
+#ifdef __ARCH_COMPAT_FLOCK64_PAD
+	__ARCH_COMPAT_FLOCK64_PAD
+#endif
+} __ARCH_COMPAT_FLOCK64_PACK;
+
 struct compat_rusage {
 	struct old_timeval32 ru_utime;
 	struct old_timeval32 ru_stime;
-- 
2.30.2


^ permalink raw reply related

* [PATCH 4/5] uapi: always define F_GETLK64/F_SETLK64/F_SETLKW64 in fcntl.h
From: Christoph Hellwig @ 2022-01-11  8:35 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: linux-arch, linux-s390, linux-parisc, x86, linux-kernel,
	linux-mips, Guo Ren, sparclinux, linuxppc-dev, linux-arm-kernel
In-Reply-To: <20220111083515.502308-1-hch@lst.de>

The fcntl F_GETLK64/F_SETLK64/F_SETLKW64 are only implemented for the
32-bit syscall APIs, but we also need them for compat handling on 64-bit
builds.  Redefining them is error prone (as shown by the example that
parisc gets it wrong currently), so we should use the same defines for
both case.  In theory we could try to hide them from userspace, but
given that only MIPS actually gets that right, while the asm-generic
version used by most architectures relies on a Kconfig symbol that can't
be relied on to be set properly by userspace is a clear indicator to not
bother.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 arch/arm64/include/asm/compat.h        | 4 ----
 arch/mips/include/asm/compat.h         | 4 ----
 arch/mips/include/uapi/asm/fcntl.h     | 2 --
 arch/powerpc/include/asm/compat.h      | 4 ----
 arch/s390/include/asm/compat.h         | 4 ----
 arch/sparc/include/asm/compat.h        | 4 ----
 arch/x86/include/asm/compat.h          | 4 ----
 include/uapi/asm-generic/fcntl.h       | 2 --
 tools/include/uapi/asm-generic/fcntl.h | 2 --
 9 files changed, 30 deletions(-)

diff --git a/arch/arm64/include/asm/compat.h b/arch/arm64/include/asm/compat.h
index eaa6ca062d89b..2763287654081 100644
--- a/arch/arm64/include/asm/compat.h
+++ b/arch/arm64/include/asm/compat.h
@@ -73,10 +73,6 @@ struct compat_flock {
 	compat_pid_t	l_pid;
 };
 
-#define F_GETLK64	12	/*  using 'struct flock64' */
-#define F_SETLK64	13
-#define F_SETLKW64	14
-
 struct compat_flock64 {
 	short		l_type;
 	short		l_whence;
diff --git a/arch/mips/include/asm/compat.h b/arch/mips/include/asm/compat.h
index bbb3bc5a42fd8..6a350c1f70d7e 100644
--- a/arch/mips/include/asm/compat.h
+++ b/arch/mips/include/asm/compat.h
@@ -65,10 +65,6 @@ struct compat_flock {
 	s32		pad[4];
 };
 
-#define F_GETLK64	33
-#define F_SETLK64	34
-#define F_SETLKW64	35
-
 struct compat_flock64 {
 	short		l_type;
 	short		l_whence;
diff --git a/arch/mips/include/uapi/asm/fcntl.h b/arch/mips/include/uapi/asm/fcntl.h
index 9e44ac810db94..1769fc50d35f0 100644
--- a/arch/mips/include/uapi/asm/fcntl.h
+++ b/arch/mips/include/uapi/asm/fcntl.h
@@ -44,11 +44,9 @@
 #define F_SETOWN	24	/*  for sockets. */
 #define F_GETOWN	23	/*  for sockets. */
 
-#ifndef __mips64
 #define F_GETLK64	33	/*  using 'struct flock64' */
 #define F_SETLK64	34
 #define F_SETLKW64	35
-#endif
 
 #if _MIPS_SIM != _MIPS_SIM_ABI64
 #define __ARCH_FLOCK_EXTRA_SYSID	long l_sysid;
diff --git a/arch/powerpc/include/asm/compat.h b/arch/powerpc/include/asm/compat.h
index 7afc96fb6524b..83d8f70779cbc 100644
--- a/arch/powerpc/include/asm/compat.h
+++ b/arch/powerpc/include/asm/compat.h
@@ -52,10 +52,6 @@ struct compat_flock {
 	compat_pid_t	l_pid;
 };
 
-#define F_GETLK64	12	/*  using 'struct flock64' */
-#define F_SETLK64	13
-#define F_SETLKW64	14
-
 struct compat_flock64 {
 	short		l_type;
 	short		l_whence;
diff --git a/arch/s390/include/asm/compat.h b/arch/s390/include/asm/compat.h
index cdc7ae72529d8..0f14b3188b1bb 100644
--- a/arch/s390/include/asm/compat.h
+++ b/arch/s390/include/asm/compat.h
@@ -110,10 +110,6 @@ struct compat_flock {
 	compat_pid_t	l_pid;
 };
 
-#define F_GETLK64       12
-#define F_SETLK64       13
-#define F_SETLKW64      14    
-
 struct compat_flock64 {
 	short		l_type;
 	short		l_whence;
diff --git a/arch/sparc/include/asm/compat.h b/arch/sparc/include/asm/compat.h
index bd949fcf9d63b..108078751bb5a 100644
--- a/arch/sparc/include/asm/compat.h
+++ b/arch/sparc/include/asm/compat.h
@@ -84,10 +84,6 @@ struct compat_flock {
 	short		__unused;
 };
 
-#define F_GETLK64	12
-#define F_SETLK64	13
-#define F_SETLKW64	14
-
 struct compat_flock64 {
 	short		l_type;
 	short		l_whence;
diff --git a/arch/x86/include/asm/compat.h b/arch/x86/include/asm/compat.h
index 7516e4199b3c6..8d19a212f4f26 100644
--- a/arch/x86/include/asm/compat.h
+++ b/arch/x86/include/asm/compat.h
@@ -58,10 +58,6 @@ struct compat_flock {
 	compat_pid_t	l_pid;
 };
 
-#define F_GETLK64	12	/*  using 'struct flock64' */
-#define F_SETLK64	13
-#define F_SETLKW64	14
-
 /*
  * IA32 uses 4 byte alignment for 64 bit quantities,
  * so we need to pack this structure.
diff --git a/include/uapi/asm-generic/fcntl.h b/include/uapi/asm-generic/fcntl.h
index 98f4ff165b776..43d7c44031be0 100644
--- a/include/uapi/asm-generic/fcntl.h
+++ b/include/uapi/asm-generic/fcntl.h
@@ -116,13 +116,11 @@
 #define F_GETSIG	11	/* for sockets. */
 #endif
 
-#ifndef CONFIG_64BIT
 #ifndef F_GETLK64
 #define F_GETLK64	12	/*  using 'struct flock64' */
 #define F_SETLK64	13
 #define F_SETLKW64	14
 #endif
-#endif
 
 #ifndef F_SETOWN_EX
 #define F_SETOWN_EX	15
diff --git a/tools/include/uapi/asm-generic/fcntl.h b/tools/include/uapi/asm-generic/fcntl.h
index bf961a71802e0..6e16722026f39 100644
--- a/tools/include/uapi/asm-generic/fcntl.h
+++ b/tools/include/uapi/asm-generic/fcntl.h
@@ -115,13 +115,11 @@
 #define F_GETSIG	11	/* for sockets. */
 #endif
 
-#ifndef CONFIG_64BIT
 #ifndef F_GETLK64
 #define F_GETLK64	12	/*  using 'struct flock64' */
 #define F_SETLK64	13
 #define F_SETLKW64	14
 #endif
-#endif
 
 #ifndef F_SETOWN_EX
 #define F_SETOWN_EX	15
-- 
2.30.2


^ permalink raw reply related

* [PATCH 1/5] uapi: remove the unused HAVE_ARCH_STRUCT_FLOCK64 define
From: Christoph Hellwig @ 2022-01-11  8:35 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: linux-arch, linux-s390, linux-parisc, x86, linux-kernel,
	linux-mips, Guo Ren, sparclinux, linuxppc-dev, linux-arm-kernel
In-Reply-To: <20220111083515.502308-1-hch@lst.de>

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 include/uapi/asm-generic/fcntl.h       | 2 --
 tools/include/uapi/asm-generic/fcntl.h | 2 --
 2 files changed, 4 deletions(-)

diff --git a/include/uapi/asm-generic/fcntl.h b/include/uapi/asm-generic/fcntl.h
index ecd0f5bdfc1d6..caa482e3b01af 100644
--- a/include/uapi/asm-generic/fcntl.h
+++ b/include/uapi/asm-generic/fcntl.h
@@ -207,7 +207,6 @@ struct flock {
 };
 #endif
 
-#ifndef HAVE_ARCH_STRUCT_FLOCK64
 #ifndef __ARCH_FLOCK64_PAD
 #define __ARCH_FLOCK64_PAD
 #endif
@@ -220,6 +219,5 @@ struct flock64 {
 	__kernel_pid_t  l_pid;
 	__ARCH_FLOCK64_PAD
 };
-#endif
 
 #endif /* _ASM_GENERIC_FCNTL_H */
diff --git a/tools/include/uapi/asm-generic/fcntl.h b/tools/include/uapi/asm-generic/fcntl.h
index ac190958c9814..4a49d33ca4d55 100644
--- a/tools/include/uapi/asm-generic/fcntl.h
+++ b/tools/include/uapi/asm-generic/fcntl.h
@@ -202,7 +202,6 @@ struct flock {
 };
 #endif
 
-#ifndef HAVE_ARCH_STRUCT_FLOCK64
 #ifndef __ARCH_FLOCK64_PAD
 #define __ARCH_FLOCK64_PAD
 #endif
@@ -215,6 +214,5 @@ struct flock64 {
 	__kernel_pid_t  l_pid;
 	__ARCH_FLOCK64_PAD
 };
-#endif
 
 #endif /* _ASM_GENERIC_FCNTL_H */
-- 
2.30.2


^ permalink raw reply related

* [PATCH 3/5] uapi: merge the 32-bit mips struct flock into the generic one
From: Christoph Hellwig @ 2022-01-11  8:35 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: linux-arch, linux-s390, linux-parisc, x86, linux-kernel,
	linux-mips, Guo Ren, sparclinux, linuxppc-dev, linux-arm-kernel
In-Reply-To: <20220111083515.502308-1-hch@lst.de>

Add a new __ARCH_FLOCK_EXTRA_SYSID macro following the style of
__ARCH_FLOCK_PAD to avoid having a separate definition just for one
architecture.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 arch/mips/include/uapi/asm/fcntl.h     | 26 +++-----------------------
 include/uapi/asm-generic/fcntl.h       |  5 +++--
 tools/include/uapi/asm-generic/fcntl.h |  5 +++--
 3 files changed, 9 insertions(+), 27 deletions(-)

diff --git a/arch/mips/include/uapi/asm/fcntl.h b/arch/mips/include/uapi/asm/fcntl.h
index 42e13dead5431..9e44ac810db94 100644
--- a/arch/mips/include/uapi/asm/fcntl.h
+++ b/arch/mips/include/uapi/asm/fcntl.h
@@ -50,30 +50,10 @@
 #define F_SETLKW64	35
 #endif
 
-/*
- * The flavours of struct flock.  "struct flock" is the ABI compliant
- * variant.  Finally struct flock64 is the LFS variant of struct flock.	 As
- * a historic accident and inconsistence with the ABI definition it doesn't
- * contain all the same fields as struct flock.
- */
-
 #if _MIPS_SIM != _MIPS_SIM_ABI64
-
-#include <linux/types.h>
-
-struct flock {
-	short	l_type;
-	short	l_whence;
-	__kernel_off_t	l_start;
-	__kernel_off_t	l_len;
-	long	l_sysid;
-	__kernel_pid_t l_pid;
-	long	pad[4];
-};
-
-#define HAVE_ARCH_STRUCT_FLOCK
-
-#endif /* _MIPS_SIM == _MIPS_SIM_ABI32 */
+#define __ARCH_FLOCK_EXTRA_SYSID	long l_sysid;
+#define __ARCH_FLOCK_PAD		long pad[4];
+#endif
 
 #include <asm-generic/fcntl.h>
 
diff --git a/include/uapi/asm-generic/fcntl.h b/include/uapi/asm-generic/fcntl.h
index c53897ca5d402..98f4ff165b776 100644
--- a/include/uapi/asm-generic/fcntl.h
+++ b/include/uapi/asm-generic/fcntl.h
@@ -192,18 +192,19 @@ struct f_owner_ex {
 
 #define F_LINUX_SPECIFIC_BASE	1024
 
-#ifndef HAVE_ARCH_STRUCT_FLOCK
 struct flock {
 	short	l_type;
 	short	l_whence;
 	__kernel_off_t	l_start;
 	__kernel_off_t	l_len;
+#ifdef __ARCH_FLOCK_EXTRA_SYSID
+	__ARCH_FLOCK_EXTRA_SYSID
+#endif
 	__kernel_pid_t	l_pid;
 #ifdef __ARCH_FLOCK_PAD
 	__ARCH_FLOCK_PAD
 #endif
 };
-#endif
 
 struct flock64 {
 	short  l_type;
diff --git a/tools/include/uapi/asm-generic/fcntl.h b/tools/include/uapi/asm-generic/fcntl.h
index 82054502b9748..bf961a71802e0 100644
--- a/tools/include/uapi/asm-generic/fcntl.h
+++ b/tools/include/uapi/asm-generic/fcntl.h
@@ -187,18 +187,19 @@ struct f_owner_ex {
 
 #define F_LINUX_SPECIFIC_BASE	1024
 
-#ifndef HAVE_ARCH_STRUCT_FLOCK
 struct flock {
 	short	l_type;
 	short	l_whence;
 	__kernel_off_t	l_start;
 	__kernel_off_t	l_len;
+#ifdef __ARCH_FLOCK_EXTRA_SYSID
+	__ARCH_FLOCK_EXTRA_SYSID
+#endif
 	__kernel_pid_t	l_pid;
 #ifdef __ARCH_FLOCK_PAD
 	__ARCH_FLOCK_PAD
 #endif
 };
-#endif
 
 struct flock64 {
 	short  l_type;
-- 
2.30.2


^ permalink raw reply related

* consolidate the compat fcntl definitions
From: Christoph Hellwig @ 2022-01-11  8:35 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: linux-arch, linux-s390, linux-parisc, x86, linux-kernel,
	linux-mips, Guo Ren, sparclinux, linuxppc-dev, linux-arm-kernel

Hi all,

currenty the compat fcnt definitions are duplicate for all compat
architectures, and the native fcntl64 definitions aren't even usable
from userspace due to a bogus CONFIG_64BIT ifdef.  This series tries
to sort out all that.

Diffstat:
 arch/arm64/include/asm/compat.h        |   20 --------------------
 arch/mips/include/asm/compat.h         |   23 ++---------------------
 arch/mips/include/uapi/asm/fcntl.h     |   28 +++-------------------------
 arch/parisc/include/asm/compat.h       |   16 ----------------
 arch/powerpc/include/asm/compat.h      |   20 --------------------
 arch/s390/include/asm/compat.h         |   20 --------------------
 arch/sparc/include/asm/compat.h        |   22 +---------------------
 arch/x86/include/asm/compat.h          |   24 +++---------------------
 include/linux/compat.h                 |   31 +++++++++++++++++++++++++++++++
 include/uapi/asm-generic/fcntl.h       |   21 +++++++--------------
 tools/include/uapi/asm-generic/fcntl.h |   21 +++++++--------------
 11 files changed, 54 insertions(+), 192 deletions(-)

^ permalink raw reply

* [PATCH 2/5] uapi: simplify __ARCH_FLOCK{,64}_PAD a little
From: Christoph Hellwig @ 2022-01-11  8:35 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: linux-arch, linux-s390, linux-parisc, x86, linux-kernel,
	linux-mips, Guo Ren, sparclinux, linuxppc-dev, linux-arm-kernel
In-Reply-To: <20220111083515.502308-1-hch@lst.de>

Don't bother to define the symbols empty, just don't use them.  That
makes the intent a little more clear.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 include/uapi/asm-generic/fcntl.h       | 12 ++++--------
 tools/include/uapi/asm-generic/fcntl.h | 12 ++++--------
 2 files changed, 8 insertions(+), 16 deletions(-)

diff --git a/include/uapi/asm-generic/fcntl.h b/include/uapi/asm-generic/fcntl.h
index caa482e3b01af..c53897ca5d402 100644
--- a/include/uapi/asm-generic/fcntl.h
+++ b/include/uapi/asm-generic/fcntl.h
@@ -193,22 +193,16 @@ struct f_owner_ex {
 #define F_LINUX_SPECIFIC_BASE	1024
 
 #ifndef HAVE_ARCH_STRUCT_FLOCK
-#ifndef __ARCH_FLOCK_PAD
-#define __ARCH_FLOCK_PAD
-#endif
-
 struct flock {
 	short	l_type;
 	short	l_whence;
 	__kernel_off_t	l_start;
 	__kernel_off_t	l_len;
 	__kernel_pid_t	l_pid;
+#ifdef __ARCH_FLOCK_PAD
 	__ARCH_FLOCK_PAD
-};
 #endif
-
-#ifndef __ARCH_FLOCK64_PAD
-#define __ARCH_FLOCK64_PAD
+};
 #endif
 
 struct flock64 {
@@ -217,7 +211,9 @@ struct flock64 {
 	__kernel_loff_t l_start;
 	__kernel_loff_t l_len;
 	__kernel_pid_t  l_pid;
+#ifdef __ARCH_FLOCK64_PAD
 	__ARCH_FLOCK64_PAD
+#endif
 };
 
 #endif /* _ASM_GENERIC_FCNTL_H */
diff --git a/tools/include/uapi/asm-generic/fcntl.h b/tools/include/uapi/asm-generic/fcntl.h
index 4a49d33ca4d55..82054502b9748 100644
--- a/tools/include/uapi/asm-generic/fcntl.h
+++ b/tools/include/uapi/asm-generic/fcntl.h
@@ -188,22 +188,16 @@ struct f_owner_ex {
 #define F_LINUX_SPECIFIC_BASE	1024
 
 #ifndef HAVE_ARCH_STRUCT_FLOCK
-#ifndef __ARCH_FLOCK_PAD
-#define __ARCH_FLOCK_PAD
-#endif
-
 struct flock {
 	short	l_type;
 	short	l_whence;
 	__kernel_off_t	l_start;
 	__kernel_off_t	l_len;
 	__kernel_pid_t	l_pid;
+#ifdef __ARCH_FLOCK_PAD
 	__ARCH_FLOCK_PAD
-};
 #endif
-
-#ifndef __ARCH_FLOCK64_PAD
-#define __ARCH_FLOCK64_PAD
+};
 #endif
 
 struct flock64 {
@@ -212,7 +206,9 @@ struct flock64 {
 	__kernel_loff_t l_start;
 	__kernel_loff_t l_len;
 	__kernel_pid_t  l_pid;
+#ifdef __ARCH_FLOCK64_PAD
 	__ARCH_FLOCK64_PAD
+#endif
 };
 
 #endif /* _ASM_GENERIC_FCNTL_H */
-- 
2.30.2


^ permalink raw reply related

* Re: [5.16.0] build error on unrecognized opcode: ptesync
From: Christophe Leroy @ 2022-01-11  6:20 UTC (permalink / raw)
  To: Mike; +Cc: linuxppc-dev@lists.ozlabs.org
In-Reply-To: <CANtoAtPcRGHafb7LddtrY5g43m82txS_mY3WO_ZtJ+pfuLvKaw@mail.gmail.com>



Le 10/01/2022 à 13:32, Mike a écrit :
> Hey, so I originally sat down to compile the fast headers V2 patch, but 
> quickly discovered other things at play, and grabbed 5.16.0 a few hours 
> after it lifted off,  arch/powerpc/mm/mmu_context.c I had to 
> specifically say had to include -maltivec or it barfed on a 'dssall', 
> I'm fine with that, I've spent years in kernel land, I can deal with 
> that, then came arch/powerpc/lib/step.c with the ptesync. This seems 
> like a totally normal instruction that shouldn't need any extra flags or 
> anything, yet the assembler throws up, and no flag I can think of fixes 
> it. This is a G4 7447. I reverted back to the Debian 5.15. defconfig 
> before dropping this mail as I had tweaked my config to be more G4.
> 

Hi Mike,

Can you provide a bit more details about your setup and config ?

Are you using GCC or LLVM ?
What version of GCC and BINUTILS or what version of LLVM ?

What is DEBIAN defconfig ? Does it correspond to one of the standard 
mainline kernel defconfigs ? If not can you provide it ?

Thanks
Christophe

^ permalink raw reply

* Re: [PATCH v2 1/2] powerpc: Fix virt_addr_valid() check
From: Christophe Leroy @ 2022-01-11  6:04 UTC (permalink / raw)
  To: Nicholas Piggin, Andrew Morton, Benjamin Herrenschmidt, Kees Cook,
	Laura Abbott, linux-kernel@vger.kernel.org, linux-mm@kvack.org,
	linuxppc-dev@lists.ozlabs.org, Mark Rutland, Michael Ellerman,
	Paul Mackerras, Kefeng Wang
In-Reply-To: <1641871726.fshx7g5r92.astroid@bobo.none>



Le 11/01/2022 à 05:37, Nicholas Piggin a écrit :
> Excerpts from Kefeng Wang's message of January 8, 2022 9:58 pm:
>> Hi PPC maintainers, ping..
> 
> Hmm. I might have confused myself about this. I'm going back and
> trying to work out what I was thinking when I suggested it. This
> works on 64e because vmalloc space is below the kernel linear map,
> right?
> 
> On 64s it is the other way around and it is still possible to enable
> flatmem on 64s. Altough we might just not hit the problem there because
> __pa() will not mask away the vmalloc offset for 64s so it will still
> return something that's outside the pfn_valid range for flatmem. That's
> very subtle though.

That's the way it works on PPC32 at least, so for me it's not chocking 
to have it work the same way on PPC64s.

The main issue here is the way __pa() works. On PPC32 __pa = va - 
PAGE_OFFSET, so it works correctly for any address.
On PPC64, __pa() works by masking out the 2 top bits instead of 
substracting PAGE_OFFSET, so the test must add a verification that we 
really have the 2 top bits set at first. This is what (addr >= 
PAGE_OFFSET) does. Once this first test is done, we can perfectly rely 
on pfn_valid() just like PPC32, I see absolutely no point in an 
additionnal test checking the addr is below KERN_VIRT_START.


> 
> The checks added to __pa actually don't prevent vmalloc memory from
> being passed to it either on 64s, only a more basic test.

That's correct. It is the role of pfn_valid() to check that.

Christophe

> 
> I think 64s wants (addr >= PAGE_OFFSET && addr < KERN_VIRT_START) as
> the condition.  Could possibly add that check to __pa as well to
> catch vmalloc addresses.
> 
> Thanks,
> Nick
> 
>>
>> On 2021/12/25 20:06, Kefeng Wang wrote:
>>> When run ethtool eth0, the BUG occurred,
>>>
>>>     usercopy: Kernel memory exposure attempt detected from SLUB object not in SLUB page?! (offset 0, size 1048)!
>>>     kernel BUG at mm/usercopy.c:99
>>>     ...
>>>     usercopy_abort+0x64/0xa0 (unreliable)
>>>     __check_heap_object+0x168/0x190
>>>     __check_object_size+0x1a0/0x200
>>>     dev_ethtool+0x2494/0x2b20
>>>     dev_ioctl+0x5d0/0x770
>>>     sock_do_ioctl+0xf0/0x1d0
>>>     sock_ioctl+0x3ec/0x5a0
>>>     __se_sys_ioctl+0xf0/0x160
>>>     system_call_exception+0xfc/0x1f0
>>>     system_call_common+0xf8/0x200
>>>
>>> The code shows below,
>>>
>>>     data = vzalloc(array_size(gstrings.len, ETH_GSTRING_LEN));
>>>     copy_to_user(useraddr, data, gstrings.len * ETH_GSTRING_LEN))
>>>
>>> The data is alloced by vmalloc(), virt_addr_valid(ptr) will return true
>>> on PowerPC64, which leads to the panic.
>>>
>>> As commit 4dd7554a6456 ("powerpc/64: Add VIRTUAL_BUG_ON checks for __va
>>> and __pa addresses") does, make sure the virt addr above PAGE_OFFSET in
>>> the virt_addr_valid().
>>>
>>> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
>>> ---
>>>    arch/powerpc/include/asm/page.h | 5 ++++-
>>>    1 file changed, 4 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/arch/powerpc/include/asm/page.h b/arch/powerpc/include/asm/page.h
>>> index 254687258f42..300d4c105a3a 100644
>>> --- a/arch/powerpc/include/asm/page.h
>>> +++ b/arch/powerpc/include/asm/page.h
>>> @@ -132,7 +132,10 @@ static inline bool pfn_valid(unsigned long pfn)
>>>    #define virt_to_page(kaddr)	pfn_to_page(virt_to_pfn(kaddr))
>>>    #define pfn_to_kaddr(pfn)	__va((pfn) << PAGE_SHIFT)
>>>    
>>> -#define virt_addr_valid(kaddr)	pfn_valid(virt_to_pfn(kaddr))
>>> +#define virt_addr_valid(vaddr)	({						\
>>> +	unsigned long _addr = (unsigned long)vaddr;				\
>>> +	(unsigned long)(_addr) >= PAGE_OFFSET && pfn_valid(virt_to_pfn(_addr));	\
>>> +})
>>>    
>>>    /*
>>>     * On Book-E parts we need __va to parse the device tree and we can't
>>

^ permalink raw reply

* Re: [PATCH v2 1/2] powerpc: Fix virt_addr_valid() check
From: Nicholas Piggin @ 2022-01-11  4:37 UTC (permalink / raw)
  To: Andrew Morton, Benjamin Herrenschmidt, Kees Cook, Laura Abbott,
	linux-kernel, linux-mm, linuxppc-dev, Mark Rutland,
	Michael Ellerman, Paul Mackerras, Kefeng Wang
In-Reply-To: <09ed46a5-6df3-ffc0-8243-61612c06153a@huawei.com>

Excerpts from Kefeng Wang's message of January 8, 2022 9:58 pm:
> Hi PPC maintainers, ping..

Hmm. I might have confused myself about this. I'm going back and
trying to work out what I was thinking when I suggested it. This
works on 64e because vmalloc space is below the kernel linear map,
right?

On 64s it is the other way around and it is still possible to enable 
flatmem on 64s. Altough we might just not hit the problem there because 
__pa() will not mask away the vmalloc offset for 64s so it will still 
return something that's outside the pfn_valid range for flatmem. That's 
very subtle though.

The checks added to __pa actually don't prevent vmalloc memory from
being passed to it either on 64s, only a more basic test.

I think 64s wants (addr >= PAGE_OFFSET && addr < KERN_VIRT_START) as
the condition.  Could possibly add that check to __pa as well to
catch vmalloc addresses.

Thanks,
Nick

> 
> On 2021/12/25 20:06, Kefeng Wang wrote:
>> When run ethtool eth0, the BUG occurred,
>>
>>    usercopy: Kernel memory exposure attempt detected from SLUB object not in SLUB page?! (offset 0, size 1048)!
>>    kernel BUG at mm/usercopy.c:99
>>    ...
>>    usercopy_abort+0x64/0xa0 (unreliable)
>>    __check_heap_object+0x168/0x190
>>    __check_object_size+0x1a0/0x200
>>    dev_ethtool+0x2494/0x2b20
>>    dev_ioctl+0x5d0/0x770
>>    sock_do_ioctl+0xf0/0x1d0
>>    sock_ioctl+0x3ec/0x5a0
>>    __se_sys_ioctl+0xf0/0x160
>>    system_call_exception+0xfc/0x1f0
>>    system_call_common+0xf8/0x200
>>
>> The code shows below,
>>
>>    data = vzalloc(array_size(gstrings.len, ETH_GSTRING_LEN));
>>    copy_to_user(useraddr, data, gstrings.len * ETH_GSTRING_LEN))
>>
>> The data is alloced by vmalloc(), virt_addr_valid(ptr) will return true
>> on PowerPC64, which leads to the panic.
>>
>> As commit 4dd7554a6456 ("powerpc/64: Add VIRTUAL_BUG_ON checks for __va
>> and __pa addresses") does, make sure the virt addr above PAGE_OFFSET in
>> the virt_addr_valid().
>>
>> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
>> ---
>>   arch/powerpc/include/asm/page.h | 5 ++++-
>>   1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/powerpc/include/asm/page.h b/arch/powerpc/include/asm/page.h
>> index 254687258f42..300d4c105a3a 100644
>> --- a/arch/powerpc/include/asm/page.h
>> +++ b/arch/powerpc/include/asm/page.h
>> @@ -132,7 +132,10 @@ static inline bool pfn_valid(unsigned long pfn)
>>   #define virt_to_page(kaddr)	pfn_to_page(virt_to_pfn(kaddr))
>>   #define pfn_to_kaddr(pfn)	__va((pfn) << PAGE_SHIFT)
>>   
>> -#define virt_addr_valid(kaddr)	pfn_valid(virt_to_pfn(kaddr))
>> +#define virt_addr_valid(vaddr)	({						\
>> +	unsigned long _addr = (unsigned long)vaddr;				\
>> +	(unsigned long)(_addr) >= PAGE_OFFSET && pfn_valid(virt_to_pfn(_addr));	\
>> +})
>>   
>>   /*
>>    * On Book-E parts we need __va to parse the device tree and we can't
> 

^ permalink raw reply

* Re: [PATCH v3 5/6] KVM: PPC: mmio: Return to guest after emulation failure
From: Nicholas Piggin @ 2022-01-11  3:23 UTC (permalink / raw)
  To: Alexey Kardashevskiy, Fabiano Rosas, kvm-ppc; +Cc: linuxppc-dev
In-Reply-To: <bf61f021-15b3-7093-f991-cdcda93d059d@ozlabs.ru>

Excerpts from Alexey Kardashevskiy's message of January 11, 2022 9:51 am:
> 
> 
> On 1/10/22 18:36, Nicholas Piggin wrote:
>> Excerpts from Fabiano Rosas's message of January 8, 2022 7:00 am:
>>> If MMIO emulation fails we don't want to crash the whole guest by
>>> returning to userspace.
>>>
>>> The original commit bbf45ba57eae ("KVM: ppc: PowerPC 440 KVM
>>> implementation") added a todo:
>>>
>>>    /* XXX Deliver Program interrupt to guest. */
>>>
>>> and later the commit d69614a295ae ("KVM: PPC: Separate loadstore
>>> emulation from priv emulation") added the Program interrupt injection
>>> but in another file, so I'm assuming it was missed that this block
>>> needed to be altered.
>>>
>>> Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com>
>>> Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>>> ---
>>>   arch/powerpc/kvm/powerpc.c | 2 +-
>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
>>> index 6daeea4a7de1..56b0faab7a5f 100644
>>> --- a/arch/powerpc/kvm/powerpc.c
>>> +++ b/arch/powerpc/kvm/powerpc.c
>>> @@ -309,7 +309,7 @@ int kvmppc_emulate_mmio(struct kvm_vcpu *vcpu)
>>>   		kvmppc_get_last_inst(vcpu, INST_GENERIC, &last_inst);
>>>   		kvmppc_core_queue_program(vcpu, 0);
>>>   		pr_info("%s: emulation failed (%08x)\n", __func__, last_inst);
>>> -		r = RESUME_HOST;
>>> +		r = RESUME_GUEST;
>> 
>> So at this point can the pr_info just go away?
>> 
>> I wonder if this shouldn't be a DSI rather than a program check.
>> DSI with DSISR[37] looks a bit more expected. Not that Linux
>> probably does much with it but at least it would give a SIGBUS
>> rather than SIGILL.
> 
> It does not like it is more expected to me, it is not about wrong memory 
> attributes, it is the instruction itself which cannot execute.

It's not an illegal instruction though, it can't execute because of the
nature of the data / address it is operating on. That says d-side to me.

DSISR[37] isn't perfect but if you squint it's not terrible. It's about
certain instructions that have restrictions operating on other than
normal cacheable mappings.

Thanks,
Nick


> 
> DSISR[37]:
> Set to 1 if the access is due to a lq, stq, lwat, ldat, lbarx, lharx, 
> lwarx, ldarx, lqarx, stwat,
> stdat, stbcx., sthcx., stwcx., stdcx., or stqcx. instruction that 
> addresses storage that is Write
> Through Required or Caching Inhibited; or if the access is due to a copy 
> or paste. instruction
> that addresses storage that is Caching Inhibited; or if the access is 
> due to a lwat, ldat, stwat, or
> stdat instruction that addresses storage that is Guarded; otherwise set 
> to 0.
> 

^ permalink raw reply

* Re: [PATCH] powerpc/time: Fix build failure due to do_hard_irq_enable() on PPC32
From: Nicholas Piggin @ 2022-01-11  3:11 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Christophe Leroy, Michael Ellerman,
	Paul Mackerras
  Cc: linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org
In-Reply-To: <247e01e0e10f4dbc59b5ff89e81702eb1ee7641e.1641828571.git.christophe.leroy@csgroup.eu>

Excerpts from Christophe Leroy's message of January 11, 2022 1:29 am:
> 	  CC      arch/powerpc/kernel/time.o
> 	In file included from <command-line>:
> 	./arch/powerpc/include/asm/hw_irq.h: In function 'do_hard_irq_enable':
> 	././include/linux/compiler_types.h:335:45: error: call to '__compiletime_assert_35' declared with attribute error: BUILD_BUG failed
> 	  335 |         _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
> 	      |                                             ^
> 	././include/linux/compiler_types.h:316:25: note: in definition of macro '__compiletime_assert'
> 	  316 |                         prefix ## suffix();                             \
> 	      |                         ^~~~~~
> 	././include/linux/compiler_types.h:335:9: note: in expansion of macro '_compiletime_assert'
> 	  335 |         _compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
> 	      |         ^~~~~~~~~~~~~~~~~~~
> 	./include/linux/build_bug.h:39:37: note: in expansion of macro 'compiletime_assert'
> 	   39 | #define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
> 	      |                                     ^~~~~~~~~~~~~~~~~~
> 	./include/linux/build_bug.h:59:21: note: in expansion of macro 'BUILD_BUG_ON_MSG'
> 	   59 | #define BUILD_BUG() BUILD_BUG_ON_MSG(1, "BUILD_BUG failed")
> 	      |                     ^~~~~~~~~~~~~~~~
> 	./arch/powerpc/include/asm/hw_irq.h:483:9: note: in expansion of macro 'BUILD_BUG'
> 	  483 |         BUILD_BUG();
> 	      |         ^~~~~~~~~
> 
> should_hard_irq_enable() returns false on PPC32 so this BUILD_BUG() shouldn't trigger.
> 
> Force inlining of should_hard_irq_enable()
> 
> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
> Fixes: 0faf20a1ad16 ("powerpc/64s/interrupt: Don't enable MSR[EE] in irq handlers unless perf is in use")
> Cc: Nicholas Piggin <npiggin@gmail.com>

Acked-by: Nicholas Piggin <npiggin@gmail.com>

Thanks,
Nick

> ---
>  arch/powerpc/include/asm/hw_irq.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/powerpc/include/asm/hw_irq.h b/arch/powerpc/include/asm/hw_irq.h
> index a58fb4aa6c81..674e5aaafcbd 100644
> --- a/arch/powerpc/include/asm/hw_irq.h
> +++ b/arch/powerpc/include/asm/hw_irq.h
> @@ -473,7 +473,7 @@ static inline bool arch_irq_disabled_regs(struct pt_regs *regs)
>  	return !(regs->msr & MSR_EE);
>  }
>  
> -static inline bool should_hard_irq_enable(void)
> +static __always_inline bool should_hard_irq_enable(void)
>  {
>  	return false;
>  }
> -- 
> 2.33.1
> 

^ permalink raw reply

* Re: [PATCH 3/6] KVM: Remove opaque from kvm_arch_check_processor_compat
From: Chao Gao @ 2022-01-11  3:19 UTC (permalink / raw)
  To: Sean Christopherson
  Cc: x86, Wanpeng Li, kvm, David Hildenbrand, Dave Hansen, linux-mips,
	Atish Patra, Paul Mackerras, H. Peter Anvin, Alexander Gordeev,
	Claudio Imbrenda, Will Deacon, Maciej S. Szmigiero, linux-s390,
	Janosch Frank, Marc Zyngier, Joerg Roedel, Huacai Chen,
	linux-riscv, kvmarm, Aleksandar Markovic, Ingo Molnar,
	Catalin Marinas, Palmer Dabbelt, Christian Borntraeger,
	Ravi Bangoria, kevin.tian, Albert Ou, Vasily Gorbik,
	Suzuki K Poulose, Heiko Carstens, Nicholas Piggin,
	Borislav Petkov, Cédric Le Goater, Paul Walmsley, tglx,
	Alexandru Elisei, linux-arm-kernel, Jim Mattson,
	Thomas Bogendoerfer, Fabiano Rosas, Anup Patel, linux-kernel,
	Bharata B Rao, James Morse, kvm-riscv, pbonzini, Vitaly Kuznetsov,
	linuxppc-dev
In-Reply-To: <Ydy8BCfE0jhJd5uE@google.com>

On Mon, Jan 10, 2022 at 11:06:44PM +0000, Sean Christopherson wrote:
>On Mon, Dec 27, 2021, Chao Gao wrote:
>> No arch implementation uses this opaque now.
>
>Except for the RISC-V part, this can be a pure revert of commit b99040853738 ("KVM:
>Pass kvm_init()'s opaque param to additional arch funcs").  I think it makes sense
>to process it as a revert, with a short blurb in the changelog to note that RISC-V
>is manually modified as RISC-V support came along in the interim.

commit b99040853738 adds opaque param to kvm_arch_hardware_setup(), which isn't
reverted in this patch. I.e., this patch is a partial revert of b99040853738
plus manual changes to RISC-V. Given that, "process it as a revert" means
clearly say in changelog that this commit contains a partial revert of commit
b99040853738 ("KVM: Pass kvm_init()'s opaque param to additional arch funcs").

Right?

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox