* [PATCH v4 3/6] arm64: Handle trapped DC CVADP
From: Andrew Murray @ 2019-04-03 10:56 UTC (permalink / raw)
To: Catalin Marinas, Will Deacon
Cc: Szabolcs Nagy, dave.martin, linux-arm-kernel, Mark Rutland,
Phil Blundell, libc-alpha, linux-api, Suzuki K Poulose
In-Reply-To: <20190403105628.39798-1-andrew.murray@arm.com>
The ARMv8.5 DC CVADP instruction may be trapped to EL1 via
SCTLR_EL1.UCI therefore let's provide a handler for it.
Just like the CVAP instruction we use a 'sys' instruction instead of
the 'dc' alias to avoid build issues with older toolchains.
Signed-off-by: Andrew Murray <andrew.murray@arm.com>
Reviewed-by: Mark Rutland <mark.rutland@arm.com>
---
arch/arm64/include/asm/esr.h | 3 ++-
arch/arm64/kernel/traps.c | 3 +++
2 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/include/asm/esr.h b/arch/arm64/include/asm/esr.h
index 52233f00d53d..07d5c026a0b3 100644
--- a/arch/arm64/include/asm/esr.h
+++ b/arch/arm64/include/asm/esr.h
@@ -198,9 +198,10 @@
/*
* User space cache operations have the following sysreg encoding
* in System instructions.
- * op0=1, op1=3, op2=1, crn=7, crm={ 5, 10, 11, 12, 14 }, WRITE (L=0)
+ * op0=1, op1=3, op2=1, crn=7, crm={ 5, 10, 11, 12, 13, 14 }, WRITE (L=0)
*/
#define ESR_ELx_SYS64_ISS_CRM_DC_CIVAC 14
+#define ESR_ELx_SYS64_ISS_CRM_DC_CVADP 13
#define ESR_ELx_SYS64_ISS_CRM_DC_CVAP 12
#define ESR_ELx_SYS64_ISS_CRM_DC_CVAU 11
#define ESR_ELx_SYS64_ISS_CRM_DC_CVAC 10
diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c
index 8ad119c3f665..f66e1ddbe4a7 100644
--- a/arch/arm64/kernel/traps.c
+++ b/arch/arm64/kernel/traps.c
@@ -459,6 +459,9 @@ static void user_cache_maint_handler(unsigned int esr, struct pt_regs *regs)
case ESR_ELx_SYS64_ISS_CRM_DC_CVAC: /* DC CVAC, gets promoted */
__user_cache_maint("dc civac", address, ret);
break;
+ case ESR_ELx_SYS64_ISS_CRM_DC_CVADP: /* DC CVADP */
+ __user_cache_maint("sys 3, c7, c13, 1", address, ret);
+ break;
case ESR_ELx_SYS64_ISS_CRM_DC_CVAP: /* DC CVAP */
__user_cache_maint("sys 3, c7, c12, 1", address, ret);
break;
--
2.21.0
^ permalink raw reply related
* [PATCH v4 2/6] arm64: HWCAP: encapsulate elf_hwcap
From: Andrew Murray @ 2019-04-03 10:56 UTC (permalink / raw)
To: Catalin Marinas, Will Deacon
Cc: Szabolcs Nagy, dave.martin, linux-arm-kernel, Mark Rutland,
Phil Blundell, libc-alpha, linux-api, Suzuki K Poulose
In-Reply-To: <20190403105628.39798-1-andrew.murray@arm.com>
The introduction of AT_HWCAP2 introduced accessors which ensure that
hwcap features are set and tested appropriately.
Let's now mandate access to elf_hwcap via these accessors by making
elf_hwcap static within cpufeature.c.
Signed-off-by: Andrew Murray <andrew.murray@arm.com>
---
arch/arm64/include/asm/cpufeature.h | 15 ++++---------
arch/arm64/include/asm/hwcap.h | 7 +++---
arch/arm64/kernel/cpufeature.c | 33 +++++++++++++++++++++++++++--
3 files changed, 38 insertions(+), 17 deletions(-)
diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h
index 347c17046668..a3f028f82def 100644
--- a/arch/arm64/include/asm/cpufeature.h
+++ b/arch/arm64/include/asm/cpufeature.h
@@ -392,19 +392,12 @@ extern DECLARE_BITMAP(boot_capabilities, ARM64_NPATCHABLE);
for_each_set_bit(cap, cpu_hwcaps, ARM64_NCAPS)
bool this_cpu_has_cap(unsigned int cap);
+void cpu_set_feature(unsigned int num);
+bool cpu_have_feature(unsigned int num);
+unsigned long cpu_get_elf_hwcap(void);
+unsigned long cpu_get_elf_hwcap2(void);
-static inline void cpu_set_feature(unsigned int num)
-{
- WARN_ON(num >= MAX_CPU_FEATURES);
- elf_hwcap |= BIT(num);
-}
#define cpu_set_named_feature(name) cpu_set_feature(cpu_feature(name))
-
-static inline bool cpu_have_feature(unsigned int num)
-{
- WARN_ON(num >= MAX_CPU_FEATURES);
- return elf_hwcap & BIT(num);
-}
#define cpu_have_named_feature(name) cpu_have_feature(cpu_feature(name))
/* System capability check for constant caps */
diff --git a/arch/arm64/include/asm/hwcap.h b/arch/arm64/include/asm/hwcap.h
index 1f38a2740f7a..509ee0d2fa0f 100644
--- a/arch/arm64/include/asm/hwcap.h
+++ b/arch/arm64/include/asm/hwcap.h
@@ -17,6 +17,7 @@
#define __ASM_HWCAP_H
#include <uapi/asm/hwcap.h>
+#include <asm/cpufeature.h>
#define COMPAT_HWCAP_HALF (1 << 1)
#define COMPAT_HWCAP_THUMB (1 << 2)
@@ -86,14 +87,13 @@
#define __khwcap2_feature(x) (ilog2(HWCAP2_ ## x) + 32)
#ifndef __ASSEMBLY__
-#include <linux/kernel.h>
/*
* This yields a mask that user programs can use to figure out what
* instruction set this cpu supports.
*/
-#define ELF_HWCAP lower_32_bits(elf_hwcap)
-#define ELF_HWCAP2 upper_32_bits(elf_hwcap)
+#define ELF_HWCAP cpu_get_elf_hwcap()
+#define ELF_HWCAP2 cpu_get_elf_hwcap2()
#ifdef CONFIG_COMPAT
#define COMPAT_ELF_HWCAP (compat_elf_hwcap)
@@ -109,6 +109,5 @@ enum {
#endif
};
-extern unsigned long elf_hwcap;
#endif
#endif
diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index 986ceeacd19f..a655d1bb1186 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -35,8 +35,8 @@
#include <asm/traps.h>
#include <asm/virt.h>
-unsigned long elf_hwcap __read_mostly;
-EXPORT_SYMBOL_GPL(elf_hwcap);
+/* Kernel representation of AT_HWCAP and AT_HWCAP2 */
+static unsigned long elf_hwcap __read_mostly;
#ifdef CONFIG_COMPAT
#define COMPAT_ELF_HWCAP_DEFAULT \
@@ -1947,6 +1947,35 @@ bool this_cpu_has_cap(unsigned int n)
return false;
}
+void cpu_set_feature(unsigned int num)
+{
+ WARN_ON(num >= MAX_CPU_FEATURES);
+ elf_hwcap |= BIT(num);
+}
+EXPORT_SYMBOL_GPL(cpu_set_feature);
+
+bool cpu_have_feature(unsigned int num)
+{
+ WARN_ON(num >= MAX_CPU_FEATURES);
+ return elf_hwcap & BIT(num);
+}
+EXPORT_SYMBOL_GPL(cpu_have_feature);
+
+unsigned long cpu_get_elf_hwcap(void)
+{
+ /*
+ * We currently only populate the first 32 bits of AT_HWCAP. Please
+ * note that for userspace compatibility we guarantee that bits 62
+ * and 63 will always be returned as 0.
+ */
+ return lower_32_bits(elf_hwcap);
+}
+
+unsigned long cpu_get_elf_hwcap2(void)
+{
+ return upper_32_bits(elf_hwcap);
+}
+
static void __init setup_system_capabilities(void)
{
/*
--
2.21.0
^ permalink raw reply related
* [PATCH v4 1/6] arm64: HWCAP: add support for AT_HWCAP2
From: Andrew Murray @ 2019-04-03 10:56 UTC (permalink / raw)
To: Catalin Marinas, Will Deacon
Cc: Szabolcs Nagy, dave.martin, linux-arm-kernel, Mark Rutland,
Phil Blundell, libc-alpha, linux-api, Suzuki K Poulose
In-Reply-To: <20190403105628.39798-1-andrew.murray@arm.com>
As we will exhaust the first 32 bits of AT_HWCAP let's start
exposing AT_HWCAP2 to userspace to give us up to 64 caps.
Whilst it's possible to use the remaining 32 bits of AT_HWCAP, we
prefer to expand into AT_HWCAP2 in order to provide a consistent
view to userspace between ILP32 and LP64. However internal to the
kernel we prefer to continue to use the full space of elf_hwcap.
To reduce complexity and allow for future expansion, we now
represent hwcaps in the kernel as ordinals and use a
KERNEL_HWCAP_ prefix. This allows us to support automatic feature
based module loading for all our hwcaps.
We introduce cpu_set_feature to set hwcaps which complements the
existing cpu_have_feature helper. These helpers allow us to clean
up existing direct uses of elf_hwcap and reduce any future effort
required to move beyond 64 caps.
For convenience we also introduce cpu_{have,set}_named_feature which
makes use of the cpu_feature macro to allow providing a hwcap name
without a {KERNEL_}HWCAP_ prefix.
Signed-off-by: Andrew Murray <andrew.murray@arm.com>
---
Documentation/arm64/elf_hwcaps.txt | 14 +++--
arch/arm64/crypto/aes-ce-ccm-glue.c | 2 +-
arch/arm64/crypto/aes-neonbs-glue.c | 2 +-
arch/arm64/crypto/chacha-neon-glue.c | 2 +-
arch/arm64/crypto/crct10dif-ce-glue.c | 4 +-
arch/arm64/crypto/ghash-ce-glue.c | 8 +--
arch/arm64/crypto/nhpoly1305-neon-glue.c | 2 +-
arch/arm64/crypto/sha256-glue.c | 4 +-
arch/arm64/include/asm/cpufeature.h | 22 ++++----
arch/arm64/include/asm/hwcap.h | 51 +++++++++++++++++-
arch/arm64/include/uapi/asm/hwcap.h | 2 +-
arch/arm64/kernel/cpufeature.c | 66 ++++++++++++------------
arch/arm64/kernel/cpuinfo.c | 2 +-
arch/arm64/kernel/fpsimd.c | 4 +-
drivers/clocksource/arm_arch_timer.c | 8 +++
15 files changed, 130 insertions(+), 63 deletions(-)
diff --git a/Documentation/arm64/elf_hwcaps.txt b/Documentation/arm64/elf_hwcaps.txt
index 13d6691b37be..c04f8e87bab8 100644
--- a/Documentation/arm64/elf_hwcaps.txt
+++ b/Documentation/arm64/elf_hwcaps.txt
@@ -13,9 +13,9 @@ architected discovery mechanism available to userspace code at EL0. The
kernel exposes the presence of these features to userspace through a set
of flags called hwcaps, exposed in the auxilliary vector.
-Userspace software can test for features by acquiring the AT_HWCAP entry
-of the auxilliary vector, and testing whether the relevant flags are
-set, e.g.
+Userspace software can test for features by acquiring the AT_HWCAP or
+AT_HWCAP2 entry of the auxiliary vector, and testing whether the relevant
+flags are set, e.g.
bool floating_point_is_present(void)
{
@@ -194,3 +194,11 @@ HWCAP_PACG
Functionality implied by ID_AA64ISAR1_EL1.GPA == 0b0001 or
ID_AA64ISAR1_EL1.GPI == 0b0001, as described by
Documentation/arm64/pointer-authentication.txt.
+
+
+4. Unused AT_HWCAP bits
+-----------------------
+
+Each AT_HWCAP and AT_HWCAP2 entry provides for up to 32 hwcaps contained
+in bits [31:0]. For interoperation with userspace we guarantee that bits
+62 and 63 of AT_HWCAP will always be returned as 0.
diff --git a/arch/arm64/crypto/aes-ce-ccm-glue.c b/arch/arm64/crypto/aes-ce-ccm-glue.c
index 5fc6f51908fd..036ea77f83bc 100644
--- a/arch/arm64/crypto/aes-ce-ccm-glue.c
+++ b/arch/arm64/crypto/aes-ce-ccm-glue.c
@@ -372,7 +372,7 @@ static struct aead_alg ccm_aes_alg = {
static int __init aes_mod_init(void)
{
- if (!(elf_hwcap & HWCAP_AES))
+ if (!cpu_have_named_feature(AES))
return -ENODEV;
return crypto_register_aead(&ccm_aes_alg);
}
diff --git a/arch/arm64/crypto/aes-neonbs-glue.c b/arch/arm64/crypto/aes-neonbs-glue.c
index e7a95a566462..bf1b321ff4c1 100644
--- a/arch/arm64/crypto/aes-neonbs-glue.c
+++ b/arch/arm64/crypto/aes-neonbs-glue.c
@@ -440,7 +440,7 @@ static int __init aes_init(void)
int err;
int i;
- if (!(elf_hwcap & HWCAP_ASIMD))
+ if (!cpu_have_named_feature(ASIMD))
return -ENODEV;
err = crypto_register_skciphers(aes_algs, ARRAY_SIZE(aes_algs));
diff --git a/arch/arm64/crypto/chacha-neon-glue.c b/arch/arm64/crypto/chacha-neon-glue.c
index bece1d85bd81..cb054f51c917 100644
--- a/arch/arm64/crypto/chacha-neon-glue.c
+++ b/arch/arm64/crypto/chacha-neon-glue.c
@@ -173,7 +173,7 @@ static struct skcipher_alg algs[] = {
static int __init chacha_simd_mod_init(void)
{
- if (!(elf_hwcap & HWCAP_ASIMD))
+ if (!cpu_have_named_feature(ASIMD))
return -ENODEV;
return crypto_register_skciphers(algs, ARRAY_SIZE(algs));
diff --git a/arch/arm64/crypto/crct10dif-ce-glue.c b/arch/arm64/crypto/crct10dif-ce-glue.c
index dd325829ee44..e81d5bd555c0 100644
--- a/arch/arm64/crypto/crct10dif-ce-glue.c
+++ b/arch/arm64/crypto/crct10dif-ce-glue.c
@@ -101,7 +101,7 @@ static struct shash_alg crc_t10dif_alg[] = {{
static int __init crc_t10dif_mod_init(void)
{
- if (elf_hwcap & HWCAP_PMULL)
+ if (cpu_have_named_feature(PMULL))
return crypto_register_shashes(crc_t10dif_alg,
ARRAY_SIZE(crc_t10dif_alg));
else
@@ -111,7 +111,7 @@ static int __init crc_t10dif_mod_init(void)
static void __exit crc_t10dif_mod_exit(void)
{
- if (elf_hwcap & HWCAP_PMULL)
+ if (cpu_have_named_feature(PMULL))
crypto_unregister_shashes(crc_t10dif_alg,
ARRAY_SIZE(crc_t10dif_alg));
else
diff --git a/arch/arm64/crypto/ghash-ce-glue.c b/arch/arm64/crypto/ghash-ce-glue.c
index 791ad422c427..4e69bb78ea89 100644
--- a/arch/arm64/crypto/ghash-ce-glue.c
+++ b/arch/arm64/crypto/ghash-ce-glue.c
@@ -704,10 +704,10 @@ static int __init ghash_ce_mod_init(void)
{
int ret;
- if (!(elf_hwcap & HWCAP_ASIMD))
+ if (!cpu_have_named_feature(ASIMD))
return -ENODEV;
- if (elf_hwcap & HWCAP_PMULL)
+ if (cpu_have_named_feature(PMULL))
ret = crypto_register_shashes(ghash_alg,
ARRAY_SIZE(ghash_alg));
else
@@ -717,7 +717,7 @@ static int __init ghash_ce_mod_init(void)
if (ret)
return ret;
- if (elf_hwcap & HWCAP_PMULL) {
+ if (cpu_have_named_feature(PMULL)) {
ret = crypto_register_aead(&gcm_aes_alg);
if (ret)
crypto_unregister_shashes(ghash_alg,
@@ -728,7 +728,7 @@ static int __init ghash_ce_mod_init(void)
static void __exit ghash_ce_mod_exit(void)
{
- if (elf_hwcap & HWCAP_PMULL)
+ if (cpu_have_named_feature(PMULL))
crypto_unregister_shashes(ghash_alg, ARRAY_SIZE(ghash_alg));
else
crypto_unregister_shash(ghash_alg);
diff --git a/arch/arm64/crypto/nhpoly1305-neon-glue.c b/arch/arm64/crypto/nhpoly1305-neon-glue.c
index 22cc32ac9448..38a589044b6c 100644
--- a/arch/arm64/crypto/nhpoly1305-neon-glue.c
+++ b/arch/arm64/crypto/nhpoly1305-neon-glue.c
@@ -56,7 +56,7 @@ static struct shash_alg nhpoly1305_alg = {
static int __init nhpoly1305_mod_init(void)
{
- if (!(elf_hwcap & HWCAP_ASIMD))
+ if (!cpu_have_named_feature(ASIMD))
return -ENODEV;
return crypto_register_shash(&nhpoly1305_alg);
diff --git a/arch/arm64/crypto/sha256-glue.c b/arch/arm64/crypto/sha256-glue.c
index 4aedeaefd61f..0cccdb9cc2c0 100644
--- a/arch/arm64/crypto/sha256-glue.c
+++ b/arch/arm64/crypto/sha256-glue.c
@@ -173,7 +173,7 @@ static int __init sha256_mod_init(void)
if (ret)
return ret;
- if (elf_hwcap & HWCAP_ASIMD) {
+ if (cpu_have_named_feature(ASIMD)) {
ret = crypto_register_shashes(neon_algs, ARRAY_SIZE(neon_algs));
if (ret)
crypto_unregister_shashes(algs, ARRAY_SIZE(algs));
@@ -183,7 +183,7 @@ static int __init sha256_mod_init(void)
static void __exit sha256_mod_fini(void)
{
- if (elf_hwcap & HWCAP_ASIMD)
+ if (cpu_have_named_feature(ASIMD))
crypto_unregister_shashes(neon_algs, ARRAY_SIZE(neon_algs));
crypto_unregister_shashes(algs, ARRAY_SIZE(algs));
}
diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h
index e505e1fbd2b9..347c17046668 100644
--- a/arch/arm64/include/asm/cpufeature.h
+++ b/arch/arm64/include/asm/cpufeature.h
@@ -14,15 +14,8 @@
#include <asm/hwcap.h>
#include <asm/sysreg.h>
-/*
- * In the arm64 world (as in the ARM world), elf_hwcap is used both internally
- * in the kernel and for user space to keep track of which optional features
- * are supported by the current system. So let's map feature 'x' to HWCAP_x.
- * Note that HWCAP_x constants are bit fields so we need to take the log.
- */
-
-#define MAX_CPU_FEATURES (8 * sizeof(elf_hwcap))
-#define cpu_feature(x) ilog2(HWCAP_ ## x)
+#define MAX_CPU_FEATURES 64
+#define cpu_feature(x) KERNEL_HWCAP_ ## x
#ifndef __ASSEMBLY__
@@ -400,10 +393,19 @@ extern DECLARE_BITMAP(boot_capabilities, ARM64_NPATCHABLE);
bool this_cpu_has_cap(unsigned int cap);
+static inline void cpu_set_feature(unsigned int num)
+{
+ WARN_ON(num >= MAX_CPU_FEATURES);
+ elf_hwcap |= BIT(num);
+}
+#define cpu_set_named_feature(name) cpu_set_feature(cpu_feature(name))
+
static inline bool cpu_have_feature(unsigned int num)
{
- return elf_hwcap & (1UL << num);
+ WARN_ON(num >= MAX_CPU_FEATURES);
+ return elf_hwcap & BIT(num);
}
+#define cpu_have_named_feature(name) cpu_have_feature(cpu_feature(name))
/* System capability check for constant caps */
static inline bool __cpus_have_const_cap(int num)
diff --git a/arch/arm64/include/asm/hwcap.h b/arch/arm64/include/asm/hwcap.h
index 400b80b49595..1f38a2740f7a 100644
--- a/arch/arm64/include/asm/hwcap.h
+++ b/arch/arm64/include/asm/hwcap.h
@@ -39,12 +39,61 @@
#define COMPAT_HWCAP2_SHA2 (1 << 3)
#define COMPAT_HWCAP2_CRC32 (1 << 4)
+/*
+ * For userspace we represent hwcaps as a collection of HWCAP{,2}_x bitfields
+ * as described in uapi/asm/hwcap.h. For the kernel we represent hwcaps as
+ * natural numbers (in a single range of size MAX_CPU_FEATURES) defined here
+ * with prefix KERNEL_HWCAP_ mapped to their HWCAP{,2}_x counterpart.
+ *
+ * Hwcaps should be set and tested within the kernel via the
+ * cpu_{set,have}_named_feature(feature) where feature is the unique suffix
+ * of KERNEL_HWCAP_{feature}.
+ */
+#define __khwcap_feature(x) ilog2(HWCAP_ ## x)
+#define KERNEL_HWCAP_FP __khwcap_feature(FP)
+#define KERNEL_HWCAP_ASIMD __khwcap_feature(ASIMD)
+#define KERNEL_HWCAP_EVTSTRM __khwcap_feature(EVTSTRM)
+#define KERNEL_HWCAP_AES __khwcap_feature(AES)
+#define KERNEL_HWCAP_PMULL __khwcap_feature(PMULL)
+#define KERNEL_HWCAP_SHA1 __khwcap_feature(SHA1)
+#define KERNEL_HWCAP_SHA2 __khwcap_feature(SHA2)
+#define KERNEL_HWCAP_CRC32 __khwcap_feature(CRC32)
+#define KERNEL_HWCAP_ATOMICS __khwcap_feature(ATOMICS)
+#define KERNEL_HWCAP_FPHP __khwcap_feature(FPHP)
+#define KERNEL_HWCAP_ASIMDHP __khwcap_feature(ASIMDHP)
+#define KERNEL_HWCAP_CPUID __khwcap_feature(CPUID)
+#define KERNEL_HWCAP_ASIMDRDM __khwcap_feature(ASIMDRDM)
+#define KERNEL_HWCAP_JSCVT __khwcap_feature(JSCVT)
+#define KERNEL_HWCAP_FCMA __khwcap_feature(FCMA)
+#define KERNEL_HWCAP_LRCPC __khwcap_feature(LRCPC)
+#define KERNEL_HWCAP_DCPOP __khwcap_feature(DCPOP)
+#define KERNEL_HWCAP_SHA3 __khwcap_feature(SHA3)
+#define KERNEL_HWCAP_SM3 __khwcap_feature(SM3)
+#define KERNEL_HWCAP_SM4 __khwcap_feature(SM4)
+#define KERNEL_HWCAP_ASIMDDP __khwcap_feature(ASIMDDP)
+#define KERNEL_HWCAP_SHA512 __khwcap_feature(SHA512)
+#define KERNEL_HWCAP_SVE __khwcap_feature(SVE)
+#define KERNEL_HWCAP_ASIMDFHM __khwcap_feature(ASIMDFHM)
+#define KERNEL_HWCAP_DIT __khwcap_feature(DIT)
+#define KERNEL_HWCAP_USCAT __khwcap_feature(USCAT)
+#define KERNEL_HWCAP_ILRCPC __khwcap_feature(ILRCPC)
+#define KERNEL_HWCAP_FLAGM __khwcap_feature(FLAGM)
+#define KERNEL_HWCAP_SSBS __khwcap_feature(SSBS)
+#define KERNEL_HWCAP_SB __khwcap_feature(SB)
+#define KERNEL_HWCAP_PACA __khwcap_feature(PACA)
+#define KERNEL_HWCAP_PACG __khwcap_feature(PACG)
+
+#define __khwcap2_feature(x) (ilog2(HWCAP2_ ## x) + 32)
+
#ifndef __ASSEMBLY__
+#include <linux/kernel.h>
+
/*
* This yields a mask that user programs can use to figure out what
* instruction set this cpu supports.
*/
-#define ELF_HWCAP (elf_hwcap)
+#define ELF_HWCAP lower_32_bits(elf_hwcap)
+#define ELF_HWCAP2 upper_32_bits(elf_hwcap)
#ifdef CONFIG_COMPAT
#define COMPAT_ELF_HWCAP (compat_elf_hwcap)
diff --git a/arch/arm64/include/uapi/asm/hwcap.h b/arch/arm64/include/uapi/asm/hwcap.h
index 5f0750c2199c..453b45af80b7 100644
--- a/arch/arm64/include/uapi/asm/hwcap.h
+++ b/arch/arm64/include/uapi/asm/hwcap.h
@@ -18,7 +18,7 @@
#define _UAPI__ASM_HWCAP_H
/*
- * HWCAP flags - for elf_hwcap (in kernel) and AT_HWCAP
+ * HWCAP flags - for AT_HWCAP
*/
#define HWCAP_FP (1 << 0)
#define HWCAP_ASIMD (1 << 1)
diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index 4061de10cea6..986ceeacd19f 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -1571,39 +1571,39 @@ static const struct arm64_cpu_capabilities ptr_auth_hwcap_gen_matches[] = {
#endif
static const struct arm64_cpu_capabilities arm64_elf_hwcaps[] = {
- HWCAP_CAP(SYS_ID_AA64ISAR0_EL1, ID_AA64ISAR0_AES_SHIFT, FTR_UNSIGNED, 2, CAP_HWCAP, HWCAP_PMULL),
- HWCAP_CAP(SYS_ID_AA64ISAR0_EL1, ID_AA64ISAR0_AES_SHIFT, FTR_UNSIGNED, 1, CAP_HWCAP, HWCAP_AES),
- HWCAP_CAP(SYS_ID_AA64ISAR0_EL1, ID_AA64ISAR0_SHA1_SHIFT, FTR_UNSIGNED, 1, CAP_HWCAP, HWCAP_SHA1),
- HWCAP_CAP(SYS_ID_AA64ISAR0_EL1, ID_AA64ISAR0_SHA2_SHIFT, FTR_UNSIGNED, 1, CAP_HWCAP, HWCAP_SHA2),
- HWCAP_CAP(SYS_ID_AA64ISAR0_EL1, ID_AA64ISAR0_SHA2_SHIFT, FTR_UNSIGNED, 2, CAP_HWCAP, HWCAP_SHA512),
- HWCAP_CAP(SYS_ID_AA64ISAR0_EL1, ID_AA64ISAR0_CRC32_SHIFT, FTR_UNSIGNED, 1, CAP_HWCAP, HWCAP_CRC32),
- HWCAP_CAP(SYS_ID_AA64ISAR0_EL1, ID_AA64ISAR0_ATOMICS_SHIFT, FTR_UNSIGNED, 2, CAP_HWCAP, HWCAP_ATOMICS),
- HWCAP_CAP(SYS_ID_AA64ISAR0_EL1, ID_AA64ISAR0_RDM_SHIFT, FTR_UNSIGNED, 1, CAP_HWCAP, HWCAP_ASIMDRDM),
- HWCAP_CAP(SYS_ID_AA64ISAR0_EL1, ID_AA64ISAR0_SHA3_SHIFT, FTR_UNSIGNED, 1, CAP_HWCAP, HWCAP_SHA3),
- HWCAP_CAP(SYS_ID_AA64ISAR0_EL1, ID_AA64ISAR0_SM3_SHIFT, FTR_UNSIGNED, 1, CAP_HWCAP, HWCAP_SM3),
- HWCAP_CAP(SYS_ID_AA64ISAR0_EL1, ID_AA64ISAR0_SM4_SHIFT, FTR_UNSIGNED, 1, CAP_HWCAP, HWCAP_SM4),
- HWCAP_CAP(SYS_ID_AA64ISAR0_EL1, ID_AA64ISAR0_DP_SHIFT, FTR_UNSIGNED, 1, CAP_HWCAP, HWCAP_ASIMDDP),
- HWCAP_CAP(SYS_ID_AA64ISAR0_EL1, ID_AA64ISAR0_FHM_SHIFT, FTR_UNSIGNED, 1, CAP_HWCAP, HWCAP_ASIMDFHM),
- HWCAP_CAP(SYS_ID_AA64ISAR0_EL1, ID_AA64ISAR0_TS_SHIFT, FTR_UNSIGNED, 1, CAP_HWCAP, HWCAP_FLAGM),
- HWCAP_CAP(SYS_ID_AA64PFR0_EL1, ID_AA64PFR0_FP_SHIFT, FTR_SIGNED, 0, CAP_HWCAP, HWCAP_FP),
- HWCAP_CAP(SYS_ID_AA64PFR0_EL1, ID_AA64PFR0_FP_SHIFT, FTR_SIGNED, 1, CAP_HWCAP, HWCAP_FPHP),
- HWCAP_CAP(SYS_ID_AA64PFR0_EL1, ID_AA64PFR0_ASIMD_SHIFT, FTR_SIGNED, 0, CAP_HWCAP, HWCAP_ASIMD),
- HWCAP_CAP(SYS_ID_AA64PFR0_EL1, ID_AA64PFR0_ASIMD_SHIFT, FTR_SIGNED, 1, CAP_HWCAP, HWCAP_ASIMDHP),
- HWCAP_CAP(SYS_ID_AA64PFR0_EL1, ID_AA64PFR0_DIT_SHIFT, FTR_SIGNED, 1, CAP_HWCAP, HWCAP_DIT),
- HWCAP_CAP(SYS_ID_AA64ISAR1_EL1, ID_AA64ISAR1_DPB_SHIFT, FTR_UNSIGNED, 1, CAP_HWCAP, HWCAP_DCPOP),
- HWCAP_CAP(SYS_ID_AA64ISAR1_EL1, ID_AA64ISAR1_JSCVT_SHIFT, FTR_UNSIGNED, 1, CAP_HWCAP, HWCAP_JSCVT),
- HWCAP_CAP(SYS_ID_AA64ISAR1_EL1, ID_AA64ISAR1_FCMA_SHIFT, FTR_UNSIGNED, 1, CAP_HWCAP, HWCAP_FCMA),
- HWCAP_CAP(SYS_ID_AA64ISAR1_EL1, ID_AA64ISAR1_LRCPC_SHIFT, FTR_UNSIGNED, 1, CAP_HWCAP, HWCAP_LRCPC),
- HWCAP_CAP(SYS_ID_AA64ISAR1_EL1, ID_AA64ISAR1_LRCPC_SHIFT, FTR_UNSIGNED, 2, CAP_HWCAP, HWCAP_ILRCPC),
- HWCAP_CAP(SYS_ID_AA64ISAR1_EL1, ID_AA64ISAR1_SB_SHIFT, FTR_UNSIGNED, 1, CAP_HWCAP, HWCAP_SB),
- HWCAP_CAP(SYS_ID_AA64MMFR2_EL1, ID_AA64MMFR2_AT_SHIFT, FTR_UNSIGNED, 1, CAP_HWCAP, HWCAP_USCAT),
+ HWCAP_CAP(SYS_ID_AA64ISAR0_EL1, ID_AA64ISAR0_AES_SHIFT, FTR_UNSIGNED, 2, CAP_HWCAP, KERNEL_HWCAP_PMULL),
+ HWCAP_CAP(SYS_ID_AA64ISAR0_EL1, ID_AA64ISAR0_AES_SHIFT, FTR_UNSIGNED, 1, CAP_HWCAP, KERNEL_HWCAP_AES),
+ HWCAP_CAP(SYS_ID_AA64ISAR0_EL1, ID_AA64ISAR0_SHA1_SHIFT, FTR_UNSIGNED, 1, CAP_HWCAP, KERNEL_HWCAP_SHA1),
+ HWCAP_CAP(SYS_ID_AA64ISAR0_EL1, ID_AA64ISAR0_SHA2_SHIFT, FTR_UNSIGNED, 1, CAP_HWCAP, KERNEL_HWCAP_SHA2),
+ HWCAP_CAP(SYS_ID_AA64ISAR0_EL1, ID_AA64ISAR0_SHA2_SHIFT, FTR_UNSIGNED, 2, CAP_HWCAP, KERNEL_HWCAP_SHA512),
+ HWCAP_CAP(SYS_ID_AA64ISAR0_EL1, ID_AA64ISAR0_CRC32_SHIFT, FTR_UNSIGNED, 1, CAP_HWCAP, KERNEL_HWCAP_CRC32),
+ HWCAP_CAP(SYS_ID_AA64ISAR0_EL1, ID_AA64ISAR0_ATOMICS_SHIFT, FTR_UNSIGNED, 2, CAP_HWCAP, KERNEL_HWCAP_ATOMICS),
+ HWCAP_CAP(SYS_ID_AA64ISAR0_EL1, ID_AA64ISAR0_RDM_SHIFT, FTR_UNSIGNED, 1, CAP_HWCAP, KERNEL_HWCAP_ASIMDRDM),
+ HWCAP_CAP(SYS_ID_AA64ISAR0_EL1, ID_AA64ISAR0_SHA3_SHIFT, FTR_UNSIGNED, 1, CAP_HWCAP, KERNEL_HWCAP_SHA3),
+ HWCAP_CAP(SYS_ID_AA64ISAR0_EL1, ID_AA64ISAR0_SM3_SHIFT, FTR_UNSIGNED, 1, CAP_HWCAP, KERNEL_HWCAP_SM3),
+ HWCAP_CAP(SYS_ID_AA64ISAR0_EL1, ID_AA64ISAR0_SM4_SHIFT, FTR_UNSIGNED, 1, CAP_HWCAP, KERNEL_HWCAP_SM4),
+ HWCAP_CAP(SYS_ID_AA64ISAR0_EL1, ID_AA64ISAR0_DP_SHIFT, FTR_UNSIGNED, 1, CAP_HWCAP, KERNEL_HWCAP_ASIMDDP),
+ HWCAP_CAP(SYS_ID_AA64ISAR0_EL1, ID_AA64ISAR0_FHM_SHIFT, FTR_UNSIGNED, 1, CAP_HWCAP, KERNEL_HWCAP_ASIMDFHM),
+ HWCAP_CAP(SYS_ID_AA64ISAR0_EL1, ID_AA64ISAR0_TS_SHIFT, FTR_UNSIGNED, 1, CAP_HWCAP, KERNEL_HWCAP_FLAGM),
+ HWCAP_CAP(SYS_ID_AA64PFR0_EL1, ID_AA64PFR0_FP_SHIFT, FTR_SIGNED, 0, CAP_HWCAP, KERNEL_HWCAP_FP),
+ HWCAP_CAP(SYS_ID_AA64PFR0_EL1, ID_AA64PFR0_FP_SHIFT, FTR_SIGNED, 1, CAP_HWCAP, KERNEL_HWCAP_FPHP),
+ HWCAP_CAP(SYS_ID_AA64PFR0_EL1, ID_AA64PFR0_ASIMD_SHIFT, FTR_SIGNED, 0, CAP_HWCAP, KERNEL_HWCAP_ASIMD),
+ HWCAP_CAP(SYS_ID_AA64PFR0_EL1, ID_AA64PFR0_ASIMD_SHIFT, FTR_SIGNED, 1, CAP_HWCAP, KERNEL_HWCAP_ASIMDHP),
+ HWCAP_CAP(SYS_ID_AA64PFR0_EL1, ID_AA64PFR0_DIT_SHIFT, FTR_SIGNED, 1, CAP_HWCAP, KERNEL_HWCAP_DIT),
+ HWCAP_CAP(SYS_ID_AA64ISAR1_EL1, ID_AA64ISAR1_DPB_SHIFT, FTR_UNSIGNED, 1, CAP_HWCAP, KERNEL_HWCAP_DCPOP),
+ HWCAP_CAP(SYS_ID_AA64ISAR1_EL1, ID_AA64ISAR1_JSCVT_SHIFT, FTR_UNSIGNED, 1, CAP_HWCAP, KERNEL_HWCAP_JSCVT),
+ HWCAP_CAP(SYS_ID_AA64ISAR1_EL1, ID_AA64ISAR1_FCMA_SHIFT, FTR_UNSIGNED, 1, CAP_HWCAP, KERNEL_HWCAP_FCMA),
+ HWCAP_CAP(SYS_ID_AA64ISAR1_EL1, ID_AA64ISAR1_LRCPC_SHIFT, FTR_UNSIGNED, 1, CAP_HWCAP, KERNEL_HWCAP_LRCPC),
+ HWCAP_CAP(SYS_ID_AA64ISAR1_EL1, ID_AA64ISAR1_LRCPC_SHIFT, FTR_UNSIGNED, 2, CAP_HWCAP, KERNEL_HWCAP_ILRCPC),
+ HWCAP_CAP(SYS_ID_AA64ISAR1_EL1, ID_AA64ISAR1_SB_SHIFT, FTR_UNSIGNED, 1, CAP_HWCAP, KERNEL_HWCAP_SB),
+ HWCAP_CAP(SYS_ID_AA64MMFR2_EL1, ID_AA64MMFR2_AT_SHIFT, FTR_UNSIGNED, 1, CAP_HWCAP, KERNEL_HWCAP_USCAT),
#ifdef CONFIG_ARM64_SVE
- HWCAP_CAP(SYS_ID_AA64PFR0_EL1, ID_AA64PFR0_SVE_SHIFT, FTR_UNSIGNED, ID_AA64PFR0_SVE, CAP_HWCAP, HWCAP_SVE),
+ HWCAP_CAP(SYS_ID_AA64PFR0_EL1, ID_AA64PFR0_SVE_SHIFT, FTR_UNSIGNED, ID_AA64PFR0_SVE, CAP_HWCAP, KERNEL_HWCAP_SVE),
#endif
- HWCAP_CAP(SYS_ID_AA64PFR1_EL1, ID_AA64PFR1_SSBS_SHIFT, FTR_UNSIGNED, ID_AA64PFR1_SSBS_PSTATE_INSNS, CAP_HWCAP, HWCAP_SSBS),
+ HWCAP_CAP(SYS_ID_AA64PFR1_EL1, ID_AA64PFR1_SSBS_SHIFT, FTR_UNSIGNED, ID_AA64PFR1_SSBS_PSTATE_INSNS, CAP_HWCAP, KERNEL_HWCAP_SSBS),
#ifdef CONFIG_ARM64_PTR_AUTH
- HWCAP_MULTI_CAP(ptr_auth_hwcap_addr_matches, CAP_HWCAP, HWCAP_PACA),
- HWCAP_MULTI_CAP(ptr_auth_hwcap_gen_matches, CAP_HWCAP, HWCAP_PACG),
+ HWCAP_MULTI_CAP(ptr_auth_hwcap_addr_matches, CAP_HWCAP, KERNEL_HWCAP_PACA),
+ HWCAP_MULTI_CAP(ptr_auth_hwcap_gen_matches, CAP_HWCAP, KERNEL_HWCAP_PACG),
#endif
{},
};
@@ -1623,7 +1623,7 @@ static void __init cap_set_elf_hwcap(const struct arm64_cpu_capabilities *cap)
{
switch (cap->hwcap_type) {
case CAP_HWCAP:
- elf_hwcap |= cap->hwcap;
+ cpu_set_feature(cap->hwcap);
break;
#ifdef CONFIG_COMPAT
case CAP_COMPAT_HWCAP:
@@ -1646,7 +1646,7 @@ static bool cpus_have_elf_hwcap(const struct arm64_cpu_capabilities *cap)
switch (cap->hwcap_type) {
case CAP_HWCAP:
- rc = (elf_hwcap & cap->hwcap) != 0;
+ rc = cpu_have_feature(cap->hwcap);
break;
#ifdef CONFIG_COMPAT
case CAP_COMPAT_HWCAP:
@@ -1667,7 +1667,7 @@ static bool cpus_have_elf_hwcap(const struct arm64_cpu_capabilities *cap)
static void __init setup_elf_hwcaps(const struct arm64_cpu_capabilities *hwcaps)
{
/* We support emulation of accesses to CPU ID feature registers */
- elf_hwcap |= HWCAP_CPUID;
+ cpu_set_named_feature(CPUID);
for (; hwcaps->matches; hwcaps++)
if (hwcaps->matches(hwcaps, cpucap_default_scope(hwcaps)))
cap_set_elf_hwcap(hwcaps);
diff --git a/arch/arm64/kernel/cpuinfo.c b/arch/arm64/kernel/cpuinfo.c
index ca0685f33900..810db95f293f 100644
--- a/arch/arm64/kernel/cpuinfo.c
+++ b/arch/arm64/kernel/cpuinfo.c
@@ -167,7 +167,7 @@ static int c_show(struct seq_file *m, void *v)
#endif /* CONFIG_COMPAT */
} else {
for (j = 0; hwcap_str[j]; j++)
- if (elf_hwcap & (1 << j))
+ if (cpu_have_feature(j))
seq_printf(m, " %s", hwcap_str[j]);
}
seq_puts(m, "\n");
diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c
index 5ebe73b69961..735cf1f8b109 100644
--- a/arch/arm64/kernel/fpsimd.c
+++ b/arch/arm64/kernel/fpsimd.c
@@ -1258,14 +1258,14 @@ static inline void fpsimd_hotplug_init(void) { }
*/
static int __init fpsimd_init(void)
{
- if (elf_hwcap & HWCAP_FP) {
+ if (cpu_have_named_feature(FP)) {
fpsimd_pm_init();
fpsimd_hotplug_init();
} else {
pr_notice("Floating-point is not implemented\n");
}
- if (!(elf_hwcap & HWCAP_ASIMD))
+ if (!cpu_have_named_feature(ASIMD))
pr_notice("Advanced SIMD is not implemented\n");
return sve_sysctl_init();
diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
index aa4ec53281ce..6cc8aff83805 100644
--- a/drivers/clocksource/arm_arch_timer.c
+++ b/drivers/clocksource/arm_arch_timer.c
@@ -833,7 +833,11 @@ static void arch_timer_evtstrm_enable(int divider)
cntkctl |= (divider << ARCH_TIMER_EVT_TRIGGER_SHIFT)
| ARCH_TIMER_VIRT_EVT_EN;
arch_timer_set_cntkctl(cntkctl);
+#ifdef CONFIG_ARM64
+ cpu_set_named_feature(EVTSTRM);
+#else
elf_hwcap |= HWCAP_EVTSTRM;
+#endif
#ifdef CONFIG_COMPAT
compat_elf_hwcap |= COMPAT_HWCAP_EVTSTRM;
#endif
@@ -1055,7 +1059,11 @@ static int arch_timer_cpu_pm_notify(struct notifier_block *self,
} else if (action == CPU_PM_ENTER_FAILED || action == CPU_PM_EXIT) {
arch_timer_set_cntkctl(__this_cpu_read(saved_cntkctl));
+#ifdef CONFIG_ARM64
+ if (cpu_have_named_feature(EVTSTRM))
+#else
if (elf_hwcap & HWCAP_EVTSTRM)
+#endif
cpumask_set_cpu(smp_processor_id(), &evtstrm_available);
}
return NOTIFY_OK;
--
2.21.0
^ permalink raw reply related
* [PATCH v4 0/6] arm64: Initial support for CVADP
From: Andrew Murray @ 2019-04-03 10:56 UTC (permalink / raw)
To: Catalin Marinas, Will Deacon
Cc: Szabolcs Nagy, dave.martin, linux-arm-kernel, Mark Rutland,
Phil Blundell, libc-alpha, linux-api, Suzuki K Poulose
ARMv8.5 introduces a DC CVADP instruction which cleans the data cache to
the point of deep persistence. This series makes the instruction
available to userspace and advertises the presence of this CPU feature.
At present when CONFIG_ARM64_PMEM is enabled and the CVAP feature is
present (ARMv8.2) the CVAP instruction is used (from memcpy_flushcache
and arch_wb_cache_pmem). No changes have been made to use CVADP in
these functions or similar.
As we have moved beyond 32 capabilities we now begin using AT_HWCAP2
for userspace.
Tested as follows:
$ dmesg | grep "Deep"
[ 0.166496] CPU features: detected: Data cache clean to Point of Deep Persistence
$ LD_SHOW_AUXV=1 sleep 2>&1 | grep AT_HWCAP
AT_HWCAP: ef91ff87
AT_HWCAP2: 0x1
Changes since v3:
- Rebased onto v5.1-rc3
- Add macros for mapping uapi HWCAPs to KERNEL_HWCAPs
- Squash AT_HWCAP2 documentation into AT_HWCAP2 patch
- Reorder series to allow HWCAP changes to be taken without
CVADP (if needed)
- Additional comments, changes to comments and commit
messages
Changes since v2:
- Rebased onto v5.1-rc2
- Renamed cpu_{have,set}_feature_name to cpu_{have,set}_named_feature
- Add additional comments and update kernel Documentation
Changes since v1:
- Rebased onto v5.0-rc7
- Introduced cpu_{have,set}_feature_name to eliminate use of
KERNEL_HWCAP prefix
- Hard coded MAX_CPU_FEATURES and added a WARN_ON
- Minor comment and tab/spacing changes
- Use elf_hwcap for all 64 caps in the kernel instead of
a new elf_hwcap2
Andrew Murray (6):
arm64: HWCAP: add support for AT_HWCAP2
arm64: HWCAP: encapsulate elf_hwcap
arm64: Handle trapped DC CVADP
arm64: Expose DC CVADP to userspace
arm64: add CVADP support to the cache maintenance helper
arm64: Advertise ARM64_HAS_DCPODP cpu feature
Documentation/arm64/elf_hwcaps.txt | 18 +++-
arch/arm64/crypto/aes-ce-ccm-glue.c | 2 +-
arch/arm64/crypto/aes-neonbs-glue.c | 2 +-
arch/arm64/crypto/chacha-neon-glue.c | 2 +-
arch/arm64/crypto/crct10dif-ce-glue.c | 4 +-
arch/arm64/crypto/ghash-ce-glue.c | 8 +-
arch/arm64/crypto/nhpoly1305-neon-glue.c | 2 +-
arch/arm64/crypto/sha256-glue.c | 4 +-
arch/arm64/include/asm/assembler.h | 4 +
arch/arm64/include/asm/cpucaps.h | 3 +-
arch/arm64/include/asm/cpufeature.h | 21 ++---
arch/arm64/include/asm/esr.h | 3 +-
arch/arm64/include/asm/hwcap.h | 53 ++++++++++-
arch/arm64/include/uapi/asm/hwcap.h | 7 +-
arch/arm64/kernel/cpufeature.c | 109 +++++++++++++++--------
arch/arm64/kernel/cpuinfo.c | 3 +-
arch/arm64/kernel/fpsimd.c | 4 +-
arch/arm64/kernel/traps.c | 3 +
drivers/clocksource/arm_arch_timer.c | 8 ++
19 files changed, 189 insertions(+), 71 deletions(-)
--
2.21.0
^ permalink raw reply
* Re: [RESEND PATCH v1] moduleparam: Save information about built-in modules in separate file
From: Masahiro Yamada @ 2019-04-03 10:48 UTC (permalink / raw)
To: Alexey Gladkov
Cc: Michal Marek, Linux Kernel Mailing List,
Linux Kbuild mailing list, linux-api, Kirill A . Shutemov,
Gleb Fotengauer-Malinovskiy, Dmitry V. Levin, Dmitry Torokhov,
Rusty Russell, Jessica Yu
In-Reply-To: <CAK7LNARd75HS_aBZgH0YhR-w1kipr0rn7g1VGOjWev8giO04tg@mail.gmail.com>
On Wed, Apr 3, 2019 at 7:45 PM Masahiro Yamada
<yamada.masahiro@socionext.com> wrote:
>
> Hi.
>
> Sorry for the delay.
> I added some comments from the
> build system point of view.
>
>
> On Wed, Mar 27, 2019 at 2:24 AM Alexey Gladkov <gladkov.alexey@gmail.com> wrote:
> >
> > On Fri, Mar 22, 2019 at 02:34:12PM +0900, Masahiro Yamada wrote:
> > > Hi.
> > >
> > > (added some people to CC)
> > >
> > >
> > > On Fri, Mar 15, 2019 at 7:10 PM Alexey Gladkov <gladkov.alexey@gmail.com> wrote:
> > > >
> > > > Problem:
> > > >
> > > > When a kernel module is compiled as a separate module, some important
> > > > information about the kernel module is available via .modinfo section of
> > > > the module. In contrast, when the kernel module is compiled into the
> > > > kernel, that information is not available.
> > >
> > >
> > > I might be missing something, but
> > > vmlinux provides info of builtin modules
> > > in /sys/module/.
> >
> > No. There are definitely not all modules. I have a builtin sha256_generic,
> > but I can't find him in the /sys/module.
> >
> > > (Looks like currently only module_param and MODULE_VERSION)
> > >
> > > This patch is not exactly the same, but I see a kind of overwrap.
> > > I'd like to be sure if we want this new scheme.
> >
> > The /sys/module is only for running kernel. One of my use cases is
> > to create an initrd for a new kernel.
> >
> > >
> > > > Information about built-in modules is necessary in the following cases:
> > > >
> > > > 1. When it is necessary to find out what additional parameters can be
> > > > passed to the kernel at boot time.
> > >
> > >
> > > Actually, /sys/module/<module>/parameters/
> > > exposes this information.
> > >
> > > Doesn't it work for your purpose?
> >
> > No, since creating an initrd needs to know all the modalias before
> > I get the sysfs for new kernel. Also there are no modalias at all.
> >
> > > > 2. When you need to know which module names and their aliases are in
> > > > the kernel. This is very useful for creating an initrd image.
> > > >
> > > > Proposal:
> > > >
> > > > The proposed patch does not remove .modinfo section with module
> > > > information from the vmlinux at the build time and saves it into a
> > > > separate file after kernel linking. So, the kernel does not increase in
> > > > size and no additional information remains in it. Information is stored
> > > > in the same format as in the separate modules (null-terminated string
> > > > array). Because the .modinfo section is already exported with a separate
> > > > modules, we are not creating a new API.
> > > >
> > > > It can be easily read in the userspace:
> > > >
> > > > $ tr '\0' '\n' < kernel.builtin.modinfo
> > > > ext4.softdep=pre: crc32c
> > > > ext4.license=GPL
> > > > ext4.description=Fourth Extended Filesystem
> > > > ext4.author=Remy Card, Stephen Tweedie, Andrew Morton, Andreas Dilger, Theodore Ts'o and others
> > > > ext4.alias=fs-ext4
> > > > ext4.alias=ext3
> > > > ext4.alias=fs-ext3
> > > > ext4.alias=ext2
> > > > ext4.alias=fs-ext2
> > > > md_mod.alias=block-major-9-*
> > > > md_mod.alias=md
> > > > md_mod.description=MD RAID framework
> > > > md_mod.license=GPL
> > > > md_mod.parmtype=create_on_open:bool
> > > > md_mod.parmtype=start_dirty_degraded:int
> > > > ...
> > > >
> > > > Co-Developed-by: Gleb Fotengauer-Malinovskiy <glebfm@altlinux.org>
> > > > Signed-off-by: Gleb Fotengauer-Malinovskiy <glebfm@altlinux.org>
> > > > Signed-off-by: Alexey Gladkov <gladkov.alexey@gmail.com>
> > > > ---
> > > > Makefile | 1 +
> > > > include/linux/moduleparam.h | 12 +++++-------
> > > > scripts/link-vmlinux.sh | 8 ++++++++
> > > > 3 files changed, 14 insertions(+), 7 deletions(-)
> > > >
> > > > diff --git a/Makefile b/Makefile
> > > > index d5713e7b1e50..971102194c92 100644
> > > > --- a/Makefile
> > > > +++ b/Makefile
> > > > @@ -1288,6 +1288,7 @@ _modinst_:
> > > > fi
> > > > @cp -f $(objtree)/modules.order $(MODLIB)/
> > > > @cp -f $(objtree)/modules.builtin $(MODLIB)/
> > > > + @cp -f $(objtree)/kernel.builtin.modinfo $(MODLIB)/
>
>
> This file should be cleaned up by 'make clean'.
>
> Please add CLEAN_FILES around line 1336.
>
>
> # Directories & files removed with 'make clean'
> CLEAN_DIRS += $(MODVERDIR) include/ksym
> CLEAN_FILES += ...
>
>
>
>
> Also, shouldn't it be added to .gitignore?
>
>
>
>
> > > > $(Q)$(MAKE) -f $(srctree)/scripts/Makefile.modinst
> > > >
> > > > # This depmod is only for convenience to give the initial
> > > > diff --git a/include/linux/moduleparam.h b/include/linux/moduleparam.h
> > > > index ba36506db4fb..5ba250d9172a 100644
> > > > --- a/include/linux/moduleparam.h
> > > > +++ b/include/linux/moduleparam.h
> > > > @@ -10,23 +10,21 @@
> > > > module name. */
> > > > #ifdef MODULE
> > > > #define MODULE_PARAM_PREFIX /* empty */
> > > > +#define __MODULE_INFO_PREFIX /* empty */
> > > > #else
> > > > #define MODULE_PARAM_PREFIX KBUILD_MODNAME "."
> > > > +/* We cannot use MODULE_PARAM_PREFIX because some modules override it. */
> > > > +#define __MODULE_INFO_PREFIX KBUILD_MODNAME "."
> > > > #endif
> > > >
> > > > /* Chosen so that structs with an unsigned long line up. */
> > > > #define MAX_PARAM_PREFIX_LEN (64 - sizeof(unsigned long))
> > > >
> > > > -#ifdef MODULE
> > > > #define __MODULE_INFO(tag, name, info) \
> > > > static const char __UNIQUE_ID(name)[] \
> > > > __used __attribute__((section(".modinfo"), unused, aligned(1))) \
> > > > - = __stringify(tag) "=" info
> > > > -#else /* !MODULE */
> > > > -/* This struct is here for syntactic coherency, it is not used */
> > > > -#define __MODULE_INFO(tag, name, info) \
> > > > - struct __UNIQUE_ID(name) {}
> > > > -#endif
> > > > + = __MODULE_INFO_PREFIX __stringify(tag) "=" info
> > > > +
> > > > #define __MODULE_PARM_TYPE(name, _type) \
> > > > __MODULE_INFO(parmtype, name##type, #name ":" _type)
> > > >
> > > > diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
> > > > index c8cf45362bd6..399d7e4d11ec 100755
> > > > --- a/scripts/link-vmlinux.sh
> > > > +++ b/scripts/link-vmlinux.sh
> > > > @@ -258,10 +258,12 @@ if [ -n "${CONFIG_KALLSYMS}" ]; then
> > > >
> > > > # step 1
> > > > vmlinux_link "" .tmp_vmlinux1
> > > > + "${OBJCOPY}" -R .modinfo .tmp_vmlinux1
> > > > kallsyms .tmp_vmlinux1 .tmp_kallsyms1.o
> > > >
> > > > # step 2
> > > > vmlinux_link .tmp_kallsyms1.o .tmp_vmlinux2
> > > > + "${OBJCOPY}" -R .modinfo .tmp_vmlinux2
> > > > kallsyms .tmp_vmlinux2 .tmp_kallsyms2.o
> > > >
> > > > # step 3
> > > > @@ -273,6 +275,7 @@ if [ -n "${CONFIG_KALLSYMS}" ]; then
> > > > kallsyms_vmlinux=.tmp_vmlinux3
> > > >
> > > > vmlinux_link .tmp_kallsyms2.o .tmp_vmlinux3
> > > > + "${OBJCOPY}" -R .modinfo .tmp_vmlinux3
> > > >
> > > > kallsyms .tmp_vmlinux3 .tmp_kallsyms3.o
> > > > fi
> > > > @@ -281,6 +284,11 @@ fi
> > > > info LD vmlinux
> > > > vmlinux_link "${kallsymso}" vmlinux
> > > >
> > > > +info MODINFO kernel.builtin.modinfo
> > > > +"${OBJCOPY}" -j .modinfo -O binary vmlinux kernel.builtin.modinfo
> > > > +chmod 444 kernel.builtin.modinfo
> > > > +"${OBJCOPY}" -R .modinfo vmlinux
> > > > +
> > > > if [ -n "${CONFIG_BUILDTIME_EXTABLE_SORT}" ]; then
> > > > info SORTEX vmlinux
> > > > sortextable vmlinux
>
>
>
> As I pointed out before, I see lots of warnings for powerpc.
>
> How about extracting modinfo before linking vmlinux?
> You can extract it from vmlinux.o instead,
> and discard it in the linker script.
>
>
> diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
> index dc0e8c5..994cd47 100755
> --- a/scripts/link-vmlinux.sh
> +++ b/scripts/link-vmlinux.sh
> @@ -193,6 +193,9 @@ modpost_link vmlinux.o
> # modpost vmlinux.o to check for section mismatches
> ${MAKE} -f "${srctree}/scripts/Makefile.modpost" vmlinux.o
>
> +info MODINFO kernel.builtin.modinfo
> +"${OBJCOPY}" -j .modinfo -O binary vmlinux kernel.builtin.modinfo
Oops, this should be:
"${OBJCOPY}" -j .modinfo -O binary vmlinux.o kernel.builtin.modinfo
--
Best Regards
Masahiro Yamada
^ permalink raw reply
* Re: [RESEND PATCH v1] moduleparam: Save information about built-in modules in separate file
From: Masahiro Yamada @ 2019-04-03 10:45 UTC (permalink / raw)
To: Alexey Gladkov
Cc: Michal Marek, Linux Kernel Mailing List,
Linux Kbuild mailing list, linux-api, Kirill A . Shutemov,
Gleb Fotengauer-Malinovskiy, Dmitry V. Levin, Dmitry Torokhov,
Rusty Russell, Jessica Yu
In-Reply-To: <20190326172411.GA15936@Legion-PC.fortress>
Hi.
Sorry for the delay.
I added some comments from the
build system point of view.
On Wed, Mar 27, 2019 at 2:24 AM Alexey Gladkov <gladkov.alexey@gmail.com> wrote:
>
> On Fri, Mar 22, 2019 at 02:34:12PM +0900, Masahiro Yamada wrote:
> > Hi.
> >
> > (added some people to CC)
> >
> >
> > On Fri, Mar 15, 2019 at 7:10 PM Alexey Gladkov <gladkov.alexey@gmail.com> wrote:
> > >
> > > Problem:
> > >
> > > When a kernel module is compiled as a separate module, some important
> > > information about the kernel module is available via .modinfo section of
> > > the module. In contrast, when the kernel module is compiled into the
> > > kernel, that information is not available.
> >
> >
> > I might be missing something, but
> > vmlinux provides info of builtin modules
> > in /sys/module/.
>
> No. There are definitely not all modules. I have a builtin sha256_generic,
> but I can't find him in the /sys/module.
>
> > (Looks like currently only module_param and MODULE_VERSION)
> >
> > This patch is not exactly the same, but I see a kind of overwrap.
> > I'd like to be sure if we want this new scheme.
>
> The /sys/module is only for running kernel. One of my use cases is
> to create an initrd for a new kernel.
>
> >
> > > Information about built-in modules is necessary in the following cases:
> > >
> > > 1. When it is necessary to find out what additional parameters can be
> > > passed to the kernel at boot time.
> >
> >
> > Actually, /sys/module/<module>/parameters/
> > exposes this information.
> >
> > Doesn't it work for your purpose?
>
> No, since creating an initrd needs to know all the modalias before
> I get the sysfs for new kernel. Also there are no modalias at all.
>
> > > 2. When you need to know which module names and their aliases are in
> > > the kernel. This is very useful for creating an initrd image.
> > >
> > > Proposal:
> > >
> > > The proposed patch does not remove .modinfo section with module
> > > information from the vmlinux at the build time and saves it into a
> > > separate file after kernel linking. So, the kernel does not increase in
> > > size and no additional information remains in it. Information is stored
> > > in the same format as in the separate modules (null-terminated string
> > > array). Because the .modinfo section is already exported with a separate
> > > modules, we are not creating a new API.
> > >
> > > It can be easily read in the userspace:
> > >
> > > $ tr '\0' '\n' < kernel.builtin.modinfo
> > > ext4.softdep=pre: crc32c
> > > ext4.license=GPL
> > > ext4.description=Fourth Extended Filesystem
> > > ext4.author=Remy Card, Stephen Tweedie, Andrew Morton, Andreas Dilger, Theodore Ts'o and others
> > > ext4.alias=fs-ext4
> > > ext4.alias=ext3
> > > ext4.alias=fs-ext3
> > > ext4.alias=ext2
> > > ext4.alias=fs-ext2
> > > md_mod.alias=block-major-9-*
> > > md_mod.alias=md
> > > md_mod.description=MD RAID framework
> > > md_mod.license=GPL
> > > md_mod.parmtype=create_on_open:bool
> > > md_mod.parmtype=start_dirty_degraded:int
> > > ...
> > >
> > > Co-Developed-by: Gleb Fotengauer-Malinovskiy <glebfm@altlinux.org>
> > > Signed-off-by: Gleb Fotengauer-Malinovskiy <glebfm@altlinux.org>
> > > Signed-off-by: Alexey Gladkov <gladkov.alexey@gmail.com>
> > > ---
> > > Makefile | 1 +
> > > include/linux/moduleparam.h | 12 +++++-------
> > > scripts/link-vmlinux.sh | 8 ++++++++
> > > 3 files changed, 14 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/Makefile b/Makefile
> > > index d5713e7b1e50..971102194c92 100644
> > > --- a/Makefile
> > > +++ b/Makefile
> > > @@ -1288,6 +1288,7 @@ _modinst_:
> > > fi
> > > @cp -f $(objtree)/modules.order $(MODLIB)/
> > > @cp -f $(objtree)/modules.builtin $(MODLIB)/
> > > + @cp -f $(objtree)/kernel.builtin.modinfo $(MODLIB)/
This file should be cleaned up by 'make clean'.
Please add CLEAN_FILES around line 1336.
# Directories & files removed with 'make clean'
CLEAN_DIRS += $(MODVERDIR) include/ksym
CLEAN_FILES += ...
Also, shouldn't it be added to .gitignore?
> > > $(Q)$(MAKE) -f $(srctree)/scripts/Makefile.modinst
> > >
> > > # This depmod is only for convenience to give the initial
> > > diff --git a/include/linux/moduleparam.h b/include/linux/moduleparam.h
> > > index ba36506db4fb..5ba250d9172a 100644
> > > --- a/include/linux/moduleparam.h
> > > +++ b/include/linux/moduleparam.h
> > > @@ -10,23 +10,21 @@
> > > module name. */
> > > #ifdef MODULE
> > > #define MODULE_PARAM_PREFIX /* empty */
> > > +#define __MODULE_INFO_PREFIX /* empty */
> > > #else
> > > #define MODULE_PARAM_PREFIX KBUILD_MODNAME "."
> > > +/* We cannot use MODULE_PARAM_PREFIX because some modules override it. */
> > > +#define __MODULE_INFO_PREFIX KBUILD_MODNAME "."
> > > #endif
> > >
> > > /* Chosen so that structs with an unsigned long line up. */
> > > #define MAX_PARAM_PREFIX_LEN (64 - sizeof(unsigned long))
> > >
> > > -#ifdef MODULE
> > > #define __MODULE_INFO(tag, name, info) \
> > > static const char __UNIQUE_ID(name)[] \
> > > __used __attribute__((section(".modinfo"), unused, aligned(1))) \
> > > - = __stringify(tag) "=" info
> > > -#else /* !MODULE */
> > > -/* This struct is here for syntactic coherency, it is not used */
> > > -#define __MODULE_INFO(tag, name, info) \
> > > - struct __UNIQUE_ID(name) {}
> > > -#endif
> > > + = __MODULE_INFO_PREFIX __stringify(tag) "=" info
> > > +
> > > #define __MODULE_PARM_TYPE(name, _type) \
> > > __MODULE_INFO(parmtype, name##type, #name ":" _type)
> > >
> > > diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
> > > index c8cf45362bd6..399d7e4d11ec 100755
> > > --- a/scripts/link-vmlinux.sh
> > > +++ b/scripts/link-vmlinux.sh
> > > @@ -258,10 +258,12 @@ if [ -n "${CONFIG_KALLSYMS}" ]; then
> > >
> > > # step 1
> > > vmlinux_link "" .tmp_vmlinux1
> > > + "${OBJCOPY}" -R .modinfo .tmp_vmlinux1
> > > kallsyms .tmp_vmlinux1 .tmp_kallsyms1.o
> > >
> > > # step 2
> > > vmlinux_link .tmp_kallsyms1.o .tmp_vmlinux2
> > > + "${OBJCOPY}" -R .modinfo .tmp_vmlinux2
> > > kallsyms .tmp_vmlinux2 .tmp_kallsyms2.o
> > >
> > > # step 3
> > > @@ -273,6 +275,7 @@ if [ -n "${CONFIG_KALLSYMS}" ]; then
> > > kallsyms_vmlinux=.tmp_vmlinux3
> > >
> > > vmlinux_link .tmp_kallsyms2.o .tmp_vmlinux3
> > > + "${OBJCOPY}" -R .modinfo .tmp_vmlinux3
> > >
> > > kallsyms .tmp_vmlinux3 .tmp_kallsyms3.o
> > > fi
> > > @@ -281,6 +284,11 @@ fi
> > > info LD vmlinux
> > > vmlinux_link "${kallsymso}" vmlinux
> > >
> > > +info MODINFO kernel.builtin.modinfo
> > > +"${OBJCOPY}" -j .modinfo -O binary vmlinux kernel.builtin.modinfo
> > > +chmod 444 kernel.builtin.modinfo
> > > +"${OBJCOPY}" -R .modinfo vmlinux
> > > +
> > > if [ -n "${CONFIG_BUILDTIME_EXTABLE_SORT}" ]; then
> > > info SORTEX vmlinux
> > > sortextable vmlinux
As I pointed out before, I see lots of warnings for powerpc.
How about extracting modinfo before linking vmlinux?
You can extract it from vmlinux.o instead,
and discard it in the linker script.
diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
index dc0e8c5..994cd47 100755
--- a/scripts/link-vmlinux.sh
+++ b/scripts/link-vmlinux.sh
@@ -193,6 +193,9 @@ modpost_link vmlinux.o
# modpost vmlinux.o to check for section mismatches
${MAKE} -f "${srctree}/scripts/Makefile.modpost" vmlinux.o
+info MODINFO kernel.builtin.modinfo
+"${OBJCOPY}" -j .modinfo -O binary vmlinux kernel.builtin.modinfo
+
kallsymso=""
kallsyms_vmlinux=""
diff --git a/include/asm-generic/vmlinux.lds.h
b/include/asm-generic/vmlinux.lds.h
index f8f6f04..bbb9e33 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -844,6 +844,7 @@
EXIT_CALL \
*(.discard) \
*(.discard.*) \
+ *(.modinfo) \
}
/**
The code is simpler and I see no more warnings.
I also want you to check MODULE_VERSION.
If CONFIG_SYSFS is enabled, MODULE_VERSION
will not be contained in the file.
Thanks.
> > > --
> > > 2.19.2
> > >
> >
> >
> > --
> > Best Regards
> > Masahiro Yamada
> >
>
> --
> Rgrds, legion
>
--
Best Regards
Masahiro Yamada
^ permalink raw reply related
* Re: [PATCH v3 6/7] arm64: Advertise ARM64_HAS_DCPODP cpu feature
From: Andrew Murray @ 2019-04-03 9:57 UTC (permalink / raw)
To: Dave Martin
Cc: Mark Rutland, libc-alpha, Szabolcs Nagy, Catalin Marinas,
Will Deacon, Phil Blundell, linux-api, linux-arm-kernel
In-Reply-To: <20190403093242.GN3567@e103592.cambridge.arm.com>
On Wed, Apr 03, 2019 at 10:32:43AM +0100, Dave Martin wrote:
> On Wed, Apr 03, 2019 at 10:23:42AM +0100, Andrew Murray wrote:
> > On Tue, Apr 02, 2019 at 03:59:58PM +0100, Dave Martin wrote:
> > > On Mon, Apr 01, 2019 at 11:45:14AM +0100, Andrew Murray wrote:
> > > > Advertise ARM64_HAS_DCPOP when both DC CVAP and DC CVADP are supported.
> > >
> > > Do you mean ARM64_HAS_DCPODP?
> >
> > Yes I did - good catch.
> >
> > >
> > > And do we need this? This capability flag doesn't currently appear to
> > > be used for anything (which makes me wonder whether it _should_ be wired
> > > up to something in the kernel).
> > >
> > > Do we expect the kernel to do something special with this in the future?
> > >
> > > OTOH, we get a nice printk when the feature is detected, and the code
> > > size cost is insignificant. So, if there's a reasonable expectation that
> > > we will use it someday, I don't see a big problem with having it.
> >
> > Yes it's not currently used, so I'm happy to drop this patch if people prefer
> > that - however it does complement the existing DCPOP cap which is present.
>
> OK, sounds fine.
>
> Maybe add a note in the commit message that this is here for consistency
> with DCPOP, and we anticipate it being used in the future.
OK thanks,
Andrew Murray
>
> Cheers
> ---Dave
^ permalink raw reply
* Re: [PATCH v3 2/7] arm64: HWCAP: add support for AT_HWCAP2
From: Dave Martin @ 2019-04-03 9:35 UTC (permalink / raw)
To: Andrew Murray
Cc: Catalin Marinas, Will Deacon, Szabolcs Nagy, linux-arm-kernel,
Mark Rutland, Phil Blundell, libc-alpha, linux-api
In-Reply-To: <20190403092951.GJ53702@e119886-lin.cambridge.arm.com>
On Wed, Apr 03, 2019 at 10:29:52AM +0100, Andrew Murray wrote:
> On Wed, Apr 03, 2019 at 10:11:20AM +0100, Dave Martin wrote:
> > On Wed, Apr 03, 2019 at 09:32:55AM +0100, Andrew Murray wrote:
> > > On Tue, Apr 02, 2019 at 03:58:31PM +0100, Dave Martin wrote:
[...]
> > > > I wonder whether we can have a generic definition for this:
> > > >
> > > > #define cpu_set_named_feature(x) (elf_hwcap |= HWCAP_ ## x)
> > >
> > > You mean specific to arm32?
> > >
> > > I will do this, along with a cpu_get_named_feature - but I think
> > > I'd prefer to do this in a separate series.
> >
> > Yes, we could do that. We could also do it more widely, but it's
> > probably not worth the overhead of doing it.
>
> I had considered this as well but also determined that it wouldn't give
> much gain.
Yep, agreed. It can always be done later if it turns out to be useful
for someone.
Cheers
---Dave
^ permalink raw reply
* Re: [PATCH v3 6/7] arm64: Advertise ARM64_HAS_DCPODP cpu feature
From: Dave Martin @ 2019-04-03 9:32 UTC (permalink / raw)
To: Andrew Murray
Cc: Catalin Marinas, Will Deacon, Szabolcs Nagy, linux-arm-kernel,
Mark Rutland, Phil Blundell, libc-alpha, linux-api
In-Reply-To: <20190403092342.GI53702@e119886-lin.cambridge.arm.com>
On Wed, Apr 03, 2019 at 10:23:42AM +0100, Andrew Murray wrote:
> On Tue, Apr 02, 2019 at 03:59:58PM +0100, Dave Martin wrote:
> > On Mon, Apr 01, 2019 at 11:45:14AM +0100, Andrew Murray wrote:
> > > Advertise ARM64_HAS_DCPOP when both DC CVAP and DC CVADP are supported.
> >
> > Do you mean ARM64_HAS_DCPODP?
>
> Yes I did - good catch.
>
> >
> > And do we need this? This capability flag doesn't currently appear to
> > be used for anything (which makes me wonder whether it _should_ be wired
> > up to something in the kernel).
> >
> > Do we expect the kernel to do something special with this in the future?
> >
> > OTOH, we get a nice printk when the feature is detected, and the code
> > size cost is insignificant. So, if there's a reasonable expectation that
> > we will use it someday, I don't see a big problem with having it.
>
> Yes it's not currently used, so I'm happy to drop this patch if people prefer
> that - however it does complement the existing DCPOP cap which is present.
OK, sounds fine.
Maybe add a note in the commit message that this is here for consistency
with DCPOP, and we anticipate it being used in the future.
Cheers
---Dave
^ permalink raw reply
* Re: [PATCH v3 2/7] arm64: HWCAP: add support for AT_HWCAP2
From: Andrew Murray @ 2019-04-03 9:29 UTC (permalink / raw)
To: Dave Martin
Cc: Catalin Marinas, Will Deacon, Szabolcs Nagy, linux-arm-kernel,
Mark Rutland, Phil Blundell, libc-alpha, linux-api
In-Reply-To: <20190403091118.GL3567@e103592.cambridge.arm.com>
On Wed, Apr 03, 2019 at 10:11:20AM +0100, Dave Martin wrote:
> On Wed, Apr 03, 2019 at 09:32:55AM +0100, Andrew Murray wrote:
> > On Tue, Apr 02, 2019 at 03:58:31PM +0100, Dave Martin wrote:
> > > On Mon, Apr 01, 2019 at 11:45:10AM +0100, Andrew Murray wrote:
>
> [...]
>
> > > > diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
> > > > index aa4ec53281ce..6cc8aff83805 100644
> > > > --- a/drivers/clocksource/arm_arch_timer.c
> > > > +++ b/drivers/clocksource/arm_arch_timer.c
> > > > @@ -833,7 +833,11 @@ static void arch_timer_evtstrm_enable(int divider)
> > > > cntkctl |= (divider << ARCH_TIMER_EVT_TRIGGER_SHIFT)
> > > > | ARCH_TIMER_VIRT_EVT_EN;
> > > > arch_timer_set_cntkctl(cntkctl);
> > > > +#ifdef CONFIG_ARM64
> > > > + cpu_set_named_feature(EVTSTRM);
> > > > +#else
> > > > elf_hwcap |= HWCAP_EVTSTRM;
> > > > +#endif
> > >
> > > I wonder whether we can have a generic definition for this:
> > >
> > > #define cpu_set_named_feature(x) (elf_hwcap |= HWCAP_ ## x)
> >
> > You mean specific to arm32?
> >
> > I will do this, along with a cpu_get_named_feature - but I think I'd prefer
> > to do this in a separate series.
>
> Yes, we could do that. We could also do it more widely, but it's
> probably not worth the overhead of doing it.
I had considered this as well but also determined that it wouldn't give
much gain.
Thanks,
Andrew Murray
>
> [...]
>
> Cheers
> ---Dave
^ permalink raw reply
* Re: [PATCH v3 6/7] arm64: Advertise ARM64_HAS_DCPODP cpu feature
From: Andrew Murray @ 2019-04-03 9:23 UTC (permalink / raw)
To: Dave Martin
Cc: Mark Rutland, libc-alpha, Szabolcs Nagy, Catalin Marinas,
Will Deacon, Phil Blundell, linux-api, linux-arm-kernel
In-Reply-To: <20190402145958.GJ3567@e103592.cambridge.arm.com>
On Tue, Apr 02, 2019 at 03:59:58PM +0100, Dave Martin wrote:
> On Mon, Apr 01, 2019 at 11:45:14AM +0100, Andrew Murray wrote:
> > Advertise ARM64_HAS_DCPOP when both DC CVAP and DC CVADP are supported.
>
> Do you mean ARM64_HAS_DCPODP?
Yes I did - good catch.
>
> And do we need this? This capability flag doesn't currently appear to
> be used for anything (which makes me wonder whether it _should_ be wired
> up to something in the kernel).
>
> Do we expect the kernel to do something special with this in the future?
>
> OTOH, we get a nice printk when the feature is detected, and the code
> size cost is insignificant. So, if there's a reasonable expectation that
> we will use it someday, I don't see a big problem with having it.
Yes it's not currently used, so I'm happy to drop this patch if people prefer
that - however it does complement the existing DCPOP cap which is present.
Thanks,
Andrew Murray
>
> Cheers
> ---Dave
>
> >
> > Signed-off-by: Andrew Murray <andrew.murray@arm.com>
> > ---
> > arch/arm64/include/asm/cpucaps.h | 3 ++-
> > arch/arm64/kernel/cpufeature.c | 9 +++++++++
> > 2 files changed, 11 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/arm64/include/asm/cpucaps.h b/arch/arm64/include/asm/cpucaps.h
> > index f6a76e43f39e..defdc67d9ab4 100644
> > --- a/arch/arm64/include/asm/cpucaps.h
> > +++ b/arch/arm64/include/asm/cpucaps.h
> > @@ -61,7 +61,8 @@
> > #define ARM64_HAS_GENERIC_AUTH_ARCH 40
> > #define ARM64_HAS_GENERIC_AUTH_IMP_DEF 41
> > #define ARM64_HAS_IRQ_PRIO_MASKING 42
> > +#define ARM64_HAS_DCPODP 43
> >
> > -#define ARM64_NCAPS 43
> > +#define ARM64_NCAPS 44
> >
> > #endif /* __ASM_CPUCAPS_H */
> > diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
> > index 5e27d2dbe45e..c74b25895c43 100644
> > --- a/arch/arm64/kernel/cpufeature.c
> > +++ b/arch/arm64/kernel/cpufeature.c
> > @@ -1339,6 +1339,15 @@ static const struct arm64_cpu_capabilities arm64_features[] = {
> > .field_pos = ID_AA64ISAR1_DPB_SHIFT,
> > .min_field_value = 1,
> > },
> > + {
> > + .desc = "Data cache clean to Point of Deep Persistence",
> > + .capability = ARM64_HAS_DCPODP,
> > + .type = ARM64_CPUCAP_SYSTEM_FEATURE,
> > + .matches = has_cpuid_feature,
> > + .sys_reg = SYS_ID_AA64ISAR1_EL1,
> > + .field_pos = ID_AA64ISAR1_DPB_SHIFT,
> > + .min_field_value = 2,
> > + },
> > #endif
> > #ifdef CONFIG_ARM64_SVE
> > {
> > --
> > 2.21.0
> >
^ permalink raw reply
* Re: [PATCH v3 3/7] arm64: HWCAP: encapsulate elf_hwcap
From: Dave Martin @ 2019-04-03 9:13 UTC (permalink / raw)
To: Andrew Murray
Cc: Suzuki K Poulose, mark.rutland, libc-alpha, Szabolcs.Nagy,
catalin.marinas, will.deacon, pb, linux-api, linux-arm-kernel
In-Reply-To: <20190403085326.GH53702@e119886-lin.cambridge.arm.com>
On Wed, Apr 03, 2019 at 09:53:26AM +0100, Andrew Murray wrote:
> On Tue, Apr 02, 2019 at 04:55:58PM +0100, Dave Martin wrote:
> > On Tue, Apr 02, 2019 at 04:32:57PM +0100, Suzuki K Poulose wrote:
[...]
> > > nit:
> > >
> > > As mentioned above we have "cpu_hwcaps" for the features only internally
> > > by the kernel. Naming it "kernel_hwcap" kind of looses the hint that the
> > > major purpose is for userspace consumption and could easily confuse with
> > > the poorly named "cpu_hwcaps" which should have been called kernel_hwcaps.
> > >
> > > How about "user_hwcaps" ? Or preferrably something closer to that.
> >
> > Yes, that may be better.
> >
> > Of course, we also have this naming in all the KERNEL_HWCAP #defined now.
> >
> > Since kernel_hwcap is just a static variable now, maybe it's sufficient
> > to stick a comment next to it explaining what it is (and what it isn't).
> > "user_hwcaps" still implies that this might be the userspace view of the
> > flags, which it isn't.
> >
> > But I don't feel strongly about this. If someone wants to make a
> > decision, I'm happy to defer to it.
>
> I think changing the name will cause more confusion - there isn't an obvious
> name for it and needing a comment to explain it hints that this may not be
> the best approach. As it's a static variable with only 4 uses in the same
> file it should be pretty clear to anyone interested. Also keeping the same
> name will help users find it and understand how it has changed if they
> incorrectly attempt to use it by setting/testing bits on it.
>
> Afterall the elf_hwcap variable does still hold the elf_hwcap bits and it's
> obtained by cpu_get_elf_hwcap. The naming of KERNEL_HWCAP also makes sense
> in this context.
>
> Perhaps a better name would be something like elf_hwcaps implying that there
> is some mapping required (though this would only last until we run out of
> space in it and need another one).
>
> Shall we stick with what we have?
I'm happy enough with what you propose: I agree, there's not an
obviously a better name, and now that this is local, the scope for
confusion is lessened. So, add a comment, but keep whetever name you're
happy with.
Cheers
---Dave
^ permalink raw reply
* Re: [PATCH v3 2/7] arm64: HWCAP: add support for AT_HWCAP2
From: Dave Martin @ 2019-04-03 9:11 UTC (permalink / raw)
To: Andrew Murray
Cc: Catalin Marinas, Will Deacon, Szabolcs Nagy, linux-arm-kernel,
Mark Rutland, Phil Blundell, libc-alpha, linux-api
In-Reply-To: <20190403083255.GG53702@e119886-lin.cambridge.arm.com>
On Wed, Apr 03, 2019 at 09:32:55AM +0100, Andrew Murray wrote:
> On Tue, Apr 02, 2019 at 03:58:31PM +0100, Dave Martin wrote:
> > On Mon, Apr 01, 2019 at 11:45:10AM +0100, Andrew Murray wrote:
[...]
> > > diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
> > > index aa4ec53281ce..6cc8aff83805 100644
> > > --- a/drivers/clocksource/arm_arch_timer.c
> > > +++ b/drivers/clocksource/arm_arch_timer.c
> > > @@ -833,7 +833,11 @@ static void arch_timer_evtstrm_enable(int divider)
> > > cntkctl |= (divider << ARCH_TIMER_EVT_TRIGGER_SHIFT)
> > > | ARCH_TIMER_VIRT_EVT_EN;
> > > arch_timer_set_cntkctl(cntkctl);
> > > +#ifdef CONFIG_ARM64
> > > + cpu_set_named_feature(EVTSTRM);
> > > +#else
> > > elf_hwcap |= HWCAP_EVTSTRM;
> > > +#endif
> >
> > I wonder whether we can have a generic definition for this:
> >
> > #define cpu_set_named_feature(x) (elf_hwcap |= HWCAP_ ## x)
>
> You mean specific to arm32?
>
> I will do this, along with a cpu_get_named_feature - but I think I'd prefer
> to do this in a separate series.
Yes, we could do that. We could also do it more widely, but it's
probably not worth the overhead of doing it.
[...]
Cheers
---Dave
^ permalink raw reply
* Re: [PATCH v3 3/7] arm64: HWCAP: encapsulate elf_hwcap
From: Andrew Murray @ 2019-04-03 8:53 UTC (permalink / raw)
To: Dave Martin
Cc: Suzuki K Poulose, mark.rutland, libc-alpha, Szabolcs.Nagy,
catalin.marinas, will.deacon, pb, linux-api, linux-arm-kernel
In-Reply-To: <20190402155558.GK3567@e103592.cambridge.arm.com>
On Tue, Apr 02, 2019 at 04:55:58PM +0100, Dave Martin wrote:
> On Tue, Apr 02, 2019 at 04:32:57PM +0100, Suzuki K Poulose wrote:
> > Hi,
> >
> > On 02/04/2019 16:06, Andrew Murray wrote:
> > >On Tue, Apr 02, 2019 at 03:58:21PM +0100, Dave Martin wrote:
> > >>On Mon, Apr 01, 2019 at 11:45:11AM +0100, Andrew Murray wrote:
> > >>>The introduction of AT_HWCAP2 introduced accessors which ensure that
> > >>>hwcap features are set and tested appropriately.
> > >>>
> > >>>Let's now mandate access to elf_hwcap via these accessors by making
> > >>>elf_hwcap static within cpufeature.c.
> > >>
> > >>Looks reasonable except for a couple of minor nits below.
> > >>
> > >>I had wondered whether putting these accessors out of line would affect
> > >>any hot paths, but I can't see these used from anything that looks like
> > >>a hot path. So we're probably fine.
> > >>
> > >>cpus_have_const_cap() is preferred for places where this matters,
> > >>anyway.
> >
> > Btw, thats for cpu_hwcaps, which is completely different from elf_hwcaps.
>
> Sure, I meant: where we need to test something fast, we can have a
> cpucap for it, rather than rely on the ELF hwcaps.
>
> In practice, we already follow this pattern today: ELF hwcap flags are
> tested in a few places, but I don't see anything that is fast-path.
>
> >
> > >>
> > >>[...]
> > >>
> > >>>diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
> > >>>index 986ceeacd19f..84ca52fa75e5 100644
> > >>>--- a/arch/arm64/kernel/cpufeature.c
> > >>>+++ b/arch/arm64/kernel/cpufeature.c
> > >>>@@ -35,8 +35,7 @@
> > >>> #include <asm/traps.h>
> > >>> #include <asm/virt.h>
> > >>>-unsigned long elf_hwcap __read_mostly;
> > >>>-EXPORT_SYMBOL_GPL(elf_hwcap);
> > >>>+static unsigned long elf_hwcap __read_mostly;
> > >>
> > >>Now that this doesn't correspond directly to ELF_HWCAP any more and we
> > >>hide it, can we rename it to avoid confusion?
> > >>
> > >>Maybe "kernel_hwcap"?
> > >
> > >Yes this seems reasonable.
> >
> > nit:
> >
> > As mentioned above we have "cpu_hwcaps" for the features only internally
> > by the kernel. Naming it "kernel_hwcap" kind of looses the hint that the
> > major purpose is for userspace consumption and could easily confuse with
> > the poorly named "cpu_hwcaps" which should have been called kernel_hwcaps.
> >
> > How about "user_hwcaps" ? Or preferrably something closer to that.
>
> Yes, that may be better.
>
> Of course, we also have this naming in all the KERNEL_HWCAP #defined now.
>
> Since kernel_hwcap is just a static variable now, maybe it's sufficient
> to stick a comment next to it explaining what it is (and what it isn't).
> "user_hwcaps" still implies that this might be the userspace view of the
> flags, which it isn't.
>
> But I don't feel strongly about this. If someone wants to make a
> decision, I'm happy to defer to it.
I think changing the name will cause more confusion - there isn't an obvious
name for it and needing a comment to explain it hints that this may not be
the best approach. As it's a static variable with only 4 uses in the same
file it should be pretty clear to anyone interested. Also keeping the same
name will help users find it and understand how it has changed if they
incorrectly attempt to use it by setting/testing bits on it.
Afterall the elf_hwcap variable does still hold the elf_hwcap bits and it's
obtained by cpu_get_elf_hwcap. The naming of KERNEL_HWCAP also makes sense
in this context.
Perhaps a better name would be something like elf_hwcaps implying that there
is some mapping required (though this would only last until we run out of
space in it and need another one).
Shall we stick with what we have?
Thanks,
Andrew Murray
>
> Cheers
> ---Dave
^ permalink raw reply
* Re: [PATCH v3 2/7] arm64: HWCAP: add support for AT_HWCAP2
From: Andrew Murray @ 2019-04-03 8:32 UTC (permalink / raw)
To: Dave Martin
Cc: Catalin Marinas, Will Deacon, Szabolcs Nagy, linux-arm-kernel,
Mark Rutland, Phil Blundell, libc-alpha, linux-api
In-Reply-To: <20190402145831.GI3567@e103592.cambridge.arm.com>
On Tue, Apr 02, 2019 at 03:58:31PM +0100, Dave Martin wrote:
> On Mon, Apr 01, 2019 at 11:45:10AM +0100, Andrew Murray wrote:
> > As we will exhaust the first 32 bits of AT_HWCAP let's start
> > exposing AT_HWCAP2 to userspace to give us up to 64 caps.
> >
> > Whilst it's possible to use the remaining 32 bits of AT_HWCAP, we
> > prefer to expand into AT_HWCAP2 in order to provide a consistent
> > view to userspace between ILP32 and LP64. However internal to the
> > kernel we prefer to continue to use the full space of elf_hwcap.
> >
> > To reduce complexity and allow for future expansion, we now
> > represent hwcaps in the kernel as ordinals and use a
> > KERNEL_HWCAP_ prefix. This allows us to support automatic feature
> > based module loading for all our hwcaps.
> >
> > We introduce cpu_set_feature to set hwcaps which compliments the
>
> Nit: maybe "complements"? (I've always been a bit fuzzy on the precise
> distinction, though.)
Yes this is the correct spelling (as I'm pretty sure the cpu_have_feature
helper doesn't have a tips jar).
>
> > existing cpu_have_feature helper. These helpers allow us to clean
> > up existing direct uses of elf_hwcap and reduce any future effort
> > required to move beyond 64 caps.
> >
> > For convenience we also introduce cpu_{have,set}_named_feature which
> > makes use of the cpu_feature macro to allow providing a hwcap name
> > without a {KERNEL_}HWCAP_ prefix.
> >
> > Signed-off-by: Andrew Murray <andrew.murray@arm.com>
> > ---
> > arch/arm64/crypto/aes-ce-ccm-glue.c | 2 +-
> > arch/arm64/crypto/aes-neonbs-glue.c | 2 +-
> > arch/arm64/crypto/chacha-neon-glue.c | 2 +-
> > arch/arm64/crypto/crct10dif-ce-glue.c | 4 +-
> > arch/arm64/crypto/ghash-ce-glue.c | 8 +--
> > arch/arm64/crypto/nhpoly1305-neon-glue.c | 2 +-
> > arch/arm64/crypto/sha256-glue.c | 4 +-
> > arch/arm64/include/asm/cpufeature.h | 22 ++++----
> > arch/arm64/include/asm/hwcap.h | 49 +++++++++++++++++-
> > arch/arm64/include/uapi/asm/hwcap.h | 2 +-
> > arch/arm64/kernel/cpufeature.c | 66 ++++++++++++------------
> > arch/arm64/kernel/cpuinfo.c | 2 +-
> > arch/arm64/kernel/fpsimd.c | 4 +-
> > drivers/clocksource/arm_arch_timer.c | 8 +++
> > 14 files changed, 117 insertions(+), 60 deletions(-)
> >
>
> [...]
>
> > diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h
> > index e505e1fbd2b9..f06e1da1d678 100644
> > --- a/arch/arm64/include/asm/cpufeature.h
> > +++ b/arch/arm64/include/asm/cpufeature.h
> > @@ -14,15 +14,8 @@
> > #include <asm/hwcap.h>
> > #include <asm/sysreg.h>
> >
> > -/*
> > - * In the arm64 world (as in the ARM world), elf_hwcap is used both internally
> > - * in the kernel and for user space to keep track of which optional features
> > - * are supported by the current system. So let's map feature 'x' to HWCAP_x.
> > - * Note that HWCAP_x constants are bit fields so we need to take the log.
> > - */
> > -
> > -#define MAX_CPU_FEATURES (8 * sizeof(elf_hwcap))
> > -#define cpu_feature(x) ilog2(HWCAP_ ## x)
> > +#define MAX_CPU_FEATURES 64
> > +#define cpu_feature(x) (KERNEL_HWCAP_ ## x)
>
> Nit: do we need the () here? They may be defensive, but I'm not sure
> they're required.
I guess not, checkpatch doesn't complain - I'll remove them.
>
> [...]
>
> > diff --git a/arch/arm64/include/asm/hwcap.h b/arch/arm64/include/asm/hwcap.h
> > index 400b80b49595..d21fe3314d90 100644
> > --- a/arch/arm64/include/asm/hwcap.h
> > +++ b/arch/arm64/include/asm/hwcap.h
> > @@ -39,12 +39,59 @@
> > #define COMPAT_HWCAP2_SHA2 (1 << 3)
> > #define COMPAT_HWCAP2_CRC32 (1 << 4)
> >
> > +/*
> > + * For userspace we represent hwcaps as a collection of HWCAP{,2}_x bitfields
> > + * as described in uapi/asm/hwcap.h. For the kernel we represent hwcaps as
> > + * natural numbers (in a single range of size MAX_CPU_FEATURES) defined here
> > + * with prefix KERNEL_HWCAP_ mapped to their HWCAP{,2}_x counterpart.
> > + *
> > + * Hwcaps should be set and tested within the kernel via the
> > + * cpu_{set,have}_named_feature(feature) where feature is the unique suffix
> > + * of KERNEL_HWCAP_{feature}.
> > + */
> > +#define KERNEL_HWCAP_FP ilog2(HWCAP_FP)
> > +#define KERNEL_HWCAP_ASIMD ilog2(HWCAP_ASIMD)
> > +#define KERNEL_HWCAP_EVTSTRM ilog2(HWCAP_EVTSTRM)
> > +#define KERNEL_HWCAP_AES ilog2(HWCAP_AES)
> > +#define KERNEL_HWCAP_PMULL ilog2(HWCAP_PMULL)
> > +#define KERNEL_HWCAP_SHA1 ilog2(HWCAP_SHA1)
> > +#define KERNEL_HWCAP_SHA2 ilog2(HWCAP_SHA2)
> > +#define KERNEL_HWCAP_CRC32 ilog2(HWCAP_CRC32)
> > +#define KERNEL_HWCAP_ATOMICS ilog2(HWCAP_ATOMICS)
> > +#define KERNEL_HWCAP_FPHP ilog2(HWCAP_FPHP)
> > +#define KERNEL_HWCAP_ASIMDHP ilog2(HWCAP_ASIMDHP)
> > +#define KERNEL_HWCAP_CPUID ilog2(HWCAP_CPUID)
> > +#define KERNEL_HWCAP_ASIMDRDM ilog2(HWCAP_ASIMDRDM)
> > +#define KERNEL_HWCAP_JSCVT ilog2(HWCAP_JSCVT)
> > +#define KERNEL_HWCAP_FCMA ilog2(HWCAP_FCMA)
> > +#define KERNEL_HWCAP_LRCPC ilog2(HWCAP_LRCPC)
> > +#define KERNEL_HWCAP_DCPOP ilog2(HWCAP_DCPOP)
> > +#define KERNEL_HWCAP_SHA3 ilog2(HWCAP_SHA3)
> > +#define KERNEL_HWCAP_SM3 ilog2(HWCAP_SM3)
> > +#define KERNEL_HWCAP_SM4 ilog2(HWCAP_SM4)
> > +#define KERNEL_HWCAP_ASIMDDP ilog2(HWCAP_ASIMDDP)
> > +#define KERNEL_HWCAP_SHA512 ilog2(HWCAP_SHA512)
> > +#define KERNEL_HWCAP_SVE ilog2(HWCAP_SVE)
> > +#define KERNEL_HWCAP_ASIMDFHM ilog2(HWCAP_ASIMDFHM)
> > +#define KERNEL_HWCAP_DIT ilog2(HWCAP_DIT)
> > +#define KERNEL_HWCAP_USCAT ilog2(HWCAP_USCAT)
> > +#define KERNEL_HWCAP_ILRCPC ilog2(HWCAP_ILRCPC)
> > +#define KERNEL_HWCAP_FLAGM ilog2(HWCAP_FLAGM)
> > +#define KERNEL_HWCAP_SSBS ilog2(HWCAP_SSBS)
> > +#define KERNEL_HWCAP_SB ilog2(HWCAP_SB)
> > +#define KERNEL_HWCAP_PACA ilog2(HWCAP_PACA)
> > +#define KERNEL_HWCAP_PACG ilog2(HWCAP_PACG)
> > +#define KERNEL_HWCAP_DCPODP (ilog2(HWCAP2_DCPODP) + 32)
>
> Nit: can we wrap this so that the "+ 32" doesn't have to be spelled out
> each time?
>
> If we are splitting ths CVADP support from this patch, then dropping
> such a wrapper macro here (maybe with a comment) will serve as a
> placeholder for whichever patch wins the race for the first HWCAP2
> flag.
>
> Say
>
> #define __khwcap2_feature(x) (ilog2(HWCAP2_ ## xx) + 32)
>
> (Optionally, we could also have __khwcap_feature() too so that
> everything looks nice and regular.)
Yes this makes sense, thanks for the suggestion.
>
> [...]
>
> > diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
> > index aa4ec53281ce..6cc8aff83805 100644
> > --- a/drivers/clocksource/arm_arch_timer.c
> > +++ b/drivers/clocksource/arm_arch_timer.c
> > @@ -833,7 +833,11 @@ static void arch_timer_evtstrm_enable(int divider)
> > cntkctl |= (divider << ARCH_TIMER_EVT_TRIGGER_SHIFT)
> > | ARCH_TIMER_VIRT_EVT_EN;
> > arch_timer_set_cntkctl(cntkctl);
> > +#ifdef CONFIG_ARM64
> > + cpu_set_named_feature(EVTSTRM);
> > +#else
> > elf_hwcap |= HWCAP_EVTSTRM;
> > +#endif
>
> I wonder whether we can have a generic definition for this:
>
> #define cpu_set_named_feature(x) (elf_hwcap |= HWCAP_ ## x)
You mean specific to arm32?
I will do this, along with a cpu_get_named_feature - but I think I'd prefer
to do this in a separate series.
>
> seems a reasonable fallback when the arch doesn't provide its own
> version.
>
>
> Although we don't have many instances, it would still be nice to avoid
> ifdeffery creeping in.
>
> [...]
>
> We can probably pull the Documentation/arm64/elf_hwcaps.txt changes into
> this patch.
>
> It probably makes sense to pull the Documentation/arm64/elf_hwcaps.txt
> updates alongside this patch in the series (or even incorporate them
> into this patch, since they're not huge.)
Yes that's OK.
>
> Other than that, looks reasonable to me.
Thanks,
Andrew Murray
>
> Cheers
> ---Dave
^ permalink raw reply
* Re: [PATCH v3 3/7] arm64: HWCAP: encapsulate elf_hwcap
From: Dave Martin @ 2019-04-02 15:55 UTC (permalink / raw)
To: Suzuki K Poulose
Cc: andrew.murray, mark.rutland, libc-alpha, Szabolcs.Nagy,
catalin.marinas, will.deacon, pb, linux-api, linux-arm-kernel
In-Reply-To: <8f47d948-e5cd-68ab-b340-d40d3b11a3f5@arm.com>
On Tue, Apr 02, 2019 at 04:32:57PM +0100, Suzuki K Poulose wrote:
> Hi,
>
> On 02/04/2019 16:06, Andrew Murray wrote:
> >On Tue, Apr 02, 2019 at 03:58:21PM +0100, Dave Martin wrote:
> >>On Mon, Apr 01, 2019 at 11:45:11AM +0100, Andrew Murray wrote:
> >>>The introduction of AT_HWCAP2 introduced accessors which ensure that
> >>>hwcap features are set and tested appropriately.
> >>>
> >>>Let's now mandate access to elf_hwcap via these accessors by making
> >>>elf_hwcap static within cpufeature.c.
> >>
> >>Looks reasonable except for a couple of minor nits below.
> >>
> >>I had wondered whether putting these accessors out of line would affect
> >>any hot paths, but I can't see these used from anything that looks like
> >>a hot path. So we're probably fine.
> >>
> >>cpus_have_const_cap() is preferred for places where this matters,
> >>anyway.
>
> Btw, thats for cpu_hwcaps, which is completely different from elf_hwcaps.
Sure, I meant: where we need to test something fast, we can have a
cpucap for it, rather than rely on the ELF hwcaps.
In practice, we already follow this pattern today: ELF hwcap flags are
tested in a few places, but I don't see anything that is fast-path.
>
> >>
> >>[...]
> >>
> >>>diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
> >>>index 986ceeacd19f..84ca52fa75e5 100644
> >>>--- a/arch/arm64/kernel/cpufeature.c
> >>>+++ b/arch/arm64/kernel/cpufeature.c
> >>>@@ -35,8 +35,7 @@
> >>> #include <asm/traps.h>
> >>> #include <asm/virt.h>
> >>>-unsigned long elf_hwcap __read_mostly;
> >>>-EXPORT_SYMBOL_GPL(elf_hwcap);
> >>>+static unsigned long elf_hwcap __read_mostly;
> >>
> >>Now that this doesn't correspond directly to ELF_HWCAP any more and we
> >>hide it, can we rename it to avoid confusion?
> >>
> >>Maybe "kernel_hwcap"?
> >
> >Yes this seems reasonable.
>
> nit:
>
> As mentioned above we have "cpu_hwcaps" for the features only internally
> by the kernel. Naming it "kernel_hwcap" kind of looses the hint that the
> major purpose is for userspace consumption and could easily confuse with
> the poorly named "cpu_hwcaps" which should have been called kernel_hwcaps.
>
> How about "user_hwcaps" ? Or preferrably something closer to that.
Yes, that may be better.
Of course, we also have this naming in all the KERNEL_HWCAP #defined now.
Since kernel_hwcap is just a static variable now, maybe it's sufficient
to stick a comment next to it explaining what it is (and what it isn't).
"user_hwcaps" still implies that this might be the userspace view of the
flags, which it isn't.
But I don't feel strongly about this. If someone wants to make a
decision, I'm happy to defer to it.
Cheers
---Dave
^ permalink raw reply
* Re: [PATCH 10/17] fpga: dfl: afu: export __port_enable/disable function.
From: Moritz Fischer @ 2019-04-02 15:50 UTC (permalink / raw)
To: Wu Hao; +Cc: atull, mdf, linux-fpga, linux-kernel, linux-api, Xu Yilun
In-Reply-To: <1553483264-5379-11-git-send-email-hao.wu@intel.com>
On Mon, Mar 25, 2019 at 11:07:37AM +0800, Wu Hao wrote:
> As these two functions are used by other private features. e.g.
> in error reporting private feature, it requires to check port status
> and reset port for error clearing.
>
> Signed-off-by: Xu Yilun <yilun.xu@intel.com>
> Signed-off-by: Wu Hao <hao.wu@intel.com>
Acked-by: Moritz Fischer <mdf@kernel.org>
> ---
> drivers/fpga/dfl-afu-main.c | 25 ++++++++++++++-----------
> drivers/fpga/dfl-afu.h | 3 +++
> 2 files changed, 17 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/fpga/dfl-afu-main.c b/drivers/fpga/dfl-afu-main.c
> index 2916876..e727d9b 100644
> --- a/drivers/fpga/dfl-afu-main.c
> +++ b/drivers/fpga/dfl-afu-main.c
> @@ -24,14 +24,16 @@
> #define DRV_VERSION "0.8"
>
> /**
> - * port_enable - enable a port
> + * __port_enable - enable a port
> * @pdev: port platform device.
> *
> * Enable Port by clear the port soft reset bit, which is set by default.
> * The AFU is unable to respond to any MMIO access while in reset.
> - * port_enable function should only be used after port_disable function.
> + * __port_enable function should only be used after __port_disable function.
> + *
> + * The caller needs to hold lock for protection.
> */
> -static void port_enable(struct platform_device *pdev)
> +void __port_enable(struct platform_device *pdev)
> {
> struct dfl_feature_platform_data *pdata = dev_get_platdata(&pdev->dev);
> void __iomem *base;
> @@ -54,13 +56,14 @@ static void port_enable(struct platform_device *pdev)
> #define RST_POLL_TIMEOUT 1000 /* us */
>
> /**
> - * port_disable - disable a port
> + * __port_disable - disable a port
> * @pdev: port platform device.
> *
> - * Disable Port by setting the port soft reset bit, it puts the port into
> - * reset.
> + * Disable Port by setting the port soft reset bit, it puts the port into reset.
> + *
> + * The caller needs to hold lock for protection.
> */
> -static int port_disable(struct platform_device *pdev)
> +int __port_disable(struct platform_device *pdev)
> {
> struct dfl_feature_platform_data *pdata = dev_get_platdata(&pdev->dev);
> void __iomem *base;
> @@ -106,9 +109,9 @@ static int __port_reset(struct platform_device *pdev)
> {
> int ret;
>
> - ret = port_disable(pdev);
> + ret = __port_disable(pdev);
> if (!ret)
> - port_enable(pdev);
> + __port_enable(pdev);
>
> return ret;
> }
> @@ -810,9 +813,9 @@ static int port_enable_set(struct platform_device *pdev, bool enable)
>
> mutex_lock(&pdata->lock);
> if (enable)
> - port_enable(pdev);
> + __port_enable(pdev);
> else
> - ret = port_disable(pdev);
> + ret = __port_disable(pdev);
> mutex_unlock(&pdata->lock);
>
> return ret;
> diff --git a/drivers/fpga/dfl-afu.h b/drivers/fpga/dfl-afu.h
> index 0c7630a..35e60c5 100644
> --- a/drivers/fpga/dfl-afu.h
> +++ b/drivers/fpga/dfl-afu.h
> @@ -79,6 +79,9 @@ struct dfl_afu {
> struct dfl_feature_platform_data *pdata;
> };
>
> +void __port_enable(struct platform_device *pdev);
> +int __port_disable(struct platform_device *pdev);
> +
> void afu_mmio_region_init(struct dfl_feature_platform_data *pdata);
> int afu_mmio_region_add(struct dfl_feature_platform_data *pdata,
> u32 region_index, u64 region_size, u64 phys, u32 flags);
> --
> 2.7.4
>
^ permalink raw reply
* [PATCH v4 17/17] fscrypt: document the new ioctls and policy version
From: Eric Biggers @ 2019-04-02 15:46 UTC (permalink / raw)
To: linux-fscrypt
Cc: Satya Tangirala, linux-api, linux-f2fs-devel, keyrings, linux-mtd,
linux-crypto, linux-fsdevel, linux-ext4, Paul Crowley
In-Reply-To: <20190402154600.32432-1-ebiggers@kernel.org>
From: Eric Biggers <ebiggers@google.com>
Update the fscrypt documentation file to catch up to all the latest
changes, including the new ioctls to manage master encryption keys in
the filesystem-level keyring and the support for v2 encryption policies.
Signed-off-by: Eric Biggers <ebiggers@google.com>
---
Documentation/filesystems/fscrypt.rst | 648 +++++++++++++++++++++-----
1 file changed, 532 insertions(+), 116 deletions(-)
diff --git a/Documentation/filesystems/fscrypt.rst b/Documentation/filesystems/fscrypt.rst
index 4f9df6ba669e5..c480bd4d9a47c 100644
--- a/Documentation/filesystems/fscrypt.rst
+++ b/Documentation/filesystems/fscrypt.rst
@@ -72,6 +72,9 @@ Online attacks
fscrypt (and storage encryption in general) can only provide limited
protection, if any at all, against online attacks. In detail:
+Side-channel attacks
+~~~~~~~~~~~~~~~~~~~~
+
fscrypt is only resistant to side-channel attacks, such as timing or
electromagnetic attacks, to the extent that the underlying Linux
Cryptographic API algorithms are. If a vulnerable algorithm is used,
@@ -80,29 +83,84 @@ attacker to mount a side channel attack against the online system.
Side channel attacks may also be mounted against applications
consuming decrypted data.
-After an encryption key has been provided, fscrypt is not designed to
-hide the plaintext file contents or filenames from other users on the
-same system, regardless of the visibility of the keyring key.
-Instead, existing access control mechanisms such as file mode bits,
-POSIX ACLs, LSMs, or mount namespaces should be used for this purpose.
-Also note that as long as the encryption keys are *anywhere* in
-memory, an online attacker can necessarily compromise them by mounting
-a physical attack or by exploiting any kernel security vulnerability
-which provides an arbitrary memory read primitive.
-
-While it is ostensibly possible to "evict" keys from the system,
-recently accessed encrypted files will remain accessible at least
-until the filesystem is unmounted or the VFS caches are dropped, e.g.
-using ``echo 2 > /proc/sys/vm/drop_caches``. Even after that, if the
-RAM is compromised before being powered off, it will likely still be
-possible to recover portions of the plaintext file contents, if not
-some of the encryption keys as well. (Since Linux v4.12, all
-in-kernel keys related to fscrypt are sanitized before being freed.
-However, userspace would need to do its part as well.)
-
-Currently, fscrypt does not prevent a user from maliciously providing
-an incorrect key for another user's existing encrypted files. A
-protection against this is planned.
+Unauthorized file access
+~~~~~~~~~~~~~~~~~~~~~~~~
+
+After an encryption key has been added, fscrypt does not hide the
+plaintext file contents or filenames from other users on the same
+system. Instead, existing access control mechanisms such as file mode
+bits, POSIX ACLs, LSMs, or namespaces should be used for this purpose.
+
+(For the reasoning behind this, understand that while the key is
+added, the confidentiality of the data, from the perspective of the
+system itself, is *not* protected by the mathematical properties of
+encryption but rather only by the correctness of the kernel.
+Therefore, any encryption-specific access control checks would merely
+be enforced by kernel *code* and therefore would be largely redundant
+with the wide variety of access control mechanisms already available.)
+
+Kernel memory compromise
+~~~~~~~~~~~~~~~~~~~~~~~~
+
+An attacker who compromises the system enough to read from arbitrary
+memory, e.g. by mounting a physical attack or by exploiting a kernel
+security vulnerability, can compromise all encryption keys that are
+currently in use.
+
+However, fscrypt allows encryption keys to be removed from the kernel,
+which may protect them from later compromise.
+
+In more detail, the FS_IOC_REMOVE_ENCRYPTION_KEY ioctl will wipe a
+master encryption key from kernel memory. Moreover, it will try to
+evict all cached inodes which had been "unlocked" using the key,
+thereby wiping their per-file keys and making them once again appear
+"locked", i.e. in ciphertext or encrypted form.
+
+However, FS_IOC_REMOVE_ENCRYPTION_KEY has some limitations:
+
+- Per-file keys for in-use files will *not* be removed or wiped.
+ Therefore, for maximum effect, userspace should close the relevant
+ encrypted files and directories before removing a master key, as
+ well as kill any processes whose working directory is in an affected
+ encrypted directory.
+
+- The kernel cannot magically wipe copies of the master key(s) that
+ userspace might have as well. Therefore, userspace must wipe all
+ copies of the master key(s) it makes as well. Naturally, the same
+ also applies to all higher levels in the key hierarchy. Userspace
+ should also follow other security precautions such as mlock()ing
+ memory containing keys to prevent it from being swapped out.
+
+- In general, decrypted contents and filenames in the kernel VFS
+ caches are freed but not wiped. Therefore, portions thereof may be
+ recoverable from freed memory, even after the corresponding key(s)
+ were wiped. To partially solve this, you can set
+ CONFIG_PAGE_POISONING=y in your kernel config and add page_poison=1
+ to your kernel command line. However, this has a performance cost.
+
+- Secret keys might still exist in CPU registers, in crypto
+ accelerator hardware (if used by the crypto API to implement any of
+ the algorithms), or in other places not explicitly considered here.
+
+Limitations of v1 policies
+~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+v1 encryption policies have some weaknesses with respect to online
+attacks:
+
+- There is no verification that the provided master key is correct.
+ Consequently, malicious users can associate the wrong key with
+ encrypted files, even files to which they have only read-only
+ access.
+
+- A compromise of a per-file key also compromises the master key from
+ which it was derived.
+
+- Non-root users cannot securely remove encryption keys.
+
+All the above problems are fixed with v2 encryption policies. For
+this reason among others, it is recommended to use v2 encryption
+policies on all new encrypted directories.
Key hierarchy
=============
@@ -123,11 +181,46 @@ appropriate master key. There can be any number of master keys, each
of which protects any number of directory trees on any number of
filesystems.
-Userspace should generate master keys either using a cryptographically
-secure random number generator, or by using a KDF (Key Derivation
-Function). Note that whenever a KDF is used to "stretch" a
-lower-entropy secret such as a passphrase, it is critical that a KDF
-designed for this purpose be used, such as scrypt, PBKDF2, or Argon2.
+Master keys should be pseudorandom, i.e. indistinguishable from random
+bytestrings of the same length. This implies that users **must not**
+directly use a password as a master key, zero-pad a shorter key, or
+repeat a shorter key. Instead, users should generate master keys
+either using a cryptographically secure random number generator, or by
+using a KDF (Key Derivation Function). Note that whenever a KDF is
+used to "stretch" a lower-entropy secret such as a passphrase, it is
+critical that a KDF designed for this purpose be used, such as scrypt,
+PBKDF2, or Argon2.
+
+Key derivation function
+-----------------------
+
+With one exception, fscrypt never uses the master key(s) for
+encryption directly. Instead, they are only used as input to a KDF
+(Key Derivation Function) to derive the actual keys.
+
+The KDF used for a particular master key differs depending on whether
+the key is used for v1 encryption policies or for v2 encryption
+policies. Users **must not** use the same key for both v1 and v2
+encryption policies.
+
+For v1 encryption policies, the KDF only supports deriving per-file
+encryption keys. It works by encrypting the master key with
+AES-128-ECB, using the file's 16-byte nonce as the AES key. The
+resulting ciphertext is used as the derived key. If the ciphertext is
+longer than needed, then it is truncated to the needed length.
+
+For v2 encryption policies, the KDF is HKDF-SHA512. The master key is
+passed as the "input keying material", a fixed value is passed as the
+"salt", and a distinct "application-specific information string" is
+used for each distinct key to be derived. For example, when a
+per-file encryption key is derived, the application-specific
+information string is the file's nonce prefixed with a context byte.
+Different context bytes are used for other types of derived keys.
+
+HKDF-SHA512 is preferred to the original AES-128-ECB based KDF because
+HKDF is more flexible, is nonreversible, and evenly distributes
+entropy from the master key. HKDF is also standardized and widely
+used by other software, whereas the AES-128-ECB based KDF is ad-hoc.
Per-file keys
-------------
@@ -138,29 +231,9 @@ files doesn't map to the same ciphertext, or vice versa. In most
cases, fscrypt does this by deriving per-file keys. When a new
encrypted inode (regular file, directory, or symlink) is created,
fscrypt randomly generates a 16-byte nonce and stores it in the
-inode's encryption xattr. Then, it uses a KDF (Key Derivation
-Function) to derive the file's key from the master key and nonce.
-
-The Adiantum encryption mode (see `Encryption modes and usage`_) is
-special, since it accepts longer IVs and is suitable for both contents
-and filenames encryption. For it, a "direct key" option is offered
-where the file's nonce is included in the IVs and the master key is
-used for encryption directly. This improves performance; however,
-users must not use the same master key for any other encryption mode.
-
-Below, the KDF and design considerations are described in more detail.
-
-The current KDF works by encrypting the master key with AES-128-ECB,
-using the file's nonce as the AES key. The output is used as the
-derived key. If the output is longer than needed, then it is
-truncated to the needed length.
-
-Note: this KDF meets the primary security requirement, which is to
-produce unique derived keys that preserve the entropy of the master
-key, assuming that the master key is already a good pseudorandom key.
-However, it is nonstandard and has some problems such as being
-reversible, so it is generally considered to be a mistake! It may be
-replaced with HKDF or another more standard KDF in the future.
+inode's encryption xattr. Then, it uses a KDF (as described in `Key
+derivation function`_) to derive the file's key from the master key
+and nonce.
Key derivation was chosen over key wrapping because wrapped keys would
require larger xattrs which would be less likely to fit in-line in the
@@ -176,6 +249,37 @@ rejected as it would have prevented ext4 filesystems from being
resized, and by itself still wouldn't have been sufficient to prevent
the same key from being directly reused for both XTS and CTS-CBC.
+DIRECT_KEY and per-mode keys
+----------------------------
+
+The Adiantum encryption mode (see `Encryption modes and usage`_) is
+suitable for both contents and filenames encryption, and it accepts
+long IVs --- long enough to hold both an 8-byte logical block number
+and a 16-byte per-file nonce. Also, the overhead of each Adiantum key
+is greater than that of an AES-256-XTS key.
+
+Therefore, to improve performance and save memory, for Adiantum a
+"direct key" configuration is supported. When the user has enabled
+this by setting FSCRYPT_POLICY_FLAG_DIRECT_KEY in the fscrypt policy,
+per-file keys are not used. Instead, whenever any data (contents or
+filenames) is encrypted, the file's 16-byte nonce is included in the
+IV. Moreover:
+
+- For v1 encryption policies, the encryption is done directly with the
+ master key. Because of this, users **must not** use the same master
+ key for any other purpose, even for other v1 policies.
+
+- For v2 encryption policies, the encryption is done with a per-mode
+ key derived using the KDF. Users may use the same master key for
+ other v2 encryption policies.
+
+Key identifiers
+---------------
+
+For master keys used for v2 encryption policies, a unique 16-byte "key
+identifier" is also derived using the KDF. This value is stored in
+the clear, since it is needed to reliably identify the key itself.
+
Encryption modes and usage
==========================
@@ -271,21 +375,38 @@ Setting an encryption policy
The FS_IOC_SET_ENCRYPTION_POLICY ioctl sets an encryption policy on an
empty directory or verifies that a directory or regular file already
has the specified encryption policy. It takes in a pointer to a
-:c:type:`struct fscrypt_policy`, defined as follows::
+:c:type:`struct fscrypt_policy_v1` or a :c:type:`struct
+fscrypt_policy_v2`, defined as follows::
- #define FSCRYPT_KEY_DESCRIPTOR_SIZE 8
-
- struct fscrypt_policy {
+ #define FSCRYPT_POLICY_V1 0
+ #define FSCRYPT_KEY_DESCRIPTOR_SIZE 8
+ struct fscrypt_policy_v1 {
__u8 version;
__u8 contents_encryption_mode;
__u8 filenames_encryption_mode;
__u8 flags;
__u8 master_key_descriptor[FSCRYPT_KEY_DESCRIPTOR_SIZE];
};
+ #define fscrypt_policy fscrypt_policy_v1
+
+ #define FSCRYPT_POLICY_V2 2
+ #define FSCRYPT_KEY_IDENTIFIER_SIZE 16
+ struct fscrypt_policy_v2 {
+ __u8 version;
+ __u8 contents_encryption_mode;
+ __u8 filenames_encryption_mode;
+ __u8 flags;
+ __u8 __reserved[4];
+ __u8 master_key_identifier[FSCRYPT_KEY_IDENTIFIER_SIZE];
+ };
This structure must be initialized as follows:
-- ``version`` must be 0.
+- ``version`` must be FSCRYPT_POLICY_V1 (0) if the struct is
+ :c:type:`fscrypt_policy_v1` or FSCRYPT_POLICY_V2 (2) if the struct
+ is :c:type:`fscrypt_policy_v2`. (Note: we refer to the original
+ policy version as "v1", though its version code is really 0.) For
+ new encrypted directories, use v2 policies.
- ``contents_encryption_mode`` and ``filenames_encryption_mode`` must
be set to constants from ``<linux/fs.h>`` which identify the
@@ -295,21 +416,30 @@ This structure must be initialized as follows:
- ``flags`` must contain a value from ``<linux/fs.h>`` which
identifies the amount of NUL-padding to use when encrypting
- filenames. If unsure, use FSCRYPT_POLICY_FLAGS_PAD_32 (0x3). In
- addition, if the chosen encryption modes are both
+ filenames. If unsure, use FSCRYPT_POLICY_FLAGS_PAD_32 (0x3).
+ Additionally, if the encryption modes are both
FSCRYPT_MODE_ADIANTUM, this can contain
- FSCRYPT_POLICY_FLAG_DIRECT_KEY to specify that the master key should
- be used directly, without key derivation.
+ FSCRYPT_POLICY_FLAG_DIRECT_KEY; see `DIRECT_KEY and per-mode keys`_.
-- ``master_key_descriptor`` specifies how to find the master key in
- the keyring; see `Adding keys`_. It is up to userspace to choose a
- unique ``master_key_descriptor`` for each master key. The e4crypt
- and fscrypt tools use the first 8 bytes of
+- For v2 encryption policies, ``__reserved`` must be zeroed.
+
+- For v1 encryption policies, ``master_key_descriptor`` specifies how
+ to find the master key in a keyring; see `Adding keys`_. It is up
+ to userspace to choose a unique ``master_key_descriptor`` for each
+ master key. The e4crypt and fscrypt tools use the first 8 bytes of
``SHA-512(SHA-512(master_key))``, but this particular scheme is not
required. Also, the master key need not be in the keyring yet when
FS_IOC_SET_ENCRYPTION_POLICY is executed. However, it must be added
before any files can be created in the encrypted directory.
+ For v2 encryption policies, ``master_key_descriptor`` has been
+ replaced with ``master_key_identifier``, which is longer and cannot
+ be arbitrarily chosen. Instead, the key must first be added using
+ FS_IOC_ADD_ENCRYPTION_KEY, as described in `Adding keys`_. Then,
+ the ``key_spec.u.identifier`` the kernel returned in the
+ :c:type:`struct fscrypt_add_key_arg` must be used as the
+ ``master_key_identifier`` in the :c:type:`struct fscrypt_policy_v2`.
+
If the file is not yet encrypted, then FS_IOC_SET_ENCRYPTION_POLICY
verifies that the file is an empty directory. If so, the specified
encryption policy is assigned to the directory, turning it into an
@@ -325,6 +455,15 @@ policy exactly matches the actual one. If they match, then the ioctl
returns 0. Otherwise, it fails with EEXIST. This works on both
regular files and directories, including nonempty directories.
+When a v2 encryption policy is assigned to a directory, it is also
+required that either the specified key has been added by the current
+user or that the caller has CAP_FOWNER in the initial user namespace.
+(This is needed to prevent a user from encrypting their data with
+another user's key.) The key must remain added while
+FS_IOC_SET_ENCRYPTION_POLICY is executing. However, if the new
+encrypted directory does not need to be accessed immediately, then the
+key can be removed right away afterwards.
+
Note that the ext4 filesystem does not allow the root directory to be
encrypted, even if it is empty. Users who want to encrypt an entire
filesystem with one key should consider using dm-crypt instead.
@@ -337,7 +476,11 @@ FS_IOC_SET_ENCRYPTION_POLICY can fail with the following errors:
- ``EEXIST``: the file is already encrypted with an encryption policy
different from the one specified
- ``EINVAL``: an invalid encryption policy was specified (invalid
- version, mode(s), or flags)
+ version, mode(s), or flags; or reserved bits were set)
+- ``ENOKEY``: a v2 encryption policy was specified, but the key with
+ the specified ``master_key_identifier`` has not been added, nor does
+ the process have the CAP_FOWNER capability in the initial user
+ namespace
- ``ENOTDIR``: the file is unencrypted and is a regular file, not a
directory
- ``ENOTEMPTY``: the file is unencrypted and is a nonempty directory
@@ -356,25 +499,78 @@ FS_IOC_SET_ENCRYPTION_POLICY can fail with the following errors:
Getting an encryption policy
----------------------------
-The FS_IOC_GET_ENCRYPTION_POLICY ioctl retrieves the :c:type:`struct
-fscrypt_policy`, if any, for a directory or regular file. See above
-for the struct definition. No additional permissions are required
-beyond the ability to open the file.
+Two ioctls are available to get a file's encryption policy:
+
+- FS_IOC_GET_ENCRYPTION_POLICY_EX
+- FS_IOC_GET_ENCRYPTION_POLICY
+
+The extended (_EX) version of the ioctl is more general and is
+recommended to use when possible. However, on older kernels only the
+original ioctl is available. Applications should try the extended
+version, and if it fails with ENOTTY fall back to the original
+version.
+
+Preferred method
+~~~~~~~~~~~~~~~~
+
+The FS_IOC_GET_ENCRYPTION_POLICY_EX ioctl retrieves the encryption
+policy, if any, for a directory or regular file. No additional
+permissions are required beyond the ability to open the file. It
+takes in a pointer to a :c:type:`struct fscrypt_get_policy_ex_arg`,
+defined as follows::
+
+ struct fscrypt_get_policy_ex_arg {
+ __u64 policy_size; /* input/output */
+ union {
+ __u8 version;
+ struct fscrypt_policy_v1 v1;
+ struct fscrypt_policy_v2 v2;
+ } policy; /* output */
+ };
+
+The caller must initialize ``policy_size`` to the size available for
+the policy struct, i.e. ``sizeof(arg.policy)``.
+
+On success, the policy struct is returned in ``policy``, and its
+actual size is returned in ``policy_size``. ``policy.version`` should
+be checked to determine the version of policy returned. Note that the
+version code for the "v1" policy is actually 0 (FSCRYPT_POLICY_V1).
-FS_IOC_GET_ENCRYPTION_POLICY can fail with the following errors:
+FS_IOC_GET_ENCRYPTION_POLICY_EX can fail with the following errors:
- ``EINVAL``: the file is encrypted, but it uses an unrecognized
- encryption context format
+ encryption policy version
- ``ENODATA``: the file is not encrypted
-- ``ENOTTY``: this type of filesystem does not implement encryption
+- ``ENOTTY``: this type of filesystem does not implement encryption,
+ or this kernel is too old to support FS_IOC_GET_ENCRYPTION_POLICY_EX
+ (try FS_IOC_GET_ENCRYPTION_POLICY instead)
- ``EOPNOTSUPP``: the kernel was not configured with encryption
support for this filesystem
+- ``EOVERFLOW``: the file is encrypted and uses a recognized
+ encryption policy version, but the policy struct does not fit into
+ the provided buffer
Note: if you only need to know whether a file is encrypted or not, on
most filesystems it is also possible to use the FS_IOC_GETFLAGS ioctl
and check for FS_ENCRYPT_FL, or to use the statx() system call and
check for STATX_ATTR_ENCRYPTED in stx_attributes.
+Legacy method
+~~~~~~~~~~~~~
+
+The FS_IOC_GET_ENCRYPTION_POLICY ioctl can also retrieve the
+encryption policy, if any, for a directory or regular file. However,
+unlike the extended version (FS_IOC_GET_ENCRYPTION_POLICY_EX),
+FS_IOC_GET_ENCRYPTION_POLICY only supports the original policy
+version. It takes in a pointer directly to a :c:type:`struct
+fscrypt_policy_v1` rather than a :c:type:`struct
+fscrypt_get_policy_ex_arg`.
+
+The error codes for FS_IOC_GET_ENCRYPTION_POLICY are the same as those
+for FS_IOC_GET_ENCRYPTION_POLICY_EX, except that
+FS_IOC_GET_ENCRYPTION_POLICY also returns ``EINVAL`` if the file is
+encrypted using a newer encryption policy version.
+
Getting the per-filesystem salt
-------------------------------
@@ -390,8 +586,98 @@ generate and manage any needed salt(s) in userspace.
Adding keys
-----------
-To provide a master key, userspace must add it to an appropriate
-keyring using the add_key() system call (see:
+Preferred method
+~~~~~~~~~~~~~~~~
+
+The FS_IOC_ADD_ENCRYPTION_KEY ioctl adds a master encryption key to
+the filesystem, making all files on the filesystem which were
+encrypted using that key appear "unlocked", i.e. in plaintext form.
+It can be executed on any file or directory on the target filesystem,
+but using the filesystem's root directory is recommended. It takes in
+a pointer to a :c:type:`struct fscrypt_add_key_arg`, defined as
+follows::
+
+ struct fscrypt_add_key_arg {
+ struct fscrypt_key_specifier key_spec;
+ __u32 raw_size;
+ __u32 __reserved[9];
+ __u8 raw[];
+ };
+
+ struct fscrypt_key_specifier {
+ #define FSCRYPT_KEY_SPEC_TYPE_DESCRIPTOR 1
+ #define FSCRYPT_KEY_SPEC_TYPE_IDENTIFIER 2
+ __u32 type;
+ __u32 __reserved;
+ union {
+ __u8 __reserved[32]; /* reserve some extra space */
+ __u8 descriptor[FSCRYPT_KEY_DESCRIPTOR_SIZE];
+ __u8 identifier[FSCRYPT_KEY_IDENTIFIER_SIZE];
+ } u;
+ };
+
+:c:type:`struct fscrypt_add_key_arg` must be zeroed, then initialized
+as follows:
+
+- If the key is being added for use by v1 encryption policies, then
+ ``key_spec.type`` must contain FSCRYPT_KEY_SPEC_TYPE_DESCRIPTOR, and
+ ``key_spec.u.descriptor`` must contain the descriptor of the key
+ being added, corresponding to the value in the
+ ``master_key_descriptor`` field of :c:type:`struct
+ fscrypt_policy_v1`. To add this type of key, the calling process
+ must have the CAP_SYS_ADMIN capability in the initial user
+ namespace.
+
+ Alternatively, if the key is being added for use by v2 encryption
+ policies, then ``key_spec.type`` must contain
+ FSCRYPT_KEY_SPEC_TYPE_IDENTIFIER, and ``key_spec.u.identifier`` is
+ an *output* field which the kernel fills in with a cryptographic
+ hash of the key. To add this type of key, the calling process does
+ not need any privileges. However, the number of keys that can be
+ added is limited by the user's quota for the keyrings service (see
+ ``Documentation/security/keys/core.rst``).
+
+- ``raw_size`` must be the size of the ``raw`` key provided, in bytes.
+
+- ``raw`` is a variable-length field which must contain the actual
+ key, ``raw_size`` bytes long.
+
+FS_IOC_ADD_ENCRYPTION_KEY can fail with the following errors:
+
+- ``EACCES``: FSCRYPT_KEY_SPEC_TYPE_DESCRIPTOR was specified, but the
+ caller does not have the CAP_SYS_ADMIN capability in the initial
+ user namespace
+- ``EDQUOT``: the key quota for this user would be exceeded by adding
+ the key
+- ``EINVAL``: invalid key size or key specifier type, or reserved bits
+ were set
+- ``ENOTTY``: this type of filesystem does not implement encryption
+- ``EOPNOTSUPP``: the kernel was not configured with encryption
+ support for this filesystem, or the filesystem superblock has not
+ had encryption enabled on it
+
+Legacy method
+~~~~~~~~~~~~~
+
+For v1 encryption policies, a master encryption key can also be
+provided by adding it to a process-subscribed keyring, e.g. to a
+session keyring, or to a user keyring if the user keyring is linked
+into the session keyring.
+
+This method is deprecated (and not supported for v2 encryption
+policies) for several reasons. First, it cannot be used in
+combination with FS_IOC_REMOVE_ENCRYPTION_KEY (see `Removing keys`_),
+so for removing a key a workaround such as keyctl_unlink() in
+combination with ``sync; echo 2 > /proc/sys/vm/drop_caches`` would
+have to be used. Second, it doesn't match the fact that the
+locked/unlocked status of encrypted files (i.e. whether they appear to
+be in plaintext form or in ciphertext form) is global. This mismatch
+has caused much confusion as well as real problems when processes
+running under different UIDs, such as a ``sudo`` command, need to
+access encrypted files.
+
+Nevertheless, to add a key to one of the process-subscribed keyrings,
+the add_key() system call can be used (see:
``Documentation/security/keys/core.rst``). The key type must be
"logon"; keys of this type are kept in kernel memory and cannot be
read back by userspace. The key description must be "fscrypt:"
@@ -399,12 +685,12 @@ followed by the 16-character lower case hex representation of the
``master_key_descriptor`` that was set in the encryption policy. The
key payload must conform to the following structure::
- #define FSCRYPT_MAX_KEY_SIZE 64
+ #define FSCRYPT_MAX_KEY_SIZE 64
struct fscrypt_key {
- u32 mode;
- u8 raw[FSCRYPT_MAX_KEY_SIZE];
- u32 size;
+ __u32 mode;
+ __u8 raw[FSCRYPT_MAX_KEY_SIZE];
+ __u32 size;
};
``mode`` is ignored; just set it to 0. The actual key is provided in
@@ -416,26 +702,145 @@ with a filesystem-specific prefix such as "ext4:". However, the
filesystem-specific prefixes are deprecated and should not be used in
new programs.
-There are several different types of keyrings in which encryption keys
-may be placed, such as a session keyring, a user session keyring, or a
-user keyring. Each key must be placed in a keyring that is "attached"
-to all processes that might need to access files encrypted with it, in
-the sense that request_key() will find the key. Generally, if only
-processes belonging to a specific user need to access a given
-encrypted directory and no session keyring has been installed, then
-that directory's key should be placed in that user's user session
-keyring or user keyring. Otherwise, a session keyring should be
-installed if needed, and the key should be linked into that session
-keyring, or in a keyring linked into that session keyring.
-
-Note: introducing the complex visibility semantics of keyrings here
-was arguably a mistake --- especially given that by design, after any
-process successfully opens an encrypted file (thereby setting up the
-per-file key), possessing the keyring key is not actually required for
-any process to read/write the file until its in-memory inode is
-evicted. In the future there probably should be a way to provide keys
-directly to the filesystem instead, which would make the intended
-semantics clearer.
+Removing keys
+-------------
+
+The FS_IOC_REMOVE_ENCRYPTION_KEY ioctl can be used to remove a master
+encryption key from the kernel, wiping the corresponding secrets from
+memory and causing any files which were "unlocked" with the key to
+appear "locked" again. It can be executed on any file or directory on
+the target filesystem, but using the filesystem's root directory is
+recommended. It takes in a pointer to a :c:type:`struct
+fscrypt_remove_key_arg`, defined as follows::
+
+ struct fscrypt_remove_key_arg {
+ struct fscrypt_key_specifier key_spec;
+ #define FSCRYPT_REMOVE_KEY_FLAG_ALL_USERS 0x00000001
+ __u32 flags;
+ __u32 __reserved[5];
+ };
+
+This structure must be zeroed, then initialized as follows:
+
+- The key to remove is specified by ``key_spec``:
+
+ - To remove a key used by v1 encryption policies, set
+ ``key_spec.type`` to FSCRYPT_KEY_SPEC_TYPE_DESCRIPTOR and fill
+ in ``key_spec.u.descriptor``. To remove this type of key, the
+ calling process must have the CAP_SYS_ADMIN capability in the
+ initial user namespace.
+
+ - To remove a key used by v2 encryption policies, set
+ ``key_spec.type`` to FSCRYPT_KEY_SPEC_TYPE_IDENTIFIER and fill
+ in ``key_spec.u.identifier``. To remove this type of key, no
+ privileges are needed. However, users can only remove keys that
+ they added themselves, subject to privileged override with
+ FSCRYPT_REMOVE_KEY_FLAG_ALL_USERS.
+
+- ``flags`` can contain the following flags:
+
+ - ``FSCRYPT_REMOVE_KEY_FLAG_ALL_USERS`` specifies that the key
+ should be removed even if it has also been added by other users.
+ Specifying this flag requires the CAP_SYS_ADMIN capability in
+ the initial user namespace.
+
+FS_IOC_REMOVE_ENCRYPTION_KEY can fail with the following errors:
+
+- ``EACCES``: The FSCRYPT_KEY_SPEC_TYPE_DESCRIPTOR key specifier type
+ and/or the FSCRYPT_REMOVE_KEY_FLAG_ALL_USERS flag was specified, but
+ the caller does not have the CAP_SYS_ADMIN capability in the initial
+ user namespace
+- ``EBUSY``: the master key secret was wiped from memory, but some
+ files which were unlocked with it are still in use. Such files
+ could not be locked, nor could their per-file keys be wiped from
+ memory. The ioctl may be retried later to re-attempt locking the
+ remaining files. Note: if the files cease being used, the key
+ removal may (but is not guaranteed to) complete before this ioctl is
+ retried; in this case, retrying this ioctl will return ``ENOKEY``.
+- ``EINVAL``: invalid flags or key specifier type, or reserved bits
+ were set
+- ``ENOKEY``: the key is not present or has already been fully removed
+- ``ENOTTY``: this type of filesystem does not implement encryption
+- ``EOPNOTSUPP``: the kernel was not configured with encryption
+ support for this filesystem, or the filesystem superblock has not
+ had encryption enabled on it
+- ``EUSERS``: the key cannot be removed because other users have added
+ it too
+
+Before using this ioctl, read the `Kernel memory compromise`_ section
+for a discussion of the security goals and limitations of this ioctl.
+
+Getting key status
+------------------
+
+The FS_IOC_GET_ENCRYPTION_KEY_STATUS ioctl retrieves the status of a
+master encryption key. It can be executed on any file or directory on
+the target filesystem, but using the filesystem's root directory is
+recommended. It takes in a pointer to a :c:type:`struct
+fscrypt_get_key_status_arg`, defined as follows::
+
+ struct fscrypt_get_key_status_arg {
+ /* input */
+ struct fscrypt_key_specifier key_spec;
+ __u32 __reserved[6];
+
+ /* output */
+ #define FSCRYPT_KEY_STATUS_ABSENT 1
+ #define FSCRYPT_KEY_STATUS_PRESENT 2
+ #define FSCRYPT_KEY_STATUS_INCOMPLETELY_REMOVED 3
+ __u32 status;
+ #define FSCRYPT_KEY_STATUS_FLAG_ADDED_BY_SELF 0x00000001
+ __u32 status_flags;
+ __u32 user_count;
+ __u32 __out_reserved[13];
+ };
+
+The caller must zero all input fields, then fill in ``key_spec``:
+
+ - To get the status of a key for v1 encryption policies, set
+ ``key_spec.type`` to FSCRYPT_KEY_SPEC_TYPE_DESCRIPTOR and fill
+ in ``key_spec.u.descriptor``.
+
+ - To get the status of a key for v2 encryption policies, set
+ ``key_spec.type`` to FSCRYPT_KEY_SPEC_TYPE_IDENTIFIER and fill
+ in ``key_spec.u.identifier``.
+
+On success, 0 is returned and the kernel fills in the output fields:
+
+- ``status`` indicates whether the key is absent, present, or
+ incompletely removed. Incompletely removed means that the master
+ secret has been removed, but some files are still in use; i.e.,
+ FS_IOC_REMOVE_ENCRYPTION_KEY returned EBUSY.
+
+- ``status_flags`` can contain the following flags:
+
+ - ``FSCRYPT_KEY_STATUS_FLAG_ADDED_BY_SELF`` indicates that the key
+ has added by the current user. This is only set for keys
+ identified by ``identifier`` rather than by ``descriptor``.
+
+- ``user_count`` specifies the number of users who have added the key.
+ This is only set for keys identified by ``identifier`` rather than
+ by ``descriptor``.
+
+FS_IOC_GET_ENCRYPTION_KEY_STATUS can fail with the following errors:
+
+- ``EINVAL``: invalid key specifier type, or reserved bits were set
+- ``ENOTTY``: this type of filesystem does not implement encryption
+- ``EOPNOTSUPP``: the kernel was not configured with encryption
+ support for this filesystem, or the filesystem superblock has not
+ had encryption enabled on it
+
+Among other use cases, FS_IOC_GET_ENCRYPTION_KEY_STATUS might be
+useful for determining whether the key for a given encrypted directory
+needs to be added before prompting the user for the passphrase needed
+to derive the key.
+
+FS_IOC_GET_ENCRYPTION_KEY_STATUS can only get the status of keys in
+the filesystem-level keyring, i.e. the keyring managed by
+FS_IOC_ADD_ENCRYPTION_KEY and FS_IOC_REMOVE_ENCRYPTION_KEY. It cannot
+get the status of a key that has only been added for use by v1
+encryption policies using the legacy mechanism involving
+process-subscribed keyrings.
Access semantics
================
@@ -498,7 +903,7 @@ Without the key
Some filesystem operations may be performed on encrypted regular
files, directories, and symlinks even before their encryption key has
-been provided:
+been added, or after their encryption key has been removed:
- File metadata may be read, e.g. using stat().
@@ -563,20 +968,20 @@ Encryption context
------------------
An encryption policy is represented on-disk by a :c:type:`struct
-fscrypt_context`. It is up to individual filesystems to decide where
-to store it, but normally it would be stored in a hidden extended
-attribute. It should *not* be exposed by the xattr-related system
-calls such as getxattr() and setxattr() because of the special
-semantics of the encryption xattr. (In particular, there would be
-much confusion if an encryption policy were to be added to or removed
-from anything other than an empty directory.) The struct is defined
-as follows::
+fscrypt_context_v1` or a :c:type:`struct fscrypt_context_v2`. It is
+up to individual filesystems to decide where to store it, but normally
+it would be stored in a hidden extended attribute. It should *not* be
+exposed by the xattr-related system calls such as getxattr() and
+setxattr() because of the special semantics of the encryption xattr.
+(In particular, there would be much confusion if an encryption policy
+were to be added to or removed from anything other than an empty
+directory.) These structs are defined as follows::
- #define FSCRYPT_KEY_DESCRIPTOR_SIZE 8
#define FS_KEY_DERIVATION_NONCE_SIZE 16
- struct fscrypt_context {
- u8 format;
+ #define FSCRYPT_KEY_DESCRIPTOR_SIZE 8
+ struct fscrypt_context_v1 {
+ u8 version;
u8 contents_encryption_mode;
u8 filenames_encryption_mode;
u8 flags;
@@ -584,12 +989,23 @@ as follows::
u8 nonce[FS_KEY_DERIVATION_NONCE_SIZE];
};
-Note that :c:type:`struct fscrypt_context` contains the same
-information as :c:type:`struct fscrypt_policy` (see `Setting an
-encryption policy`_), except that :c:type:`struct fscrypt_context`
-also contains a nonce. The nonce is randomly generated by the kernel
-and is used to derive the inode's encryption key as described in
-`Per-file keys`_.
+ #define FSCRYPT_KEY_IDENTIFIER_SIZE 16
+ struct fscrypt_context_v2 {
+ u8 version;
+ u8 contents_encryption_mode;
+ u8 filenames_encryption_mode;
+ u8 flags;
+ u8 __reserved[4];
+ u8 master_key_identifier[FSCRYPT_KEY_IDENTIFIER_SIZE];
+ u8 nonce[FS_KEY_DERIVATION_NONCE_SIZE];
+ };
+
+The context structs contain the same information as the corresponding
+policy structs (see `Setting an encryption policy`_), except that the
+context structs also contain a nonce. The nonce is randomly generated
+by the kernel and is used as KDF input or as a tweak to cause
+different files to be encrypted differently; see `Per-file keys`_ and
+`DIRECT_KEY and per-mode keys`_.
Data path changes
-----------------
--
2.21.0
^ permalink raw reply related
* [PATCH v4 16/17] ubifs: wire up new fscrypt ioctls
From: Eric Biggers @ 2019-04-02 15:45 UTC (permalink / raw)
To: linux-fscrypt
Cc: Satya Tangirala, linux-api, linux-f2fs-devel, keyrings, linux-mtd,
linux-crypto, linux-fsdevel, linux-ext4, Paul Crowley
In-Reply-To: <20190402154600.32432-1-ebiggers@kernel.org>
From: Eric Biggers <ebiggers@google.com>
Wire up the new ioctls for adding and removing fscrypt keys to/from the
filesystem, and the new ioctl for retrieving v2 encryption policies.
FS_IOC_REMOVE_ENCRYPTION_KEY also required making UBIFS use
fscrypt_drop_inode().
For more details see Documentation/filesystems/fscrypt.rst, as well as
the fscrypt patches that added the implementation of these ioctls.
Signed-off-by: Eric Biggers <ebiggers@google.com>
---
fs/ubifs/ioctl.c | 23 +++++++++++++++++------
fs/ubifs/super.c | 11 +++++++++++
2 files changed, 28 insertions(+), 6 deletions(-)
diff --git a/fs/ubifs/ioctl.c b/fs/ubifs/ioctl.c
index 82e4e6a30b044..3c640a7e937f4 100644
--- a/fs/ubifs/ioctl.c
+++ b/fs/ubifs/ioctl.c
@@ -205,13 +205,20 @@ long ubifs_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
return -EOPNOTSUPP;
#endif
}
- case FS_IOC_GET_ENCRYPTION_POLICY: {
-#ifdef CONFIG_FS_ENCRYPTION
+ case FS_IOC_GET_ENCRYPTION_POLICY:
return fscrypt_ioctl_get_policy(file, (void __user *)arg);
-#else
- return -EOPNOTSUPP;
-#endif
- }
+
+ case FS_IOC_GET_ENCRYPTION_POLICY_EX:
+ return fscrypt_ioctl_get_policy_ex(file, (void __user *)arg);
+
+ case FS_IOC_ADD_ENCRYPTION_KEY:
+ return fscrypt_ioctl_add_key(file, (void __user *)arg);
+
+ case FS_IOC_REMOVE_ENCRYPTION_KEY:
+ return fscrypt_ioctl_remove_key(file, (const void __user *)arg);
+
+ case FS_IOC_GET_ENCRYPTION_KEY_STATUS:
+ return fscrypt_ioctl_get_key_status(file, (void __user *)arg);
default:
return -ENOTTY;
@@ -230,6 +237,10 @@ long ubifs_compat_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
break;
case FS_IOC_SET_ENCRYPTION_POLICY:
case FS_IOC_GET_ENCRYPTION_POLICY:
+ case FS_IOC_GET_ENCRYPTION_POLICY_EX:
+ case FS_IOC_ADD_ENCRYPTION_KEY:
+ case FS_IOC_REMOVE_ENCRYPTION_KEY:
+ case FS_IOC_GET_ENCRYPTION_KEY_STATUS:
break;
default:
return -ENOIOCTLCMD;
diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c
index 8dc2818fdd849..cd4a3e2be2043 100644
--- a/fs/ubifs/super.c
+++ b/fs/ubifs/super.c
@@ -334,6 +334,16 @@ static int ubifs_write_inode(struct inode *inode, struct writeback_control *wbc)
return err;
}
+static int ubifs_drop_inode(struct inode *inode)
+{
+ int drop = generic_drop_inode(inode);
+
+ if (!drop)
+ drop = fscrypt_drop_inode(inode);
+
+ return drop;
+}
+
static void ubifs_evict_inode(struct inode *inode)
{
int err;
@@ -1982,6 +1992,7 @@ const struct super_operations ubifs_super_operations = {
.destroy_inode = ubifs_destroy_inode,
.put_super = ubifs_put_super,
.write_inode = ubifs_write_inode,
+ .drop_inode = ubifs_drop_inode,
.evict_inode = ubifs_evict_inode,
.statfs = ubifs_statfs,
.dirty_inode = ubifs_dirty_inode,
--
2.21.0
^ permalink raw reply related
* [PATCH v4 15/17] f2fs: wire up new fscrypt ioctls
From: Eric Biggers @ 2019-04-02 15:45 UTC (permalink / raw)
To: linux-fscrypt
Cc: Satya Tangirala, linux-api, linux-f2fs-devel, keyrings, linux-mtd,
linux-crypto, linux-fsdevel, linux-ext4, Paul Crowley
In-Reply-To: <20190402154600.32432-1-ebiggers@kernel.org>
From: Eric Biggers <ebiggers@google.com>
Wire up the new ioctls for adding and removing fscrypt keys to/from the
filesystem, and the new ioctl for retrieving v2 encryption policies.
FS_IOC_REMOVE_ENCRYPTION_KEY also required making f2fs_drop_inode() call
fscrypt_drop_inode().
For more details see Documentation/filesystems/fscrypt.rst, as well as
the fscrypt patches that added the implementation of these ioctls.
Signed-off-by: Eric Biggers <ebiggers@google.com>
---
fs/f2fs/file.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
fs/f2fs/super.c | 2 ++
2 files changed, 48 insertions(+)
diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index 5742ab8b57dc6..251e374353da8 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -2095,6 +2095,40 @@ static int f2fs_ioc_get_encryption_pwsalt(struct file *filp, unsigned long arg)
return err;
}
+static int f2fs_ioc_get_encryption_policy_ex(struct file *filp,
+ unsigned long arg)
+{
+ if (!f2fs_sb_has_encrypt(F2FS_I_SB(file_inode(filp))))
+ return -EOPNOTSUPP;
+
+ return fscrypt_ioctl_get_policy_ex(filp, (void __user *)arg);
+}
+
+static int f2fs_ioc_add_encryption_key(struct file *filp, unsigned long arg)
+{
+ if (!f2fs_sb_has_encrypt(F2FS_I_SB(file_inode(filp))))
+ return -EOPNOTSUPP;
+
+ return fscrypt_ioctl_add_key(filp, (void __user *)arg);
+}
+
+static int f2fs_ioc_remove_encryption_key(struct file *filp, unsigned long arg)
+{
+ if (!f2fs_sb_has_encrypt(F2FS_I_SB(file_inode(filp))))
+ return -EOPNOTSUPP;
+
+ return fscrypt_ioctl_remove_key(filp, (const void __user *)arg);
+}
+
+static int f2fs_ioc_get_encryption_key_status(struct file *filp,
+ unsigned long arg)
+{
+ if (!f2fs_sb_has_encrypt(F2FS_I_SB(file_inode(filp))))
+ return -EOPNOTSUPP;
+
+ return fscrypt_ioctl_get_key_status(filp, (void __user *)arg);
+}
+
static int f2fs_ioc_gc(struct file *filp, unsigned long arg)
{
struct inode *inode = file_inode(filp);
@@ -3000,6 +3034,14 @@ long f2fs_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
return f2fs_ioc_get_encryption_policy(filp, arg);
case F2FS_IOC_GET_ENCRYPTION_PWSALT:
return f2fs_ioc_get_encryption_pwsalt(filp, arg);
+ case FS_IOC_GET_ENCRYPTION_POLICY_EX:
+ return f2fs_ioc_get_encryption_policy_ex(filp, arg);
+ case FS_IOC_ADD_ENCRYPTION_KEY:
+ return f2fs_ioc_add_encryption_key(filp, arg);
+ case FS_IOC_REMOVE_ENCRYPTION_KEY:
+ return f2fs_ioc_remove_encryption_key(filp, arg);
+ case FS_IOC_GET_ENCRYPTION_KEY_STATUS:
+ return f2fs_ioc_get_encryption_key_status(filp, arg);
case F2FS_IOC_GARBAGE_COLLECT:
return f2fs_ioc_gc(filp, arg);
case F2FS_IOC_GARBAGE_COLLECT_RANGE:
@@ -3119,6 +3161,10 @@ long f2fs_compat_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
case F2FS_IOC_SET_ENCRYPTION_POLICY:
case F2FS_IOC_GET_ENCRYPTION_PWSALT:
case F2FS_IOC_GET_ENCRYPTION_POLICY:
+ case FS_IOC_GET_ENCRYPTION_POLICY_EX:
+ case FS_IOC_ADD_ENCRYPTION_KEY:
+ case FS_IOC_REMOVE_ENCRYPTION_KEY:
+ case FS_IOC_GET_ENCRYPTION_KEY_STATUS:
case F2FS_IOC_GARBAGE_COLLECT:
case F2FS_IOC_GARBAGE_COLLECT_RANGE:
case F2FS_IOC_WRITE_CHECKPOINT:
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index f2aaa2cc6b3e0..7219b3b2af759 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -934,6 +934,8 @@ static int f2fs_drop_inode(struct inode *inode)
return 0;
}
ret = generic_drop_inode(inode);
+ if (!ret)
+ ret = fscrypt_drop_inode(inode);
trace_f2fs_drop_inode(inode, ret);
return ret;
}
--
2.21.0
^ permalink raw reply related
* [PATCH v4 14/17] ext4: wire up new fscrypt ioctls
From: Eric Biggers @ 2019-04-02 15:45 UTC (permalink / raw)
To: linux-fscrypt
Cc: Satya Tangirala, linux-api, linux-f2fs-devel, keyrings, linux-mtd,
linux-crypto, linux-fsdevel, linux-ext4, Paul Crowley
In-Reply-To: <20190402154600.32432-1-ebiggers@kernel.org>
From: Eric Biggers <ebiggers@google.com>
Wire up the new ioctls for adding and removing fscrypt keys to/from the
filesystem, and the new ioctl for retrieving v2 encryption policies.
FS_IOC_REMOVE_ENCRYPTION_KEY also required making ext4_drop_inode() call
fscrypt_drop_inode().
For more details see Documentation/filesystems/fscrypt.rst, as well as
the fscrypt patches that added the implementation of these ioctls.
Signed-off-by: Eric Biggers <ebiggers@google.com>
---
fs/ext4/ioctl.c | 24 ++++++++++++++++++++++++
fs/ext4/super.c | 3 +++
2 files changed, 27 insertions(+)
diff --git a/fs/ext4/ioctl.c b/fs/ext4/ioctl.c
index bab3da4f1e0d3..3ed66628e9301 100644
--- a/fs/ext4/ioctl.c
+++ b/fs/ext4/ioctl.c
@@ -1074,6 +1074,26 @@ long ext4_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
case EXT4_IOC_GET_ENCRYPTION_POLICY:
return fscrypt_ioctl_get_policy(filp, (void __user *)arg);
+ case FS_IOC_GET_ENCRYPTION_POLICY_EX:
+ if (!ext4_has_feature_encrypt(sb))
+ return -EOPNOTSUPP;
+ return fscrypt_ioctl_get_policy_ex(filp, (void __user *)arg);
+
+ case FS_IOC_ADD_ENCRYPTION_KEY:
+ if (!ext4_has_feature_encrypt(sb))
+ return -EOPNOTSUPP;
+ return fscrypt_ioctl_add_key(filp, (void __user *)arg);
+
+ case FS_IOC_REMOVE_ENCRYPTION_KEY:
+ if (!ext4_has_feature_encrypt(sb))
+ return -EOPNOTSUPP;
+ return fscrypt_ioctl_remove_key(filp, (const void __user *)arg);
+
+ case FS_IOC_GET_ENCRYPTION_KEY_STATUS:
+ if (!ext4_has_feature_encrypt(sb))
+ return -EOPNOTSUPP;
+ return fscrypt_ioctl_get_key_status(filp, (void __user *)arg);
+
case EXT4_IOC_FSGETXATTR:
{
struct fsxattr fa;
@@ -1192,6 +1212,10 @@ long ext4_compat_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
case EXT4_IOC_SET_ENCRYPTION_POLICY:
case EXT4_IOC_GET_ENCRYPTION_PWSALT:
case EXT4_IOC_GET_ENCRYPTION_POLICY:
+ case FS_IOC_GET_ENCRYPTION_POLICY_EX:
+ case FS_IOC_ADD_ENCRYPTION_KEY:
+ case FS_IOC_REMOVE_ENCRYPTION_KEY:
+ case FS_IOC_GET_ENCRYPTION_KEY_STATUS:
case EXT4_IOC_SHUTDOWN:
case FS_IOC_GETFSMAP:
break;
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 6ed4eb81e6743..624543a3f4d3e 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -1103,6 +1103,9 @@ static int ext4_drop_inode(struct inode *inode)
{
int drop = generic_drop_inode(inode);
+ if (!drop)
+ drop = fscrypt_drop_inode(inode);
+
trace_ext4_drop_inode(inode, drop);
return drop;
}
--
2.21.0
^ permalink raw reply related
* [PATCH v4 13/17] fscrypt: require that key be added when setting a v2 encryption policy
From: Eric Biggers @ 2019-04-02 15:45 UTC (permalink / raw)
To: linux-fscrypt
Cc: Satya Tangirala, linux-api, linux-f2fs-devel, keyrings, linux-mtd,
linux-crypto, linux-fsdevel, linux-ext4, Paul Crowley
In-Reply-To: <20190402154600.32432-1-ebiggers@kernel.org>
From: Eric Biggers <ebiggers@google.com>
By looking up the master keys in a filesystem-level keyring rather than
in the calling processes' key hierarchy, it becomes possible for a user
to set an encryption policy which refers to some key they don't actually
know, then encrypt their files using that key. Cryptographically this
isn't much of a problem, but the semantics of this would be a bit weird.
Thus, enforce that a v2 encryption policy can only be set if the user
has previously added the key, or has capable(CAP_FOWNER).
We tolerate that this problem will continue to exist for v1 encryption
policies, however; there is no way around that.
Signed-off-by: Eric Biggers <ebiggers@google.com>
---
fs/crypto/fscrypt_private.h | 3 +++
fs/crypto/keyring.c | 47 +++++++++++++++++++++++++++++++++++++
fs/crypto/policy.c | 6 +++++
3 files changed, 56 insertions(+)
diff --git a/fs/crypto/fscrypt_private.h b/fs/crypto/fscrypt_private.h
index 5f679ba32cece..a398383ae9e34 100644
--- a/fs/crypto/fscrypt_private.h
+++ b/fs/crypto/fscrypt_private.h
@@ -411,6 +411,9 @@ extern struct key *
fscrypt_find_master_key(struct super_block *sb,
const struct fscrypt_key_specifier *mk_spec);
+extern int fscrypt_verify_key_added(struct super_block *sb,
+ const u8 identifier[FSCRYPT_KEY_IDENTIFIER_SIZE]);
+
extern int __init fscrypt_init_keyring(void);
extern void fscrypt_exit_keyring(void);
diff --git a/fs/crypto/keyring.c b/fs/crypto/keyring.c
index 3d17c13136a4a..2ccabf95f294c 100644
--- a/fs/crypto/keyring.c
+++ b/fs/crypto/keyring.c
@@ -562,6 +562,53 @@ int fscrypt_ioctl_add_key(struct file *filp, void __user *_uarg)
}
EXPORT_SYMBOL_GPL(fscrypt_ioctl_add_key);
+/*
+ * Verify that the current user has added a master key with the given identifier
+ * (returns -ENOKEY if not). This is needed to prevent a user from encrypting
+ * their files using some other user's key which they don't actually know.
+ * Cryptographically this isn't much of a problem, but the semantics of this
+ * would be a bit weird, so it's best to just forbid it.
+ *
+ * The system administrator (CAP_FOWNER) can override this, which should be
+ * enough for any use cases where encryption policies are being set using keys
+ * that were chosen ahead of time but aren't available at the moment.
+ *
+ * Note that the key may have already removed by the time this returns, but
+ * that's okay; we just care whether the key was there at some point.
+ *
+ * Return: 0 if the key is added, -ENOKEY if it isn't, or another -errno code
+ */
+int fscrypt_verify_key_added(struct super_block *sb,
+ const u8 identifier[FSCRYPT_KEY_IDENTIFIER_SIZE])
+{
+ struct fscrypt_key_specifier mk_spec;
+ struct key *key, *mk_user;
+ struct fscrypt_master_key *mk;
+ int err;
+
+ mk_spec.type = FSCRYPT_KEY_SPEC_TYPE_IDENTIFIER;
+ memcpy(mk_spec.u.identifier, identifier, FSCRYPT_KEY_IDENTIFIER_SIZE);
+
+ key = fscrypt_find_master_key(sb, &mk_spec);
+ if (IS_ERR(key)) {
+ err = PTR_ERR(key);
+ goto out;
+ }
+ mk = key->payload.data[0];
+ mk_user = find_master_key_user(mk);
+ if (IS_ERR(mk_user)) {
+ err = PTR_ERR(mk_user);
+ } else {
+ key_put(mk_user);
+ err = 0;
+ }
+ key_put(key);
+out:
+ if (err == -ENOKEY && capable(CAP_FOWNER))
+ err = 0;
+ return err;
+}
+
static void evict_dentries_for_decrypted_inodes(struct fscrypt_master_key *mk)
{
struct fscrypt_info *ci;
diff --git a/fs/crypto/policy.c b/fs/crypto/policy.c
index 912ff1d030796..fa86f96741308 100644
--- a/fs/crypto/policy.c
+++ b/fs/crypto/policy.c
@@ -209,6 +209,7 @@ static int set_encryption_policy(struct inode *inode,
{
union fscrypt_context ctx;
int ctxsize;
+ int err;
if (!fscrypt_supported_policy(policy))
return -EINVAL;
@@ -227,6 +228,11 @@ static int set_encryption_policy(struct inode *inode,
*/
pr_warn_once("%s (pid %d) is setting deprecated v1 encryption policy; recommend upgrading to v2.\n",
current->comm, current->pid);
+ } else {
+ err = fscrypt_verify_key_added(inode->i_sb,
+ policy->v2.master_key_identifier);
+ if (err)
+ return err;
}
ctxsize = fscrypt_new_context_from_policy(&ctx, policy);
--
2.21.0
^ permalink raw reply related
* [PATCH v4 12/17] fscrypt: allow unprivileged users to add/remove keys for v2 policies
From: Eric Biggers @ 2019-04-02 15:45 UTC (permalink / raw)
To: linux-fscrypt
Cc: Satya Tangirala, linux-api, linux-f2fs-devel, keyrings, linux-mtd,
linux-crypto, linux-fsdevel, linux-ext4, Paul Crowley
In-Reply-To: <20190402154600.32432-1-ebiggers@kernel.org>
From: Eric Biggers <ebiggers@google.com>
Allow the FS_IOC_ADD_ENCRYPTION_KEY and FS_IOC_REMOVE_ENCRYPTION_KEY
ioctls to be used by non-root users to add and remove encryption keys
from the filesystem-level crypto keyrings, subject to limitations.
Motivation: while privileged fscrypt key management is sufficient for
some users (e.g. Android and Chromium OS, where a privileged process
manages all keys), the old API by design also allows non-root users to
set up and use encrypted directories, and we don't want to regress on
that. Especially, we don't want to force users to continue using the
old API, running into the visibility mismatch between files and keyrings
and being unable to "lock" encrypted directories.
Intuitively, the ioctls have to be privileged since they manipulate
filesystem-level state. However, it's actually safe to make them
unprivileged if we very carefully enforce some specific limitations.
First, each key must be identified by a cryptographic hash so that a
user can't add the wrong key for another user's files. For v2
encryption policies, we use the key_identifier for this. v1 policies
don't have this, so managing keys for them remains privileged.
Second, each key a user adds is charged to their quota for the keyrings
service. Thus, a user can't exhaust memory by adding a huge number of
keys. By default each non-root user is allowed up to 200 keys; this can
be changed using the existing sysctl 'kernel.keys.maxkeys'.
Third, if multiple users add the same key, we keep track of those users
of the key (of which there remains a single copy), and won't really
remove the key, i.e. "lock" the encrypted files, until all those users
have removed it. This prevents denial of service attacks that would be
possible under simpler schemes, such allowing the first user who added a
key to remove it -- since that could be a malicious user who has
compromised the key. Of course, encryption keys should be kept secret,
but the idea is that using encryption should never be *less* secure than
not using encryption, even if your key was compromised.
We tolerate that a user will be unable to really remove a key, i.e.
unable to "lock" their encrypted files, if another user has added the
same key. But in a sense, this is actually a good thing because it will
avoid providing a false notion of security where a key appears to have
been removed when actually it's still in memory, available to any
attacker who compromises the operating system kernel.
Signed-off-by: Eric Biggers <ebiggers@google.com>
---
fs/crypto/fscrypt_private.h | 31 +++-
fs/crypto/keyring.c | 310 +++++++++++++++++++++++++++++++++--
fs/crypto/keysetup.c | 18 +-
include/uapi/linux/fscrypt.h | 9 +-
4 files changed, 340 insertions(+), 28 deletions(-)
diff --git a/fs/crypto/fscrypt_private.h b/fs/crypto/fscrypt_private.h
index 9591f5fd72b33..5f679ba32cece 100644
--- a/fs/crypto/fscrypt_private.h
+++ b/fs/crypto/fscrypt_private.h
@@ -338,9 +338,16 @@ struct fscrypt_master_key {
* FS_IOC_REMOVE_ENCRYPTION_KEY can be retried, or
* FS_IOC_ADD_ENCRYPTION_KEY can add the secret again.
*
- * Locking: protected by key->sem.
+ * Locking: protected by key->sem (outer) and mk_secret_sem (inner).
+ * The reason for two locks is that key->sem also protects modifying
+ * mk_users, which ranks it above the semaphore for the keyring key
+ * type, which is in turn above page faults (via keyring_read). But
+ * sometimes filesystems call fscrypt_get_encryption_info() from within
+ * a transaction, which ranks it below page faults. So we need a
+ * separate lock which protects mk_secret but not also mk_users.
*/
struct fscrypt_master_key_secret mk_secret;
+ struct rw_semaphore mk_secret_sem;
/*
* For v1 policy keys: an arbitrary key descriptor which was assigned by
@@ -350,6 +357,22 @@ struct fscrypt_master_key {
*/
struct fscrypt_key_specifier mk_spec;
+ /*
+ * Keyring which contains a key of type 'key_type_fscrypt_user' for each
+ * user who has added this key. Normally each key will be added by just
+ * one user, but it's possible that multiple users share a key, and in
+ * that case we need to keep track of those users so that one user can't
+ * remove the key before the others want it removed too.
+ *
+ * This is NULL for v1 policy keys; those can only be added by root.
+ *
+ * Locking: in addition to this keyrings own semaphore, this is
+ * protected by the master key's key->sem, so we can do atomic
+ * search+insert. It can also be searched without taking any locks, but
+ * in that case the returned key may have already been removed.
+ */
+ struct key *mk_users;
+
/*
* Length of ->mk_decrypted_inodes, plus one if mk_secret is present.
* Once this goes to 0, the master key is removed from ->s_master_keys.
@@ -377,9 +400,9 @@ is_master_key_secret_present(const struct fscrypt_master_key_secret *secret)
/*
* The READ_ONCE() is only necessary for fscrypt_drop_inode() and
* fscrypt_key_describe(). These run in atomic context, so they can't
- * take key->sem and thus 'secret' can change concurrently which would
- * be a data race. But they only need to know whether the secret *was*
- * present at the time of check, so READ_ONCE() suffices.
+ * take ->mk_secret_sem and thus 'secret' can change concurrently which
+ * would be a data race. But they only need to know whether the secret
+ * *was* present at the time of check, so READ_ONCE() suffices.
*/
return READ_ONCE(secret->size) != 0;
}
diff --git a/fs/crypto/keyring.c b/fs/crypto/keyring.c
index fe1b003975c4f..3d17c13136a4a 100644
--- a/fs/crypto/keyring.c
+++ b/fs/crypto/keyring.c
@@ -42,6 +42,7 @@ static void free_master_key(struct fscrypt_master_key *mk)
for (i = 0; i < ARRAY_SIZE(mk->mk_mode_keys); i++)
crypto_free_skcipher(mk->mk_mode_keys[i]);
+ key_put(mk->mk_users);
kzfree(mk);
}
@@ -101,7 +102,39 @@ static struct key_type key_type_fscrypt = {
.describe = fscrypt_key_describe,
};
-/* Search ->s_master_keys */
+static int fscrypt_user_key_instantiate(struct key *key,
+ struct key_preparsed_payload *prep)
+{
+ /*
+ * We just charge FSCRYPT_MAX_KEY_SIZE bytes to the user's key quota for
+ * each key, regardless of the exact key size. The amount of memory
+ * actually used is greater than the size of the raw key anyway.
+ */
+ return key_payload_reserve(key, FSCRYPT_MAX_KEY_SIZE);
+}
+
+static void fscrypt_user_key_describe(const struct key *key, struct seq_file *m)
+{
+ seq_puts(m, key->description);
+}
+
+/*
+ * Type of key in ->mk_users. Each key of this type represents a particular
+ * user who has added a particular master key.
+ *
+ * Note that the name of this key type really should be something like
+ * ".fscrypt-user" instead of simply ".fscrypt". But the shorter name is chosen
+ * mainly for simplicity of presentation in /proc/keys when read by a non-root
+ * user. And it is expected to be rare that a key is actually added by multiple
+ * users, since users should keep their encryption keys confidential.
+ */
+static struct key_type key_type_fscrypt_user = {
+ .name = ".fscrypt",
+ .instantiate = fscrypt_user_key_instantiate,
+ .describe = fscrypt_user_key_describe,
+};
+
+/* Search ->s_master_keys or ->mk_users */
static struct key *search_fscrypt_keyring(struct key *keyring,
struct key_type *type,
const char *description)
@@ -127,6 +160,13 @@ static struct key *search_fscrypt_keyring(struct key *keyring,
#define FSCRYPT_MK_DESCRIPTION_SIZE (2 * FSCRYPT_KEY_IDENTIFIER_SIZE + 1)
+#define FSCRYPT_MK_USERS_DESCRIPTION_SIZE \
+ (CONST_STRLEN("fscrypt-") + 2 * FSCRYPT_KEY_IDENTIFIER_SIZE + \
+ CONST_STRLEN("-users") + 1)
+
+#define FSCRYPT_MK_USER_DESCRIPTION_SIZE \
+ (2 * FSCRYPT_KEY_IDENTIFIER_SIZE + CONST_STRLEN(".uid.") + 10 + 1)
+
static void format_fs_keyring_description(
char description[FSCRYPT_FS_KEYRING_DESCRIPTION_SIZE],
const struct super_block *sb)
@@ -142,6 +182,23 @@ static void format_mk_description(
master_key_spec_len(mk_spec), (u8 *)&mk_spec->u);
}
+static void format_mk_users_keyring_description(
+ char description[FSCRYPT_MK_USERS_DESCRIPTION_SIZE],
+ const u8 mk_identifier[FSCRYPT_KEY_IDENTIFIER_SIZE])
+{
+ sprintf(description, "fscrypt-%*phN-users",
+ FSCRYPT_KEY_IDENTIFIER_SIZE, mk_identifier);
+}
+
+static void format_mk_user_description(
+ char description[FSCRYPT_MK_USER_DESCRIPTION_SIZE],
+ const u8 mk_identifier[FSCRYPT_KEY_IDENTIFIER_SIZE])
+{
+
+ sprintf(description, "%*phN.uid.%u", FSCRYPT_KEY_IDENTIFIER_SIZE,
+ mk_identifier, __kuid_val(current_fsuid()));
+}
+
/* Create ->s_master_keys if needed. Synchronized by fscrypt_add_key_mutex. */
static int allocate_filesystem_keyring(struct super_block *sb)
{
@@ -189,6 +246,81 @@ struct key *fscrypt_find_master_key(struct super_block *sb,
return search_fscrypt_keyring(keyring, &key_type_fscrypt, description);
}
+static int allocate_master_key_users_keyring(struct fscrypt_master_key *mk)
+{
+ char description[FSCRYPT_MK_USERS_DESCRIPTION_SIZE];
+ struct key *keyring;
+
+ format_mk_users_keyring_description(description,
+ mk->mk_spec.u.identifier);
+ keyring = keyring_alloc(description, GLOBAL_ROOT_UID, GLOBAL_ROOT_GID,
+ current_cred(), KEY_POS_SEARCH |
+ KEY_USR_SEARCH | KEY_USR_READ | KEY_USR_VIEW,
+ KEY_ALLOC_NOT_IN_QUOTA, NULL, NULL);
+ if (IS_ERR(keyring))
+ return PTR_ERR(keyring);
+
+ mk->mk_users = keyring;
+ return 0;
+}
+
+/*
+ * Find the current user's key in the master key's ->mk_users.
+ * Returns ERR_PTR(-ENOKEY) if not found.
+ */
+static struct key *find_master_key_user(struct fscrypt_master_key *mk)
+{
+ char description[FSCRYPT_MK_USER_DESCRIPTION_SIZE];
+
+ format_mk_user_description(description, mk->mk_spec.u.identifier);
+ return search_fscrypt_keyring(mk->mk_users, &key_type_fscrypt_user,
+ description);
+}
+
+/*
+ * Give the current user a key in ->mk_users. This charges the user's quota and
+ * marks the master key as added by the current user, so that it cannot be
+ * removed by another user with the key. Either the master key's key->sem must
+ * be held for write, or the master key must be still undergoing initialization.
+ */
+static int add_master_key_user(struct fscrypt_master_key *mk)
+{
+ char description[FSCRYPT_MK_USER_DESCRIPTION_SIZE];
+ struct key *mk_user;
+ int err;
+
+ format_mk_user_description(description, mk->mk_spec.u.identifier);
+ mk_user = key_alloc(&key_type_fscrypt_user, description,
+ current_fsuid(), current_gid(), current_cred(),
+ KEY_POS_SEARCH | KEY_USR_VIEW, 0, NULL);
+ if (IS_ERR(mk_user))
+ return PTR_ERR(mk_user);
+
+ err = key_instantiate_and_link(mk_user, NULL, 0, mk->mk_users, NULL);
+ key_put(mk_user);
+ return err;
+}
+
+/*
+ * Remove the current user's key from ->mk_users, if present.
+ * The master key's key->sem must be held for write.
+ */
+static int remove_master_key_user(struct fscrypt_master_key *mk)
+{
+ struct key *mk_user;
+ int err;
+
+ mk_user = find_master_key_user(mk);
+ if (IS_ERR(mk_user)) {
+ if (mk_user != ERR_PTR(-ENOKEY))
+ return PTR_ERR(mk_user);
+ return 0;
+ }
+ err = key_unlink(mk->mk_users, mk_user);
+ key_put(mk_user);
+ return err;
+}
+
/*
* Allocate a new fscrypt_master_key which contains the given secret, set it as
* the payload of a new 'struct key' of type fscrypt, and link the 'struct key'
@@ -210,11 +342,26 @@ static int add_new_master_key(struct fscrypt_master_key_secret *secret,
mk->mk_spec = *mk_spec;
move_master_key_secret(&mk->mk_secret, secret);
+ init_rwsem(&mk->mk_secret_sem);
refcount_set(&mk->mk_refcount, 1); /* secret is present */
INIT_LIST_HEAD(&mk->mk_decrypted_inodes);
spin_lock_init(&mk->mk_decrypted_inodes_lock);
+ if (mk_spec->type != FSCRYPT_KEY_SPEC_TYPE_DESCRIPTOR) {
+ err = allocate_master_key_users_keyring(mk);
+ if (err)
+ goto out_free_mk;
+ err = add_master_key_user(mk);
+ if (err)
+ goto out_free_mk;
+ }
+
+ /*
+ * Note that we don't charge this key to anyone's quota, since when
+ * ->mk_users is in use those keys are charged instead, and otherwise
+ * (when ->mk_users isn't in use) only root can add these keys.
+ */
format_mk_description(description, mk_spec);
key = key_alloc(&key_type_fscrypt, description,
GLOBAL_ROOT_UID, GLOBAL_ROOT_GID, current_cred(),
@@ -242,13 +389,45 @@ static int add_existing_master_key(struct fscrypt_master_key *mk,
struct fscrypt_master_key_secret *secret,
const struct fscrypt_key_specifier *mk_spec)
{
- if (is_master_key_secret_present(&mk->mk_secret))
- return 0;
+ struct key *mk_user;
+ bool rekey;
+ int err;
+
+ /*
+ * If the current user is already in ->mk_users, then there's nothing to
+ * do. (Not applicable for v1 policy keys, which have NULL ->mk_users.)
+ */
+ if (mk->mk_users) {
+ mk_user = find_master_key_user(mk);
+ if (mk_user != ERR_PTR(-ENOKEY)) {
+ if (IS_ERR(mk_user))
+ return PTR_ERR(mk_user);
+ key_put(mk_user);
+ return 0;
+ }
+ }
- if (!refcount_inc_not_zero(&mk->mk_refcount))
+ /* If we'll be re-adding ->mk_secret, try to take the reference. */
+ rekey = !is_master_key_secret_present(&mk->mk_secret);
+ if (rekey && !refcount_inc_not_zero(&mk->mk_refcount))
return KEY_DEAD;
- move_master_key_secret(&mk->mk_secret, secret);
+ /* Add the current user to ->mk_users, if applicable. */
+ if (mk->mk_users) {
+ err = add_master_key_user(mk);
+ if (err) {
+ if (rekey && refcount_dec_and_test(&mk->mk_refcount))
+ return KEY_DEAD;
+ return err;
+ }
+ }
+
+ /* Re-add the secret if needed. */
+ if (rekey) {
+ down_write(&mk->mk_secret_sem);
+ move_master_key_secret(&mk->mk_secret, secret);
+ up_write(&mk->mk_secret_sem);
+ }
return 0;
}
@@ -275,7 +454,7 @@ static int add_master_key(struct super_block *sb,
} else {
/*
* Found the key in ->s_master_keys. Re-add the secret if
- * needed.
+ * needed, and add the user to ->mk_users if needed.
*/
down_write(&key->sem);
err = add_existing_master_key(key->payload.data[0], secret,
@@ -297,6 +476,23 @@ static int add_master_key(struct super_block *sb,
/*
* Add a master encryption key to the filesystem, causing all files which were
* encrypted with it to appear "unlocked" (decrypted) when accessed.
+ *
+ * When adding a key for use by v1 encryption policies, this ioctl is
+ * privileged, and userspace must provide the 'key_descriptor'.
+ *
+ * When adding a key for use by v2+ encryption policies, this ioctl is
+ * unprivileged. This is needed, in general, to allow non-root users to use
+ * encryption without encountering the visibility problems of process-subscribed
+ * keyrings and the inability to properly remove keys. This works by having
+ * each key identified by its cryptographically secure hash --- the
+ * 'key_identifier'. The cryptographic hash ensures that a malicious user
+ * cannot add the wrong key for a given identifier. Furthermore, each added key
+ * is charged to the appropriate user's quota for the keyrings service, which
+ * prevents a malicious user from adding too many keys. Finally, we forbid a
+ * user from removing a key while other users have added it too, which prevents
+ * a user who knows another user's key from causing a denial-of-service by
+ * removing it at an inopportune time. (We tolerate that a user who knows a key
+ * can prevent other users from removing it.)
*/
int fscrypt_ioctl_add_key(struct file *filp, void __user *_uarg)
{
@@ -325,11 +521,16 @@ int fscrypt_ioctl_add_key(struct file *filp, void __user *_uarg)
if (copy_from_user(secret.raw, uarg->raw, secret.size))
goto out_wipe_secret;
- err = -EACCES;
- if (!capable(CAP_SYS_ADMIN))
- goto out_wipe_secret;
-
- if (arg.key_spec.type != FSCRYPT_KEY_SPEC_TYPE_DESCRIPTOR) {
+ if (arg.key_spec.type == FSCRYPT_KEY_SPEC_TYPE_DESCRIPTOR) {
+ /*
+ * Only root can add keys that are identified by an arbitrary
+ * descriptor rather than by a cryptographic hash --- since
+ * otherwise a malicious user could add the wrong key.
+ */
+ err = -EACCES;
+ if (!capable(CAP_SYS_ADMIN))
+ goto out_wipe_secret;
+ } else {
err = fscrypt_init_hkdf(&secret.hkdf, secret.raw, secret.size);
if (err)
goto out_wipe_secret;
@@ -481,10 +682,26 @@ int fscrypt_ioctl_remove_key(struct file *filp, const void __user *uarg)
if (!valid_key_spec(&arg.key_spec))
return -EINVAL;
+ if (arg.flags & ~FSCRYPT_REMOVE_KEY_FLAG_ALL_USERS)
+ return -EINVAL;
+
if (memchr_inv(arg.__reserved, 0, sizeof(arg.__reserved)))
return -EINVAL;
- if (!capable(CAP_SYS_ADMIN))
+ /*
+ * Only root can request that the key be removed no matter how many
+ * users have added it.
+ */
+ if ((arg.flags & FSCRYPT_REMOVE_KEY_FLAG_ALL_USERS) &&
+ !capable(CAP_SYS_ADMIN))
+ return -EACCES;
+
+ /*
+ * Only root can add and remove keys that are identified by an arbitrary
+ * descriptor rather than by a cryptographic hash.
+ */
+ if (arg.key_spec.type == FSCRYPT_KEY_SPEC_TYPE_DESCRIPTOR &&
+ !capable(CAP_SYS_ADMIN))
return -EACCES;
/* Find the key being removed. */
@@ -495,11 +712,35 @@ int fscrypt_ioctl_remove_key(struct file *filp, const void __user *uarg)
down_write(&key->sem);
- /* Wipe the secret. */
+ /* If relevant, remove current user's (or all users) usage of the key */
+ if (mk->mk_users && mk->mk_users->keys.nr_leaves_on_tree != 0) {
+ if (arg.flags & FSCRYPT_REMOVE_KEY_FLAG_ALL_USERS)
+ err = keyring_clear(mk->mk_users);
+ else
+ err = remove_master_key_user(mk);
+ if (err) {
+ up_write(&key->sem);
+ goto out_put_key;
+ }
+ if (mk->mk_users->keys.nr_leaves_on_tree != 0) {
+ /*
+ * Other users have still added the key too. We removed
+ * the current user's usage of the key if there was one,
+ * but we still can't remove the key itself.
+ */
+ err = -EUSERS;
+ up_write(&key->sem);
+ goto out_put_key;
+ }
+ }
+
+ /* No usages remaining. Go ahead and wipe the secret. */
dead = false;
if (is_master_key_secret_present(&mk->mk_secret)) {
+ down_write(&mk->mk_secret_sem);
wipe_master_key_secret(&mk->mk_secret);
dead = refcount_dec_and_test(&mk->mk_refcount);
+ up_write(&mk->mk_secret_sem);
}
up_write(&key->sem);
if (dead) {
@@ -513,6 +754,7 @@ int fscrypt_ioctl_remove_key(struct file *filp, const void __user *uarg)
/* Some inodes still reference this key; try to evict them. */
err = try_to_lock_encrypted_files(sb, mk);
}
+out_put_key:
key_put(key);
return err;
}
@@ -528,6 +770,15 @@ EXPORT_SYMBOL_GPL(fscrypt_ioctl_remove_key);
* of an encrypted directory without using a hack such as trying to open a
* regular file in it (which can confuse the "incompletely removed" state with
* absent or present).
+ *
+ * In addition, for v2 policy keys we allow applications to determine, via
+ * ->status_flags and ->user_count, whether the key has been added by the
+ * current user, by other users, or by both. Most applications should not need
+ * this, since ordinarily only one user should know a given key. However, if a
+ * secret key is shared by multiple users, applications may wish to add an
+ * already-present key to prevent other users from removing it. This ioctl can
+ * be used to check whether that really is the case before the work is done to
+ * add the key --- which might e.g. require prompting the user for a passphrase.
*/
int fscrypt_ioctl_get_key_status(struct file *filp, void __user *uarg)
{
@@ -546,6 +797,8 @@ int fscrypt_ioctl_get_key_status(struct file *filp, void __user *uarg)
if (memchr_inv(arg.__reserved, 0, sizeof(arg.__reserved)))
return -EINVAL;
+ arg.status_flags = 0;
+ arg.user_count = 0;
memset(arg.__out_reserved, 0, sizeof(arg.__out_reserved));
key = fscrypt_find_master_key(sb, &arg.key_spec);
@@ -566,6 +819,20 @@ int fscrypt_ioctl_get_key_status(struct file *filp, void __user *uarg)
}
arg.status = FSCRYPT_KEY_STATUS_PRESENT;
+ if (mk->mk_users) {
+ struct key *mk_user;
+
+ arg.user_count = mk->mk_users->keys.nr_leaves_on_tree;
+ mk_user = find_master_key_user(mk);
+ if (!IS_ERR(mk_user)) {
+ arg.status_flags |=
+ FSCRYPT_KEY_STATUS_FLAG_ADDED_BY_SELF;
+ key_put(mk_user);
+ } else if (mk_user != ERR_PTR(-ENOKEY)) {
+ err = PTR_ERR(mk_user);
+ goto out_release_key;
+ }
+ }
err = 0;
out_release_key:
up_read(&key->sem);
@@ -579,10 +846,25 @@ EXPORT_SYMBOL_GPL(fscrypt_ioctl_get_key_status);
int __init fscrypt_init_keyring(void)
{
- return register_key_type(&key_type_fscrypt);
+ int err;
+
+ err = register_key_type(&key_type_fscrypt);
+ if (err)
+ return err;
+
+ err = register_key_type(&key_type_fscrypt_user);
+ if (err)
+ goto err_unregister_fscrypt;
+
+ return 0;
+
+err_unregister_fscrypt:
+ unregister_key_type(&key_type_fscrypt);
+ return err;
}
void fscrypt_exit_keyring(void)
{
unregister_key_type(&key_type_fscrypt);
+ unregister_key_type(&key_type_fscrypt_user);
}
diff --git a/fs/crypto/keysetup.c b/fs/crypto/keysetup.c
index c81447a438ea9..6b6673355538c 100644
--- a/fs/crypto/keysetup.c
+++ b/fs/crypto/keysetup.c
@@ -283,10 +283,10 @@ static int fscrypt_setup_v2_file_key(struct fscrypt_info *ci,
*
* If the master key is found in the filesystem-level keyring, then the
* corresponding 'struct key' is returned in *master_key_ret with
- * ->sem read-locked. This is needed to ensure that only one task links the
- * fscrypt_info into ->mk_decrypted_inodes (as multiple tasks may race to create
- * an fscrypt_info for the same inode), and to synchronize the key being removed
- * with a new inode starting to use it.
+ * ->mk_secret_sem read-locked. This is needed to ensure that only one task
+ * links the fscrypt_info into ->mk_decrypted_inodes (as multiple tasks may race
+ * to create an fscrypt_info for the same inode), and to synchronize the key
+ * being removed with a new inode starting to use it.
*/
static int setup_file_encryption_key(struct fscrypt_info *ci,
struct key **master_key_ret)
@@ -324,7 +324,7 @@ static int setup_file_encryption_key(struct fscrypt_info *ci,
}
mk = key->payload.data[0];
- down_read(&key->sem);
+ down_read(&mk->mk_secret_sem);
/* Has the secret been removed (via FS_IOC_REMOVE_ENCRYPTION_KEY)? */
if (!is_master_key_secret_present(&mk->mk_secret)) {
@@ -358,7 +358,7 @@ static int setup_file_encryption_key(struct fscrypt_info *ci,
return 0;
out_release_key:
- up_read(&key->sem);
+ up_read(&mk->mk_secret_sem);
key_put(key);
return err;
}
@@ -484,7 +484,9 @@ int fscrypt_get_encryption_info(struct inode *inode)
res = 0;
out:
if (master_key) {
- up_read(&master_key->sem);
+ struct fscrypt_master_key *mk = master_key->payload.data[0];
+
+ up_read(&mk->mk_secret_sem);
key_put(master_key);
}
if (res == -ENOKEY)
@@ -526,7 +528,7 @@ int fscrypt_drop_inode(struct inode *inode)
mk = ci->ci_master_key->payload.data[0];
/*
- * Note: since we aren't holding key->sem, the result here can
+ * Note: since we aren't holding ->mk_secret_sem, the result here can
* immediately become outdated. But there's no correctness problem with
* unnecessarily evicting. Nor is there a correctness problem with not
* evicting while iput() is racing with the key being removed, since
diff --git a/include/uapi/linux/fscrypt.h b/include/uapi/linux/fscrypt.h
index 26879e669dfa0..fd1c3e2b0b7c1 100644
--- a/include/uapi/linux/fscrypt.h
+++ b/include/uapi/linux/fscrypt.h
@@ -103,7 +103,9 @@ struct fscrypt_add_key_arg {
/* Struct passed to FS_IOC_REMOVE_ENCRYPTION_KEY */
struct fscrypt_remove_key_arg {
struct fscrypt_key_specifier key_spec;
- __u32 __reserved[6];
+#define FSCRYPT_REMOVE_KEY_FLAG_ALL_USERS 0x00000001
+ __u32 flags;
+ __u32 __reserved[5];
};
/* Struct passed to FS_IOC_GET_ENCRYPTION_KEY_STATUS */
@@ -117,7 +119,10 @@ struct fscrypt_get_key_status_arg {
#define FSCRYPT_KEY_STATUS_PRESENT 2
#define FSCRYPT_KEY_STATUS_INCOMPLETELY_REMOVED 3
__u32 status;
- __u32 __out_reserved[15];
+#define FSCRYPT_KEY_STATUS_FLAG_ADDED_BY_SELF 0x00000001
+ __u32 status_flags;
+ __u32 user_count;
+ __u32 __out_reserved[13];
};
#define FS_IOC_SET_ENCRYPTION_POLICY _IOR('f', 19, struct fscrypt_policy)
--
2.21.0
^ permalink raw reply related
* [PATCH v4 11/17] fscrypt: v2 encryption policy support
From: Eric Biggers @ 2019-04-02 15:45 UTC (permalink / raw)
To: linux-fscrypt
Cc: Satya Tangirala, linux-api, linux-f2fs-devel, keyrings, linux-mtd,
linux-crypto, linux-fsdevel, linux-ext4, Paul Crowley
In-Reply-To: <20190402154600.32432-1-ebiggers@kernel.org>
From: Eric Biggers <ebiggers@google.com>
Add a new fscrypt policy version, "v2". It has the following changes
from the original policy version, which we call "v1" (*):
- Master keys (the user-provided encryption keys) are only ever used as
input to HKDF-SHA512. This is more flexible and less error-prone, and
it avoids the quirks and limitations of the AES-128-ECB based KDF.
Three classes of cryptographically isolated subkeys are defined:
- Per-file keys, like used in v1 policies except for the new KDF.
- Per-mode keys. These implement the semantics of the DIRECT_KEY
flag, which for v1 policies made the master key be used directly.
- Key identifiers (see below).
- Each master key is identified by a 16-byte master_key_identifier,
which is derived from the key itself using HKDF-SHA512. This prevents
users from associating the wrong key with an encrypted file or
directory. This was easily possible with v1 policies, which
identified the key by an arbitrary 8-byte master_key_descriptor.
- The key must be provided in the filesystem-level keyring, not in a
process-subscribed keyring.
The following UAPI additions are made:
- The existing ioctl FS_IOC_SET_ENCRYPTION_POLICY can now be passed a
fscrypt_policy_v2 to set a v2 encryption policy. It's disambiguated
from fscrypt_policy/fscrypt_policy_v1 by the version code prefix.
- A new ioctl FS_IOC_GET_ENCRYPTION_POLICY_EX is added. It allows
getting the v1 or v2 encryption policy of an encrypted file or
directory. The existing FS_IOC_GET_ENCRYPTION_POLICY ioctl could not
be used because it did not have a way for userspace to indicate which
policy structure is expected. The new ioctl includes a size field, so
it is extensible to future fscrypt policy versions.
- The ioctls FS_IOC_ADD_ENCRYPTION_KEY, FS_IOC_REMOVE_ENCRYPTION_KEY,
and FS_IOC_GET_ENCRYPTION_KEY_STATUS now support managing keys for v2
encryption policies. Such keys are kept logically separate from keys
for v1 encryption policies, and are identified by 'identifier' rather
than by 'descriptor'. The 'identifier' need not be provided when
adding a key, since the kernel will calculate it anyway.
This patch temporarily keeps adding/removing v2 policy keys behind the
same permission check done for adding/removing v1 policy keys:
capable(CAP_SYS_ADMIN). However, the next patch will carefully take
advantage of the cryptographically secure master_key_identifier to allow
non-root users to add/remove v2 policy keys, thus providing a full
replacement for v1 policies.
(*) Actually, in the API fscrypt_policy::version is 0 while on-disk
fscrypt_context::format is 1. But I believe it makes the most sense
to advance both to '2' to have them be in sync, and to consider the
numbering to start at 1 except for the API quirk.
Signed-off-by: Eric Biggers <ebiggers@google.com>
---
fs/crypto/crypto.c | 3 +-
fs/crypto/fname.c | 3 +-
fs/crypto/fscrypt_private.h | 182 ++++++++++++++---
fs/crypto/keyring.c | 37 +++-
fs/crypto/keysetup.c | 174 ++++++++++++----
fs/crypto/keysetup_legacy.c | 18 +-
fs/crypto/policy.c | 382 +++++++++++++++++++++++++----------
include/linux/fscrypt.h | 9 +-
include/uapi/linux/fscrypt.h | 42 +++-
9 files changed, 667 insertions(+), 183 deletions(-)
diff --git a/fs/crypto/crypto.c b/fs/crypto/crypto.c
index 8726c4e940773..7abc6695fa4d9 100644
--- a/fs/crypto/crypto.c
+++ b/fs/crypto/crypto.c
@@ -26,7 +26,6 @@
#include <linux/ratelimit.h>
#include <linux/dcache.h>
#include <linux/namei.h>
-#include <crypto/aes.h>
#include <crypto/skcipher.h>
#include "fscrypt_private.h"
@@ -139,7 +138,7 @@ void fscrypt_generate_iv(union fscrypt_iv *iv, u64 lblk_num,
memset(iv, 0, ci->ci_mode->ivsize);
iv->lblk_num = cpu_to_le64(lblk_num);
- if (ci->ci_flags & FSCRYPT_POLICY_FLAG_DIRECT_KEY)
+ if (fscrypt_is_direct_key_policy(&ci->ci_policy))
memcpy(iv->nonce, ci->ci_nonce, FS_KEY_DERIVATION_NONCE_SIZE);
if (ci->ci_essiv_tfm != NULL)
diff --git a/fs/crypto/fname.c b/fs/crypto/fname.c
index 37c26ad36d0e3..57e0caba9d126 100644
--- a/fs/crypto/fname.c
+++ b/fs/crypto/fname.c
@@ -186,7 +186,8 @@ static int digest_decode(const char *src, int len, char *dst)
bool fscrypt_fname_encrypted_size(const struct inode *inode, u32 orig_len,
u32 max_len, u32 *encrypted_len_ret)
{
- int padding = 4 << (inode->i_crypt_info->ci_flags &
+ const struct fscrypt_info *ci = inode->i_crypt_info;
+ int padding = 4 << (fscrypt_policy_flags(&ci->ci_policy) &
FSCRYPT_POLICY_FLAGS_PAD_MASK);
u32 encrypted_len;
diff --git a/fs/crypto/fscrypt_private.h b/fs/crypto/fscrypt_private.h
index e2a65189eb578..9591f5fd72b33 100644
--- a/fs/crypto/fscrypt_private.h
+++ b/fs/crypto/fscrypt_private.h
@@ -20,27 +20,127 @@
#define FSCRYPT_MIN_KEY_SIZE 16
-/**
- * Encryption context for inode
- *
- * Protector format:
- * 1 byte: Protector format (1 = this version)
- * 1 byte: File contents encryption mode
- * 1 byte: File names encryption mode
- * 1 byte: Flags
- * 8 bytes: Master Key descriptor
- * 16 bytes: Encryption Key derivation nonce
- */
-struct fscrypt_context {
- u8 format;
+#define FSCRYPT_CONTEXT_V1 1
+#define FSCRYPT_CONTEXT_V2 2
+
+struct fscrypt_context_v1 {
+ u8 version; /* FSCRYPT_CONTEXT_V1 */
u8 contents_encryption_mode;
u8 filenames_encryption_mode;
u8 flags;
u8 master_key_descriptor[FSCRYPT_KEY_DESCRIPTOR_SIZE];
u8 nonce[FS_KEY_DERIVATION_NONCE_SIZE];
-} __packed;
+};
+
+struct fscrypt_context_v2 {
+ u8 version; /* FSCRYPT_CONTEXT_V2 */
+ u8 contents_encryption_mode;
+ u8 filenames_encryption_mode;
+ u8 flags;
+ u8 __reserved[4];
+ u8 master_key_identifier[FSCRYPT_KEY_IDENTIFIER_SIZE];
+ u8 nonce[FS_KEY_DERIVATION_NONCE_SIZE];
+};
+
+/**
+ * fscrypt_context - the encryption context of an inode
+ *
+ * This is the on-disk equivalent of an fscrypt_policy, stored alongside each
+ * encrypted file usually in a hidden extended attribute. It contains the
+ * fields from the fscrypt_policy, in order to identify the encryption algorithm
+ * and key with which the file is encrypted. It also contains a nonce that was
+ * randomly generated by fscrypt itself; this is used as KDF input or as a tweak
+ * to cause different files to be encrypted differently.
+ */
+union fscrypt_context {
+ u8 version;
+ struct fscrypt_context_v1 v1;
+ struct fscrypt_context_v2 v2;
+};
+
+/*
+ * Return the size expected for the given fscrypt_context based on its version
+ * number, or 0 if the context version is unrecognized.
+ */
+static inline int fscrypt_context_size(const union fscrypt_context *ctx)
+{
+ switch (ctx->version) {
+ case FSCRYPT_CONTEXT_V1:
+ BUILD_BUG_ON(sizeof(ctx->v1) != 28);
+ return sizeof(ctx->v1);
+ case FSCRYPT_CONTEXT_V2:
+ BUILD_BUG_ON(sizeof(ctx->v2) != 40);
+ return sizeof(ctx->v2);
+ }
+ return 0;
+}
+
+#undef fscrypt_policy
+union fscrypt_policy {
+ __u8 version;
+ struct fscrypt_policy_v1 v1;
+ struct fscrypt_policy_v2 v2;
+};
+
+/*
+ * Return the size expected for the given fscrypt_policy based on its version
+ * number, or 0 if the policy version is unrecognized.
+ */
+static inline int fscrypt_policy_size(const union fscrypt_policy *policy)
+{
+ switch (policy->version) {
+ case FSCRYPT_POLICY_V1:
+ return sizeof(policy->v1);
+ case FSCRYPT_POLICY_V2:
+ return sizeof(policy->v2);
+ }
+ return 0;
+}
-#define FS_ENCRYPTION_CONTEXT_FORMAT_V1 1
+/* Return the contents encryption mode of a valid encryption policy */
+static inline u8
+fscrypt_policy_contents_mode(const union fscrypt_policy *policy)
+{
+ switch (policy->version) {
+ case FSCRYPT_POLICY_V1:
+ return policy->v1.contents_encryption_mode;
+ case FSCRYPT_POLICY_V2:
+ return policy->v2.contents_encryption_mode;
+ }
+ BUG();
+}
+
+/* Return the filenames encryption mode of a valid encryption policy */
+static inline u8
+fscrypt_policy_fnames_mode(const union fscrypt_policy *policy)
+{
+ switch (policy->version) {
+ case FSCRYPT_POLICY_V1:
+ return policy->v1.filenames_encryption_mode;
+ case FSCRYPT_POLICY_V2:
+ return policy->v2.filenames_encryption_mode;
+ }
+ BUG();
+}
+
+/* Return the flags (FSCRYPT_POLICY_FLAG*) of a valid encryption policy */
+static inline u8
+fscrypt_policy_flags(const union fscrypt_policy *policy)
+{
+ switch (policy->version) {
+ case FSCRYPT_POLICY_V1:
+ return policy->v1.flags;
+ case FSCRYPT_POLICY_V2:
+ return policy->v2.flags;
+ }
+ BUG();
+}
+
+static inline bool
+fscrypt_is_direct_key_policy(const union fscrypt_policy *policy)
+{
+ return fscrypt_policy_flags(policy) & FSCRYPT_POLICY_FLAG_DIRECT_KEY;
+}
/**
* For encrypted symlinks, the ciphertext length is stored at the beginning
@@ -70,8 +170,8 @@ struct fscrypt_info {
struct crypto_cipher *ci_essiv_tfm;
/*
- * Encryption mode used for this inode. It corresponds to either
- * ci_data_mode or ci_filename_mode, depending on the inode type.
+ * Encryption mode used for this inode. It corresponds to either the
+ * contents or filenames encryption mode, depending on the inode type.
*/
struct fscrypt_mode *ci_mode;
@@ -97,11 +197,10 @@ struct fscrypt_info {
*/
struct fscrypt_direct_key *ci_direct_key;
- /* fields from the fscrypt_context */
- u8 ci_data_mode;
- u8 ci_filename_mode;
- u8 ci_flags;
- u8 ci_master_key_descriptor[FSCRYPT_KEY_DESCRIPTOR_SIZE];
+ /* The encryption policy used by this inode */
+ union fscrypt_policy ci_policy;
+
+ /* This inode's nonce, copied from the fscrypt_context */
u8 ci_nonce[FS_KEY_DERIVATION_NONCE_SIZE];
};
@@ -184,6 +283,17 @@ struct fscrypt_hkdf {
extern int fscrypt_init_hkdf(struct fscrypt_hkdf *hkdf, const u8 *master_key,
unsigned int master_key_size);
+/*
+ * The list of contexts in which fscrypt uses HKDF. These values are used as
+ * the first byte of the HKDF application-specific info string to guarantee that
+ * info strings are never repeated between contexts. This ensures that all HKDF
+ * outputs are unique and cryptographically isolated, i.e. knowledge of one
+ * output doesn't reveal another.
+ */
+#define HKDF_CONTEXT_KEY_IDENTIFIER 1
+#define HKDF_CONTEXT_PER_FILE_KEY 2
+#define HKDF_CONTEXT_PER_MODE_KEY 3
+
extern int fscrypt_hkdf_expand(struct fscrypt_hkdf *hkdf, u8 context,
const u8 *info, unsigned int infolen,
u8 *okm, unsigned int okmlen);
@@ -197,10 +307,16 @@ extern void fscrypt_destroy_hkdf(struct fscrypt_hkdf *hkdf);
*/
struct fscrypt_master_key_secret {
- /* Size of the raw key in bytes */
+ /*
+ * For v2 policy keys: HKDF context keyed by this master key.
+ * For v1 policy keys: not set (hkdf.hmac_tfm == NULL).
+ */
+ struct fscrypt_hkdf hkdf;
+
+ /* Size of the raw key in bytes. Set even if ->raw isn't set. */
u32 size;
- /* The raw key */
+ /* For v1 policy keys: the raw key. Wiped for v2 policy keys. */
u8 raw[FSCRYPT_MAX_KEY_SIZE];
} __randomize_layout;
@@ -226,7 +342,12 @@ struct fscrypt_master_key {
*/
struct fscrypt_master_key_secret mk_secret;
- /* Arbitrary key descriptor which was assigned by userspace */
+ /*
+ * For v1 policy keys: an arbitrary key descriptor which was assigned by
+ * userspace (->descriptor).
+ *
+ * For v2 policy keys: a cryptographic hash of this key (->identifier).
+ */
struct fscrypt_key_specifier mk_spec;
/*
@@ -245,6 +366,9 @@ struct fscrypt_master_key {
struct list_head mk_decrypted_inodes;
spinlock_t mk_decrypted_inodes_lock;
+ /* Per-mode tfms for DIRECT_KEY policies, allocated on-demand */
+ struct crypto_skcipher *mk_mode_keys[__FSCRYPT_MODE_MAX + 1];
+
} __randomize_layout;
static inline bool
@@ -302,5 +426,13 @@ extern int fscrypt_setup_v1_file_key(struct fscrypt_info *ci,
extern int fscrypt_setup_v1_file_key_via_subscribed_keyrings(
struct fscrypt_info *ci);
+/* policy.c */
+
+extern bool fscrypt_policies_equal(const union fscrypt_policy *policy1,
+ const union fscrypt_policy *policy2);
+extern bool fscrypt_supported_policy(const union fscrypt_policy *policy_u);
+extern int fscrypt_policy_from_context(union fscrypt_policy *policy_u,
+ const union fscrypt_context *ctx_u,
+ int ctx_size);
#endif /* _FSCRYPT_PRIVATE_H */
diff --git a/fs/crypto/keyring.c b/fs/crypto/keyring.c
index 08e14497c8e30..fe1b003975c4f 100644
--- a/fs/crypto/keyring.c
+++ b/fs/crypto/keyring.c
@@ -14,6 +14,7 @@
* - FS_IOC_GET_ENCRYPTION_KEY_STATUS: get key status
*/
+#include <crypto/skcipher.h>
#include <linux/key-type.h>
#include <linux/seq_file.h>
@@ -21,6 +22,7 @@
static void wipe_master_key_secret(struct fscrypt_master_key_secret *secret)
{
+ fscrypt_destroy_hkdf(&secret->hkdf);
memzero_explicit(secret, sizeof(*secret));
}
@@ -33,7 +35,13 @@ static void move_master_key_secret(struct fscrypt_master_key_secret *dst,
static void free_master_key(struct fscrypt_master_key *mk)
{
+ size_t i;
+
wipe_master_key_secret(&mk->mk_secret);
+
+ for (i = 0; i < ARRAY_SIZE(mk->mk_mode_keys); i++)
+ crypto_free_skcipher(mk->mk_mode_keys[i]);
+
kzfree(mk);
}
@@ -42,6 +50,8 @@ static inline int master_key_spec_len(const struct fscrypt_key_specifier *spec)
switch (spec->type) {
case FSCRYPT_KEY_SPEC_TYPE_DESCRIPTOR:
return FSCRYPT_KEY_DESCRIPTOR_SIZE;
+ case FSCRYPT_KEY_SPEC_TYPE_IDENTIFIER:
+ return FSCRYPT_KEY_IDENTIFIER_SIZE;
}
return 0;
}
@@ -115,7 +125,7 @@ static struct key *search_fscrypt_keyring(struct key *keyring,
#define FSCRYPT_FS_KEYRING_DESCRIPTION_SIZE \
(CONST_STRLEN("fscrypt-") + FIELD_SIZEOF(struct super_block, s_id))
-#define FSCRYPT_MK_DESCRIPTION_SIZE (2 * FSCRYPT_KEY_DESCRIPTOR_SIZE + 1)
+#define FSCRYPT_MK_DESCRIPTION_SIZE (2 * FSCRYPT_KEY_IDENTIFIER_SIZE + 1)
static void format_fs_keyring_description(
char description[FSCRYPT_FS_KEYRING_DESCRIPTION_SIZE],
@@ -319,6 +329,31 @@ int fscrypt_ioctl_add_key(struct file *filp, void __user *_uarg)
if (!capable(CAP_SYS_ADMIN))
goto out_wipe_secret;
+ if (arg.key_spec.type != FSCRYPT_KEY_SPEC_TYPE_DESCRIPTOR) {
+ err = fscrypt_init_hkdf(&secret.hkdf, secret.raw, secret.size);
+ if (err)
+ goto out_wipe_secret;
+
+ /*
+ * Now that the HKDF context is initialized, the raw key is no
+ * longer needed.
+ */
+ memzero_explicit(secret.raw, sizeof(secret.raw));
+
+ /* Calculate the key identifier and return it to userspace. */
+ err = fscrypt_hkdf_expand(&secret.hkdf,
+ HKDF_CONTEXT_KEY_IDENTIFIER,
+ NULL, 0, arg.key_spec.u.identifier,
+ FSCRYPT_KEY_IDENTIFIER_SIZE);
+ if (err)
+ goto out_wipe_secret;
+ err = -EFAULT;
+ if (copy_to_user(uarg->key_spec.u.identifier,
+ arg.key_spec.u.identifier,
+ FSCRYPT_KEY_IDENTIFIER_SIZE))
+ goto out_wipe_secret;
+ }
+
err = add_master_key(sb, &secret, &arg.key_spec);
out_wipe_secret:
wipe_master_key_secret(&secret);
diff --git a/fs/crypto/keysetup.c b/fs/crypto/keysetup.c
index f8fb115c63ab9..c81447a438ea9 100644
--- a/fs/crypto/keysetup.c
+++ b/fs/crypto/keysetup.c
@@ -52,21 +52,14 @@ static struct fscrypt_mode available_modes[] = {
};
static struct fscrypt_mode *
-select_encryption_mode(const struct fscrypt_info *ci, const struct inode *inode)
+select_encryption_mode(const union fscrypt_policy *policy,
+ const struct inode *inode)
{
- if (!fscrypt_valid_enc_modes(ci->ci_data_mode, ci->ci_filename_mode)) {
- fscrypt_warn(inode->i_sb,
- "inode %lu uses unsupported encryption modes (contents mode %d, filenames mode %d)",
- inode->i_ino, ci->ci_data_mode,
- ci->ci_filename_mode);
- return ERR_PTR(-EINVAL);
- }
-
if (S_ISREG(inode->i_mode))
- return &available_modes[ci->ci_data_mode];
+ return &available_modes[fscrypt_policy_contents_mode(policy)];
if (S_ISDIR(inode->i_mode) || S_ISLNK(inode->i_mode))
- return &available_modes[ci->ci_filename_mode];
+ return &available_modes[fscrypt_policy_fnames_mode(policy)];
WARN_ONCE(1, "fscrypt: filesystem tried to load encryption info for inode %lu, which is not encryptable (file type %d)\n",
inode->i_ino, (inode->i_mode & S_IFMT));
@@ -208,6 +201,83 @@ int fscrypt_set_derived_key(struct fscrypt_info *ci, const u8 *derived_key)
return 0;
}
+static int setup_per_mode_key(struct fscrypt_info *ci,
+ struct fscrypt_master_key *mk)
+{
+ u8 mode_num = ci->ci_mode - available_modes;
+ struct crypto_skcipher **tfm_p;
+ struct crypto_skcipher *tfm;
+ u8 mode_key[FSCRYPT_MAX_KEY_SIZE];
+ int err;
+
+ if (WARN_ON(mode_num >= ARRAY_SIZE(mk->mk_mode_keys)))
+ return -EINVAL;
+ tfm_p = &mk->mk_mode_keys[mode_num];
+retry:
+ /* pairs with cmpxchg_release() below */
+ tfm = smp_load_acquire(tfm_p);
+ if (likely(tfm != NULL))
+ goto done;
+
+ BUILD_BUG_ON(sizeof(mode_num) != 1);
+ err = fscrypt_hkdf_expand(&mk->mk_secret.hkdf,
+ HKDF_CONTEXT_PER_MODE_KEY,
+ &mode_num, sizeof(mode_num),
+ mode_key, ci->ci_mode->keysize);
+ if (err)
+ return err;
+ tfm = fscrypt_allocate_skcipher(ci->ci_mode, mode_key, ci->ci_inode);
+ memzero_explicit(mode_key, ci->ci_mode->keysize);
+ if (IS_ERR(tfm))
+ return PTR_ERR(tfm);
+
+ /* pairs with smp_load_acquire() above */
+ if (cmpxchg_release(tfm_p, NULL, tfm) != NULL) {
+ crypto_free_skcipher(tfm);
+ goto retry;
+ }
+done:
+ ci->ci_ctfm = tfm;
+ return 0;
+}
+
+static int fscrypt_setup_v2_file_key(struct fscrypt_info *ci,
+ struct fscrypt_master_key *mk)
+{
+ u8 derived_key[FSCRYPT_MAX_KEY_SIZE];
+ int err;
+
+ if (ci->ci_policy.v2.flags & FSCRYPT_POLICY_FLAG_DIRECT_KEY) {
+ /*
+ * DIRECT_KEY: instead of deriving per-file keys, the per-file
+ * nonce will be included in all the IVs. But unlike v1
+ * policies, for v2 policies in this case we don't encrypt with
+ * the master key directly but rather derive a per-mode key.
+ * This ensures that the master key is consistently used only
+ * for HKDF, avoiding key reuse issues.
+ */
+ if (!fscrypt_mode_supports_direct_key(ci->ci_mode)) {
+ fscrypt_warn(ci->ci_inode->i_sb,
+ "direct key flag not allowed with %s",
+ ci->ci_mode->friendly_name);
+ return -EINVAL;
+ }
+ return setup_per_mode_key(ci, mk);
+ }
+
+ err = fscrypt_hkdf_expand(&mk->mk_secret.hkdf,
+ HKDF_CONTEXT_PER_FILE_KEY,
+ ci->ci_nonce,
+ FS_KEY_DERIVATION_NONCE_SIZE,
+ derived_key, ci->ci_mode->keysize);
+ if (err)
+ return err;
+
+ err = fscrypt_set_derived_key(ci, derived_key);
+ memzero_explicit(derived_key, ci->ci_mode->keysize);
+ return err;
+}
+
/*
* Find the master key, then set up the inode's actual encryption key.
*
@@ -226,15 +296,30 @@ static int setup_file_encryption_key(struct fscrypt_info *ci,
struct fscrypt_key_specifier mk_spec;
int err;
- mk_spec.type = FSCRYPT_KEY_SPEC_TYPE_DESCRIPTOR;
- memcpy(mk_spec.u.descriptor, ci->ci_master_key_descriptor,
- FSCRYPT_KEY_DESCRIPTOR_SIZE);
+ if (ci->ci_policy.version == FSCRYPT_POLICY_V1) {
+ mk_spec.type = FSCRYPT_KEY_SPEC_TYPE_DESCRIPTOR;
+ memcpy(mk_spec.u.descriptor,
+ ci->ci_policy.v1.master_key_descriptor,
+ FSCRYPT_KEY_DESCRIPTOR_SIZE);
+ } else {
+ mk_spec.type = FSCRYPT_KEY_SPEC_TYPE_IDENTIFIER;
+ memcpy(mk_spec.u.identifier,
+ ci->ci_policy.v2.master_key_identifier,
+ FSCRYPT_KEY_IDENTIFIER_SIZE);
+ }
key = fscrypt_find_master_key(ci->ci_inode->i_sb, &mk_spec);
if (IS_ERR(key)) {
- if (key != ERR_PTR(-ENOKEY))
+ if (key != ERR_PTR(-ENOKEY) ||
+ ci->ci_policy.version != FSCRYPT_POLICY_V1)
return PTR_ERR(key);
+ /*
+ * As a legacy fallback for v1 policies, search for the key in
+ * the current task's subscribed keyrings too. Don't move this
+ * to before the search of ->s_master_keys, since users
+ * shouldn't be able to override filesystem-level keys.
+ */
return fscrypt_setup_v1_file_key_via_subscribed_keyrings(ci);
}
@@ -247,6 +332,12 @@ static int setup_file_encryption_key(struct fscrypt_info *ci,
goto out_release_key;
}
+ /*
+ * Require that the master key be at least as long as the derived key.
+ * Otherwise, the derived key cannot possibly contain as much entropy as
+ * that required by the encryption mode it will be used for. For v1
+ * policies it's also required for the KDF to work at all.
+ */
if (mk->mk_secret.size < ci->ci_mode->keysize) {
fscrypt_warn(NULL,
"key with description '%s' is too short (got %u bytes, need %u+ bytes)",
@@ -256,7 +347,10 @@ static int setup_file_encryption_key(struct fscrypt_info *ci,
goto out_release_key;
}
- err = fscrypt_setup_v1_file_key(ci, mk->mk_secret.raw);
+ if (ci->ci_policy.version == FSCRYPT_POLICY_V1)
+ err = fscrypt_setup_v1_file_key(ci, mk->mk_secret.raw);
+ else
+ err = fscrypt_setup_v2_file_key(ci, mk);
if (err)
goto out_release_key;
@@ -278,7 +372,8 @@ static void put_crypt_info(struct fscrypt_info *ci)
if (ci->ci_direct_key) {
fscrypt_put_direct_key(ci->ci_direct_key);
- } else {
+ } else if ((ci->ci_ctfm != NULL || ci->ci_essiv_tfm != NULL) &&
+ !fscrypt_is_direct_key_policy(&ci->ci_policy)) {
crypto_free_skcipher(ci->ci_ctfm);
crypto_free_cipher(ci->ci_essiv_tfm);
}
@@ -308,7 +403,7 @@ static void put_crypt_info(struct fscrypt_info *ci)
int fscrypt_get_encryption_info(struct inode *inode)
{
struct fscrypt_info *crypt_info;
- struct fscrypt_context ctx;
+ union fscrypt_context ctx;
struct fscrypt_mode *mode;
struct key *master_key = NULL;
int res;
@@ -327,35 +422,40 @@ int fscrypt_get_encryption_info(struct inode *inode)
return res;
/* Fake up a context for an unencrypted directory */
memset(&ctx, 0, sizeof(ctx));
- ctx.format = FS_ENCRYPTION_CONTEXT_FORMAT_V1;
- ctx.contents_encryption_mode = FSCRYPT_MODE_AES_256_XTS;
- ctx.filenames_encryption_mode = FSCRYPT_MODE_AES_256_CTS;
- memset(ctx.master_key_descriptor, 0x42,
+ ctx.version = FSCRYPT_CONTEXT_V1;
+ ctx.v1.contents_encryption_mode = FSCRYPT_MODE_AES_256_XTS;
+ ctx.v1.filenames_encryption_mode = FSCRYPT_MODE_AES_256_CTS;
+ memset(ctx.v1.master_key_descriptor, 0x42,
FSCRYPT_KEY_DESCRIPTOR_SIZE);
- } else if (res != sizeof(ctx)) {
- return -EINVAL;
+ res = sizeof(ctx.v1);
}
- if (ctx.format != FS_ENCRYPTION_CONTEXT_FORMAT_V1)
- return -EINVAL;
-
- if (ctx.flags & ~FSCRYPT_POLICY_FLAGS_VALID)
- return -EINVAL;
-
crypt_info = kmem_cache_zalloc(fscrypt_info_cachep, GFP_NOFS);
if (!crypt_info)
return -ENOMEM;
crypt_info->ci_inode = inode;
- crypt_info->ci_flags = ctx.flags;
- crypt_info->ci_data_mode = ctx.contents_encryption_mode;
- crypt_info->ci_filename_mode = ctx.filenames_encryption_mode;
- memcpy(crypt_info->ci_master_key_descriptor, ctx.master_key_descriptor,
- FSCRYPT_KEY_DESCRIPTOR_SIZE);
- memcpy(crypt_info->ci_nonce, ctx.nonce, FS_KEY_DERIVATION_NONCE_SIZE);
+ res = fscrypt_policy_from_context(&crypt_info->ci_policy, &ctx, res);
+ if (res)
+ goto out;
+
+ if (ctx.version == FSCRYPT_CONTEXT_V1)
+ memcpy(crypt_info->ci_nonce, ctx.v1.nonce,
+ FS_KEY_DERIVATION_NONCE_SIZE);
+ else
+ memcpy(crypt_info->ci_nonce, ctx.v2.nonce,
+ FS_KEY_DERIVATION_NONCE_SIZE);
+
+ if (!fscrypt_supported_policy(&crypt_info->ci_policy)) {
+ fscrypt_warn(inode->i_sb,
+ "inode %lu uses unsupported encryption policy",
+ inode->i_ino);
+ res = -EINVAL;
+ goto out;
+ }
- mode = select_encryption_mode(crypt_info, inode);
+ mode = select_encryption_mode(&crypt_info->ci_policy, inode);
if (IS_ERR(mode)) {
res = PTR_ERR(mode);
goto out;
diff --git a/fs/crypto/keysetup_legacy.c b/fs/crypto/keysetup_legacy.c
index 407daa0b64d82..1ec420b189f0f 100644
--- a/fs/crypto/keysetup_legacy.c
+++ b/fs/crypto/keysetup_legacy.c
@@ -189,12 +189,13 @@ find_or_insert_direct_key(struct fscrypt_direct_key *to_insert,
*/
BUILD_BUG_ON(sizeof(hash_key) > FSCRYPT_KEY_DESCRIPTOR_SIZE);
- memcpy(&hash_key, ci->ci_master_key_descriptor, sizeof(hash_key));
+ memcpy(&hash_key, ci->ci_policy.v1.master_key_descriptor,
+ sizeof(hash_key));
spin_lock(&fscrypt_direct_keys_lock);
hash_for_each_possible(fscrypt_direct_keys, dk, dk_node, hash_key) {
- if (memcmp(ci->ci_master_key_descriptor, dk->dk_descriptor,
- FSCRYPT_KEY_DESCRIPTOR_SIZE) != 0)
+ if (memcmp(ci->ci_policy.v1.master_key_descriptor,
+ dk->dk_descriptor, FSCRYPT_KEY_DESCRIPTOR_SIZE) != 0)
continue;
if (ci->ci_mode != dk->dk_mode)
continue;
@@ -237,7 +238,7 @@ fscrypt_get_direct_key(const struct fscrypt_info *ci, const u8 *raw_key)
dk->dk_ctfm = NULL;
goto err_free_dk;
}
- memcpy(dk->dk_descriptor, ci->ci_master_key_descriptor,
+ memcpy(dk->dk_descriptor, ci->ci_policy.v1.master_key_descriptor,
FSCRYPT_KEY_DESCRIPTOR_SIZE);
memcpy(dk->dk_raw, raw_key, ci->ci_mode->keysize);
@@ -262,7 +263,8 @@ static int setup_v1_file_key_direct(struct fscrypt_info *ci,
return -EINVAL;
}
- if (ci->ci_data_mode != ci->ci_filename_mode) {
+ if (ci->ci_policy.v1.contents_encryption_mode !=
+ ci->ci_policy.v1.filenames_encryption_mode) {
fscrypt_warn(ci->ci_inode->i_sb,
"direct key flag not allowed with different contents and filenames modes");
return -EINVAL;
@@ -308,7 +310,7 @@ static int setup_v1_file_key_derived(struct fscrypt_info *ci,
int fscrypt_setup_v1_file_key(struct fscrypt_info *ci, const u8 *raw_master_key)
{
- if (ci->ci_flags & FSCRYPT_POLICY_FLAG_DIRECT_KEY)
+ if (ci->ci_policy.v1.flags & FSCRYPT_POLICY_FLAG_DIRECT_KEY)
return setup_v1_file_key_direct(ci, raw_master_key);
else
return setup_v1_file_key_derived(ci, raw_master_key);
@@ -321,11 +323,11 @@ int fscrypt_setup_v1_file_key_via_subscribed_keyrings(struct fscrypt_info *ci)
int err;
key = find_and_lock_process_key(FSCRYPT_KEY_DESC_PREFIX,
- ci->ci_master_key_descriptor,
+ ci->ci_policy.v1.master_key_descriptor,
ci->ci_mode->keysize, &payload);
if (key == ERR_PTR(-ENOKEY) && ci->ci_inode->i_sb->s_cop->key_prefix) {
key = find_and_lock_process_key(ci->ci_inode->i_sb->s_cop->key_prefix,
- ci->ci_master_key_descriptor,
+ ci->ci_policy.v1.master_key_descriptor,
ci->ci_mode->keysize, &payload);
}
if (IS_ERR(key))
diff --git a/fs/crypto/policy.c b/fs/crypto/policy.c
index 1e0563cea1a56..912ff1d030796 100644
--- a/fs/crypto/policy.c
+++ b/fs/crypto/policy.c
@@ -5,8 +5,9 @@
* Copyright (C) 2015, Google, Inc.
* Copyright (C) 2015, Motorola Mobility.
*
- * Written by Michael Halcrow, 2015.
+ * Originally written by Michael Halcrow, 2015.
* Modified by Jaegeuk Kim, 2015.
+ * Modified by Eric Biggers, 2019 for v2 policy support.
*/
#include <linux/random.h>
@@ -14,84 +15,263 @@
#include <linux/mount.h>
#include "fscrypt_private.h"
-/*
- * check whether an encryption policy is consistent with an encryption context
+/**
+ * fscrypt_policies_equal - check whether two encryption policies are the same
+ *
+ * Return: %true if equal, else %false
+ */
+bool fscrypt_policies_equal(const union fscrypt_policy *policy1,
+ const union fscrypt_policy *policy2)
+{
+ if (policy1->version != policy2->version)
+ return false;
+
+ return !memcmp(policy1, policy2, fscrypt_policy_size(policy1));
+}
+
+/**
+ * fscrypt_supported_policy - check whether an encryption policy is supported
+ *
+ * Given an encryption policy, check whether all its encryption modes and other
+ * settings are supported by this kernel. (But we don't currently don't check
+ * for crypto API support here, so attempting to use an algorithm not configured
+ * into the crypto API will still fail later.)
+ *
+ * Return: %true if supported, else %false
*/
-static bool is_encryption_context_consistent_with_policy(
- const struct fscrypt_context *ctx,
- const struct fscrypt_policy *policy)
+bool fscrypt_supported_policy(const union fscrypt_policy *policy_u)
{
- return memcmp(ctx->master_key_descriptor, policy->master_key_descriptor,
- FSCRYPT_KEY_DESCRIPTOR_SIZE) == 0 &&
- (ctx->flags == policy->flags) &&
- (ctx->contents_encryption_mode ==
- policy->contents_encryption_mode) &&
- (ctx->filenames_encryption_mode ==
- policy->filenames_encryption_mode);
+ switch (policy_u->version) {
+ case FSCRYPT_POLICY_V1: {
+ const struct fscrypt_policy_v1 *policy = &policy_u->v1;
+
+ if (!fscrypt_valid_enc_modes(policy->contents_encryption_mode,
+ policy->filenames_encryption_mode))
+ return false;
+
+ if (policy->flags & ~FSCRYPT_POLICY_FLAGS_VALID)
+ return false;
+
+ return true;
+ }
+ case FSCRYPT_POLICY_V2: {
+ const struct fscrypt_policy_v2 *policy = &policy_u->v2;
+
+ if (!fscrypt_valid_enc_modes(policy->contents_encryption_mode,
+ policy->filenames_encryption_mode))
+ return false;
+
+ if (policy->flags & ~FSCRYPT_POLICY_FLAGS_VALID)
+ return false;
+
+ if (memchr_inv(policy->__reserved, 0,
+ sizeof(policy->__reserved)))
+ return false;
+
+ return true;
+ }
+ }
+ return false;
}
-static int create_encryption_context_from_policy(struct inode *inode,
- const struct fscrypt_policy *policy)
+/**
+ * fscrypt_new_context_from_policy - create a new fscrypt_context from a policy
+ *
+ * Create an fscrypt_context for an inode that is being assigned the given
+ * encryption policy. A new nonce is randomly generated.
+ *
+ * Return: the size of the new context in bytes.
+ */
+static int fscrypt_new_context_from_policy(union fscrypt_context *ctx_u,
+ const union fscrypt_policy *policy_u)
{
- struct fscrypt_context ctx;
+ memset(ctx_u, 0, sizeof(*ctx_u));
+
+ switch (policy_u->version) {
+ case FSCRYPT_POLICY_V1: {
+ const struct fscrypt_policy_v1 *policy = &policy_u->v1;
+ struct fscrypt_context_v1 *ctx = &ctx_u->v1;
+
+ ctx->version = FSCRYPT_CONTEXT_V1;
+ ctx->contents_encryption_mode =
+ policy->contents_encryption_mode;
+ ctx->filenames_encryption_mode =
+ policy->filenames_encryption_mode;
+ ctx->flags = policy->flags;
+ memcpy(ctx->master_key_descriptor,
+ policy->master_key_descriptor,
+ sizeof(ctx->master_key_descriptor));
+ get_random_bytes(ctx->nonce, sizeof(ctx->nonce));
+ return sizeof(*ctx);
+ }
+ case FSCRYPT_POLICY_V2: {
+ const struct fscrypt_policy_v2 *policy = &policy_u->v2;
+ struct fscrypt_context_v2 *ctx = &ctx_u->v2;
+
+ ctx->version = FSCRYPT_CONTEXT_V2;
+ ctx->contents_encryption_mode =
+ policy->contents_encryption_mode;
+ ctx->filenames_encryption_mode =
+ policy->filenames_encryption_mode;
+ ctx->flags = policy->flags;
+ memcpy(ctx->master_key_identifier,
+ policy->master_key_identifier,
+ sizeof(ctx->master_key_identifier));
+ get_random_bytes(ctx->nonce, sizeof(ctx->nonce));
+ return sizeof(*ctx);
+ }
+ }
+ BUG();
+}
- ctx.format = FS_ENCRYPTION_CONTEXT_FORMAT_V1;
- memcpy(ctx.master_key_descriptor, policy->master_key_descriptor,
- FSCRYPT_KEY_DESCRIPTOR_SIZE);
+/**
+ * fscrypt_policy_from_context - convert an fscrypt_context to an fscrypt_policy
+ *
+ * Given an fscrypt_context, build the corresponding fscrypt_policy.
+ *
+ * Return: 0 on success, or -EINVAL if the fscrypt_context has an unrecognized
+ * version number or size.
+ *
+ * This does *not* validate the settings within the policy itself, e.g. the
+ * modes, flags, and reserved bits. Use fscrypt_supported_policy() for that.
+ */
+int fscrypt_policy_from_context(union fscrypt_policy *policy_u,
+ const union fscrypt_context *ctx_u,
+ int ctx_size)
+{
+ memset(policy_u, 0, sizeof(*policy_u));
- if (!fscrypt_valid_enc_modes(policy->contents_encryption_mode,
- policy->filenames_encryption_mode))
+ if (ctx_size <= 0 || ctx_size != fscrypt_context_size(ctx_u))
return -EINVAL;
- if (policy->flags & ~FSCRYPT_POLICY_FLAGS_VALID)
+ switch (ctx_u->version) {
+ case FSCRYPT_CONTEXT_V1: {
+ const struct fscrypt_context_v1 *ctx = &ctx_u->v1;
+ struct fscrypt_policy_v1 *policy = &policy_u->v1;
+
+ policy->version = FSCRYPT_POLICY_V1;
+ policy->contents_encryption_mode =
+ ctx->contents_encryption_mode;
+ policy->filenames_encryption_mode =
+ ctx->filenames_encryption_mode;
+ policy->flags = ctx->flags;
+ memcpy(policy->master_key_descriptor,
+ ctx->master_key_descriptor,
+ sizeof(policy->master_key_descriptor));
+ return 0;
+ }
+ case FSCRYPT_CONTEXT_V2: {
+ const struct fscrypt_context_v2 *ctx = &ctx_u->v2;
+ struct fscrypt_policy_v2 *policy = &policy_u->v2;
+
+ policy->version = FSCRYPT_POLICY_V2;
+ policy->contents_encryption_mode =
+ ctx->contents_encryption_mode;
+ policy->filenames_encryption_mode =
+ ctx->filenames_encryption_mode;
+ policy->flags = ctx->flags;
+ memcpy(policy->__reserved, ctx->__reserved,
+ sizeof(policy->__reserved));
+ memcpy(policy->master_key_identifier,
+ ctx->master_key_identifier,
+ sizeof(policy->master_key_identifier));
+ return 0;
+ }
+ }
+ /* unreachable */
+ return -EINVAL;
+}
+
+/* Retrieve an inode's encryption policy */
+static int fscrypt_get_policy(struct inode *inode, union fscrypt_policy *policy)
+{
+ union fscrypt_context ctx;
+ int ret;
+
+ if (inode->i_crypt_info) {
+ /* key available, use the cached policy */
+ *policy = inode->i_crypt_info->ci_policy;
+ return 0;
+ }
+
+ if (!IS_ENCRYPTED(inode))
+ return -ENODATA;
+
+ ret = inode->i_sb->s_cop->get_context(inode, &ctx, sizeof(ctx));
+ if (ret < 0)
+ return (ret == -ERANGE) ? -EINVAL : ret;
+
+ return fscrypt_policy_from_context(policy, &ctx, ret);
+}
+
+static int set_encryption_policy(struct inode *inode,
+ const union fscrypt_policy *policy)
+{
+ union fscrypt_context ctx;
+ int ctxsize;
+
+ if (!fscrypt_supported_policy(policy))
return -EINVAL;
- ctx.contents_encryption_mode = policy->contents_encryption_mode;
- ctx.filenames_encryption_mode = policy->filenames_encryption_mode;
- ctx.flags = policy->flags;
- BUILD_BUG_ON(sizeof(ctx.nonce) != FS_KEY_DERIVATION_NONCE_SIZE);
- get_random_bytes(ctx.nonce, FS_KEY_DERIVATION_NONCE_SIZE);
+ if (policy->version == FSCRYPT_POLICY_V1) {
+ /*
+ * The original encryption policy version provided no way of
+ * verifying that the correct master key was supplied, which was
+ * insecure in scenarios where multiple users have access to the
+ * same encrypted files (even just read-only access). The new
+ * encryption policy version fixes this and also implies use of
+ * an improved key derivation function and allows non-root users
+ * to securely remove keys. So as long as compatibility with
+ * old kernels isn't required, it is recommended to use the new
+ * policy version for all new encrypted directories.
+ */
+ pr_warn_once("%s (pid %d) is setting deprecated v1 encryption policy; recommend upgrading to v2.\n",
+ current->comm, current->pid);
+ }
+
+ ctxsize = fscrypt_new_context_from_policy(&ctx, policy);
- return inode->i_sb->s_cop->set_context(inode, &ctx, sizeof(ctx), NULL);
+ return inode->i_sb->s_cop->set_context(inode, &ctx, ctxsize, NULL);
}
int fscrypt_ioctl_set_policy(struct file *filp, const void __user *arg)
{
- struct fscrypt_policy policy;
+ union fscrypt_policy policy;
+ union fscrypt_policy existing_policy;
struct inode *inode = file_inode(filp);
+ int size;
int ret;
- struct fscrypt_context ctx;
- if (copy_from_user(&policy, arg, sizeof(policy)))
+ if (copy_from_user(&policy, arg, sizeof(u8)))
+ return -EFAULT;
+
+ size = fscrypt_policy_size(&policy);
+ if (size <= 0)
+ return -EINVAL;
+
+ if (copy_from_user((u8 *)&policy + 1, arg + 1, size - 1))
return -EFAULT;
if (!inode_owner_or_capable(inode))
return -EACCES;
- if (policy.version != 0)
- return -EINVAL;
-
ret = mnt_want_write_file(filp);
if (ret)
return ret;
inode_lock(inode);
- ret = inode->i_sb->s_cop->get_context(inode, &ctx, sizeof(ctx));
+ ret = fscrypt_get_policy(inode, &existing_policy);
if (ret == -ENODATA) {
if (!S_ISDIR(inode->i_mode))
ret = -ENOTDIR;
else if (!inode->i_sb->s_cop->empty_dir(inode))
ret = -ENOTEMPTY;
else
- ret = create_encryption_context_from_policy(inode,
- &policy);
- } else if (ret == sizeof(ctx) &&
- is_encryption_context_consistent_with_policy(&ctx,
- &policy)) {
- /* The file already uses the same encryption policy. */
- ret = 0;
- } else if (ret >= 0 || ret == -ERANGE) {
+ ret = set_encryption_policy(inode, &policy);
+ } else if (ret == -EINVAL ||
+ (ret == 0 && !fscrypt_policies_equal(&policy,
+ &existing_policy))) {
/* The file already uses a different encryption policy. */
ret = -EEXIST;
}
@@ -103,37 +283,55 @@ int fscrypt_ioctl_set_policy(struct file *filp, const void __user *arg)
}
EXPORT_SYMBOL(fscrypt_ioctl_set_policy);
+/* Original ioctl version; can only get the original policy version */
int fscrypt_ioctl_get_policy(struct file *filp, void __user *arg)
{
- struct inode *inode = file_inode(filp);
- struct fscrypt_context ctx;
- struct fscrypt_policy policy;
- int res;
+ union fscrypt_policy policy;
+ int err;
- if (!IS_ENCRYPTED(inode))
- return -ENODATA;
+ err = fscrypt_get_policy(file_inode(filp), &policy);
+ if (err)
+ return err;
- res = inode->i_sb->s_cop->get_context(inode, &ctx, sizeof(ctx));
- if (res < 0 && res != -ERANGE)
- return res;
- if (res != sizeof(ctx))
- return -EINVAL;
- if (ctx.format != FS_ENCRYPTION_CONTEXT_FORMAT_V1)
+ if (policy.version != FSCRYPT_POLICY_V1)
return -EINVAL;
- policy.version = 0;
- policy.contents_encryption_mode = ctx.contents_encryption_mode;
- policy.filenames_encryption_mode = ctx.filenames_encryption_mode;
- policy.flags = ctx.flags;
- memcpy(policy.master_key_descriptor, ctx.master_key_descriptor,
- FSCRYPT_KEY_DESCRIPTOR_SIZE);
-
- if (copy_to_user(arg, &policy, sizeof(policy)))
+ if (copy_to_user(arg, &policy, sizeof(policy.v1)))
return -EFAULT;
return 0;
}
EXPORT_SYMBOL(fscrypt_ioctl_get_policy);
+/* Extended ioctl version; can get policies of any version */
+int fscrypt_ioctl_get_policy_ex(struct file *filp, void __user *_uarg)
+{
+ struct fscrypt_get_policy_ex_arg __user *uarg = _uarg;
+ struct fscrypt_get_policy_ex_arg arg;
+ union fscrypt_policy *policy = (union fscrypt_policy *)&arg.policy;
+ u64 avail_size;
+ int err;
+
+ BUILD_BUG_ON(sizeof(arg.policy) != sizeof(union fscrypt_policy));
+ BUILD_BUG_ON(sizeof(arg.policy_size) !=
+ offsetof(struct fscrypt_get_policy_ex_arg, policy));
+
+ err = fscrypt_get_policy(file_inode(filp), policy);
+ if (err)
+ return err;
+ arg.policy_size = fscrypt_policy_size(policy);
+
+ if (get_user(avail_size, &uarg->policy_size))
+ return -EFAULT;
+
+ if (avail_size < arg.policy_size)
+ return -EOVERFLOW;
+
+ if (copy_to_user(uarg, &arg, sizeof(arg.policy_size) + arg.policy_size))
+ return -EFAULT;
+ return 0;
+}
+EXPORT_SYMBOL_GPL(fscrypt_ioctl_get_policy_ex);
+
/**
* fscrypt_has_permitted_context() - is a file's encryption policy permitted
* within its directory?
@@ -155,10 +353,8 @@ EXPORT_SYMBOL(fscrypt_ioctl_get_policy);
*/
int fscrypt_has_permitted_context(struct inode *parent, struct inode *child)
{
- const struct fscrypt_operations *cops = parent->i_sb->s_cop;
- const struct fscrypt_info *parent_ci, *child_ci;
- struct fscrypt_context parent_ctx, child_ctx;
- int res;
+ union fscrypt_policy parent_policy, child_policy;
+ int err;
/* No restrictions on file types which are never encrypted */
if (!S_ISREG(child->i_mode) && !S_ISDIR(child->i_mode) &&
@@ -188,41 +384,22 @@ int fscrypt_has_permitted_context(struct inode *parent, struct inode *child)
* In any case, if an unexpected error occurs, fall back to "forbidden".
*/
- res = fscrypt_get_encryption_info(parent);
- if (res)
+ err = fscrypt_get_encryption_info(parent);
+ if (err)
return 0;
- res = fscrypt_get_encryption_info(child);
- if (res)
+ err = fscrypt_get_encryption_info(child);
+ if (err)
return 0;
- parent_ci = parent->i_crypt_info;
- child_ci = child->i_crypt_info;
-
- if (parent_ci && child_ci) {
- return memcmp(parent_ci->ci_master_key_descriptor,
- child_ci->ci_master_key_descriptor,
- FSCRYPT_KEY_DESCRIPTOR_SIZE) == 0 &&
- (parent_ci->ci_data_mode == child_ci->ci_data_mode) &&
- (parent_ci->ci_filename_mode ==
- child_ci->ci_filename_mode) &&
- (parent_ci->ci_flags == child_ci->ci_flags);
- }
- res = cops->get_context(parent, &parent_ctx, sizeof(parent_ctx));
- if (res != sizeof(parent_ctx))
+ err = fscrypt_get_policy(parent, &parent_policy);
+ if (err)
return 0;
- res = cops->get_context(child, &child_ctx, sizeof(child_ctx));
- if (res != sizeof(child_ctx))
+ err = fscrypt_get_policy(child, &child_policy);
+ if (err)
return 0;
- return memcmp(parent_ctx.master_key_descriptor,
- child_ctx.master_key_descriptor,
- FSCRYPT_KEY_DESCRIPTOR_SIZE) == 0 &&
- (parent_ctx.contents_encryption_mode ==
- child_ctx.contents_encryption_mode) &&
- (parent_ctx.filenames_encryption_mode ==
- child_ctx.filenames_encryption_mode) &&
- (parent_ctx.flags == child_ctx.flags);
+ return fscrypt_policies_equal(&parent_policy, &child_policy);
}
EXPORT_SYMBOL(fscrypt_has_permitted_context);
@@ -238,7 +415,8 @@ EXPORT_SYMBOL(fscrypt_has_permitted_context);
int fscrypt_inherit_context(struct inode *parent, struct inode *child,
void *fs_data, bool preload)
{
- struct fscrypt_context ctx;
+ union fscrypt_context ctx;
+ int ctxsize;
struct fscrypt_info *ci;
int res;
@@ -250,16 +428,10 @@ int fscrypt_inherit_context(struct inode *parent, struct inode *child,
if (ci == NULL)
return -ENOKEY;
- ctx.format = FS_ENCRYPTION_CONTEXT_FORMAT_V1;
- ctx.contents_encryption_mode = ci->ci_data_mode;
- ctx.filenames_encryption_mode = ci->ci_filename_mode;
- ctx.flags = ci->ci_flags;
- memcpy(ctx.master_key_descriptor, ci->ci_master_key_descriptor,
- FSCRYPT_KEY_DESCRIPTOR_SIZE);
- get_random_bytes(ctx.nonce, FS_KEY_DERIVATION_NONCE_SIZE);
+ ctxsize = fscrypt_new_context_from_policy(&ctx, &ci->ci_policy);
+
BUILD_BUG_ON(sizeof(ctx) != FSCRYPT_SET_CONTEXT_MAX_SIZE);
- res = parent->i_sb->s_cop->set_context(child, &ctx,
- sizeof(ctx), fs_data);
+ res = parent->i_sb->s_cop->set_context(child, &ctx, ctxsize, fs_data);
if (res)
return res;
return preload ? fscrypt_get_encryption_info(child): 0;
diff --git a/include/linux/fscrypt.h b/include/linux/fscrypt.h
index 5f251cad255b8..3ed02a0849db6 100644
--- a/include/linux/fscrypt.h
+++ b/include/linux/fscrypt.h
@@ -42,7 +42,7 @@ struct fscrypt_name {
#define fname_len(p) ((p)->disk_name.len)
/* Maximum value for the third parameter of fscrypt_operations.set_context(). */
-#define FSCRYPT_SET_CONTEXT_MAX_SIZE 28
+#define FSCRYPT_SET_CONTEXT_MAX_SIZE 40
#ifdef CONFIG_FS_ENCRYPTION
/*
@@ -109,6 +109,7 @@ extern void fscrypt_restore_control_page(struct page *);
/* policy.c */
extern int fscrypt_ioctl_set_policy(struct file *, const void __user *);
extern int fscrypt_ioctl_get_policy(struct file *, void __user *);
+extern int fscrypt_ioctl_get_policy_ex(struct file *, void __user *);
extern int fscrypt_has_permitted_context(struct inode *, struct inode *);
extern int fscrypt_inherit_context(struct inode *, struct inode *,
void *, bool);
@@ -306,6 +307,12 @@ static inline int fscrypt_ioctl_get_policy(struct file *filp, void __user *arg)
return -EOPNOTSUPP;
}
+static inline int fscrypt_ioctl_get_policy_ex(struct file *filp,
+ void __user *arg)
+{
+ return -EOPNOTSUPP;
+}
+
static inline int fscrypt_has_permitted_context(struct inode *parent,
struct inode *child)
{
diff --git a/include/uapi/linux/fscrypt.h b/include/uapi/linux/fscrypt.h
index 042e70a4ff7ee..26879e669dfa0 100644
--- a/include/uapi/linux/fscrypt.h
+++ b/include/uapi/linux/fscrypt.h
@@ -8,7 +8,6 @@
* File system encryption support
*/
/* Policy provided via an ioctl on the topmost directory */
-#define FSCRYPT_KEY_DESCRIPTOR_SIZE 8
/* Encryption policy flags */
#define FSCRYPT_POLICY_FLAGS_PAD_4 0x00
@@ -16,7 +15,7 @@
#define FSCRYPT_POLICY_FLAGS_PAD_16 0x02
#define FSCRYPT_POLICY_FLAGS_PAD_32 0x03
#define FSCRYPT_POLICY_FLAGS_PAD_MASK 0x03
-#define FSCRYPT_POLICY_FLAG_DIRECT_KEY 0x04 /* use master key directly */
+#define FSCRYPT_POLICY_FLAG_DIRECT_KEY 0x04
#define FSCRYPT_POLICY_FLAGS_VALID 0x07
/* Encryption algorithms */
@@ -25,14 +24,24 @@
#define FSCRYPT_MODE_AES_128_CBC 5
#define FSCRYPT_MODE_AES_128_CTS 6
#define FSCRYPT_MODE_ADIANTUM 9
+#define __FSCRYPT_MODE_MAX 9
-struct fscrypt_policy {
+/*
+ * Legacy policy version; ad-hoc KDF and no key verification.
+ * For new encrypted directories, use fscrypt_policy_v2 instead.
+ *
+ * Careful: the .version field for this is actually 0, not 1.
+ */
+#define FSCRYPT_POLICY_V1 0
+#define FSCRYPT_KEY_DESCRIPTOR_SIZE 8
+struct fscrypt_policy_v1 {
__u8 version;
__u8 contents_encryption_mode;
__u8 filenames_encryption_mode;
__u8 flags;
__u8 master_key_descriptor[FSCRYPT_KEY_DESCRIPTOR_SIZE];
};
+#define fscrypt_policy fscrypt_policy_v1
/*
* Process-subscribed "logon" key description prefix and payload format.
@@ -47,13 +56,39 @@ struct fscrypt_key {
__u32 size;
};
+/*
+ * New policy version with HKDF and key verification (recommended).
+ */
+#define FSCRYPT_POLICY_V2 2
+#define FSCRYPT_KEY_IDENTIFIER_SIZE 16
+struct fscrypt_policy_v2 {
+ __u8 version;
+ __u8 contents_encryption_mode;
+ __u8 filenames_encryption_mode;
+ __u8 flags;
+ __u8 __reserved[4];
+ __u8 master_key_identifier[FSCRYPT_KEY_IDENTIFIER_SIZE];
+};
+
+/* Struct passed to FS_IOC_GET_ENCRYPTION_POLICY_EX */
+struct fscrypt_get_policy_ex_arg {
+ __u64 policy_size; /* input/output */
+ union {
+ __u8 version;
+ struct fscrypt_policy_v1 v1;
+ struct fscrypt_policy_v2 v2;
+ } policy; /* output */
+};
+
struct fscrypt_key_specifier {
#define FSCRYPT_KEY_SPEC_TYPE_DESCRIPTOR 1
+#define FSCRYPT_KEY_SPEC_TYPE_IDENTIFIER 2
__u32 type;
__u32 __reserved;
union {
__u8 __reserved[32]; /* reserve some extra space */
__u8 descriptor[FSCRYPT_KEY_DESCRIPTOR_SIZE];
+ __u8 identifier[FSCRYPT_KEY_IDENTIFIER_SIZE];
} u;
};
@@ -88,6 +123,7 @@ struct fscrypt_get_key_status_arg {
#define FS_IOC_SET_ENCRYPTION_POLICY _IOR('f', 19, struct fscrypt_policy)
#define FS_IOC_GET_ENCRYPTION_PWSALT _IOW('f', 20, __u8[16])
#define FS_IOC_GET_ENCRYPTION_POLICY _IOW('f', 21, struct fscrypt_policy)
+#define FS_IOC_GET_ENCRYPTION_POLICY_EX _IOWR('f', 22, __u8[9]) /* size + version */
#define FS_IOC_ADD_ENCRYPTION_KEY _IOWR('f', 23, struct fscrypt_add_key_arg)
#define FS_IOC_REMOVE_ENCRYPTION_KEY _IOW('f', 24, struct fscrypt_remove_key_arg)
#define FS_IOC_GET_ENCRYPTION_KEY_STATUS _IOWR('f', 25, struct fscrypt_get_key_status_arg)
--
2.21.0
^ permalink raw reply related
* [PATCH v4 10/17] fscrypt: add an HKDF-SHA512 implementation
From: Eric Biggers @ 2019-04-02 15:45 UTC (permalink / raw)
To: linux-fscrypt
Cc: Satya Tangirala, linux-api, linux-f2fs-devel, keyrings, linux-mtd,
linux-crypto, linux-fsdevel, linux-ext4, Paul Crowley
In-Reply-To: <20190402154600.32432-1-ebiggers@kernel.org>
From: Eric Biggers <ebiggers@google.com>
Add an implementation of HKDF (RFC 5869) to fscrypt, for the purpose of
deriving additional key material from the fscrypt master keys for v2
encryption policies. HKDF is a key derivation function built on top of
HMAC. We choose SHA-512 for the underlying unkeyed hash, and use an
"hmac(sha512)" transform allocated from the crypto API.
We'll be using this to replace the AES-ECB based KDF currently used to
derive the per-file encryption keys. While the AES-ECB based KDF is
believed to meet the original security requirements, it is nonstandard
and has problems that don't exist in modern KDFs such as HKDF:
1. It's reversible. Given a derived key and nonce, an attacker can
easily compute the master key. This is okay if the master key and
derived keys are equally hard to compromise, but now we'd like to be
more robust against threats such as a derived key being compromised
through a timing attack, or a derived key for an in-use file being
compromised after the master key has already been removed.
2. It doesn't evenly distribute the entropy from the master key; each 16
input bytes only affects the corresponding 16 output bytes.
3. It isn't easily extensible to deriving other values or keys that are
or will be needed, such as per-mode keys (which is what DIRECT_KEY
policies really should use, rather than the master key directly as
they do now) or a public hash for securely identifying the key (to
ensure that an encrypted file cannot be set up with the wrong key).
HKDF solves all the above problems.
Signed-off-by: Eric Biggers <ebiggers@google.com>
---
fs/crypto/Kconfig | 2 +
fs/crypto/Makefile | 1 +
fs/crypto/fscrypt_private.h | 15 +++
fs/crypto/hkdf.c | 188 ++++++++++++++++++++++++++++++++++++
4 files changed, 206 insertions(+)
create mode 100644 fs/crypto/hkdf.c
diff --git a/fs/crypto/Kconfig b/fs/crypto/Kconfig
index f0de238000c02..c160598a9fe2c 100644
--- a/fs/crypto/Kconfig
+++ b/fs/crypto/Kconfig
@@ -7,6 +7,8 @@ config FS_ENCRYPTION
select CRYPTO_XTS
select CRYPTO_CTS
select CRYPTO_SHA256
+ select CRYPTO_SHA512
+ select CRYPTO_HMAC
select KEYS
help
Enable encryption of files and directories. This
diff --git a/fs/crypto/Makefile b/fs/crypto/Makefile
index accdd622c9083..4977b43479289 100644
--- a/fs/crypto/Makefile
+++ b/fs/crypto/Makefile
@@ -2,6 +2,7 @@ obj-$(CONFIG_FS_ENCRYPTION) += fscrypto.o
fscrypto-y := crypto.o \
fname.o \
+ hkdf.o \
hooks.o \
keyring.o \
keysetup.o \
diff --git a/fs/crypto/fscrypt_private.h b/fs/crypto/fscrypt_private.h
index cc1862c9baa1d..e2a65189eb578 100644
--- a/fs/crypto/fscrypt_private.h
+++ b/fs/crypto/fscrypt_private.h
@@ -175,6 +175,21 @@ extern bool fscrypt_fname_encrypted_size(const struct inode *inode,
u32 orig_len, u32 max_len,
u32 *encrypted_len_ret);
+/* hkdf.c */
+
+struct fscrypt_hkdf {
+ struct crypto_shash *hmac_tfm;
+};
+
+extern int fscrypt_init_hkdf(struct fscrypt_hkdf *hkdf, const u8 *master_key,
+ unsigned int master_key_size);
+
+extern int fscrypt_hkdf_expand(struct fscrypt_hkdf *hkdf, u8 context,
+ const u8 *info, unsigned int infolen,
+ u8 *okm, unsigned int okmlen);
+
+extern void fscrypt_destroy_hkdf(struct fscrypt_hkdf *hkdf);
+
/* keyring.c */
/*
diff --git a/fs/crypto/hkdf.c b/fs/crypto/hkdf.c
new file mode 100644
index 0000000000000..3f9dbf171d967
--- /dev/null
+++ b/fs/crypto/hkdf.c
@@ -0,0 +1,188 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Implementation of HKDF ("HMAC-based Extract-and-Expand Key Derivation
+ * Function"), aka RFC 5869. See also the original paper (Krawczyk 2010):
+ * "Cryptographic Extraction and Key Derivation: The HKDF Scheme".
+ *
+ * This is used to derive keys from the fscrypt master keys.
+ *
+ * Copyright 2019 Google LLC
+ */
+
+#include <crypto/hash.h>
+#include <crypto/sha.h>
+
+#include "fscrypt_private.h"
+
+/*
+ * HKDF supports any unkeyed cryptographic hash algorithm, but fscrypt uses
+ * SHA-512 because it is reasonably secure and efficient; and since it produces
+ * a 64-byte digest, deriving an AES-256-XTS key preserves all 64 bytes of
+ * entropy from the master key and requires only one iteration of HKDF-Expand.
+ */
+#define HKDF_HMAC_ALG "hmac(sha512)"
+#define HKDF_HASHLEN SHA512_DIGEST_SIZE
+
+/*
+ * HKDF consists of two steps:
+ *
+ * 1. HKDF-Extract: extract a pseudorandom key of length HKDF_HASHLEN bytes from
+ * the input keying material and optional salt.
+ * 2. HKDF-Expand: expand the pseudorandom key into output keying material of
+ * any length, parameterized by an application-specific info string.
+ *
+ * HKDF-Extract can be skipped if the input is already a pseudorandom key of
+ * length HKDF_HASHLEN bytes. However, cipher modes other than AES-256-XTS take
+ * shorter keys, and we don't want to force users of those modes to generate
+ * unnecessarily long keys. Thus fscrypt still does HKDF-Extract.
+ *
+ * HKDF-Extract also supports a salt. Choosing a random salt per input key
+ * would permit the input key to come from *any* source with sufficient entropy,
+ * even if it's not distributed uniformly. However, fscrypt doesn't take
+ * advantage of this because userspace should already provide good pseudorandom
+ * keys, which makes this unnecessary; also, having to persist a random salt per
+ * key from kernel mode would pose significant implementation complexity. Thus
+ * fscrypt uses a fixed salt. But to be slightly more robust against userspace
+ * (unwisely) reusing the fscrypt keys for another purpose, and to force
+ * brute-force attacks to target the fscrypt KDF specifically, fscrypt uses
+ * "fscrypt_hkdf_salt" rather than the default of all 0's defined by RFC 5869.
+ */
+
+#define HKDF_SALT "fscrypt_hkdf_salt"
+#define HKDF_SALTLEN CONST_STRLEN(HKDF_SALT)
+
+/* HKDF-Extract (RFC 5869 section 2.2), see explanation above */
+static int hkdf_extract(struct crypto_shash *hmac_tfm, const u8 *ikm,
+ unsigned int ikmlen, u8 prk[HKDF_HASHLEN])
+{
+ SHASH_DESC_ON_STACK(desc, hmac_tfm);
+ int err;
+
+ err = crypto_shash_setkey(hmac_tfm, HKDF_SALT, HKDF_SALTLEN);
+ if (err)
+ return err;
+
+ desc->tfm = hmac_tfm;
+ desc->flags = 0;
+ err = crypto_shash_digest(desc, ikm, ikmlen, prk);
+ shash_desc_zero(desc);
+ return err;
+}
+
+/*
+ * Compute HKDF-Extract using the given master key as the input keying material,
+ * and prepare an HMAC transform object keyed by the resulting pseudorandom key.
+ *
+ * Afterwards, the keyed HMAC transform object can be used for HKDF-Expand many
+ * times without having to recompute HKDF-Extract each time.
+ */
+int fscrypt_init_hkdf(struct fscrypt_hkdf *hkdf, const u8 *master_key,
+ unsigned int master_key_size)
+{
+ struct crypto_shash *hmac_tfm;
+ u8 prk[HKDF_HASHLEN];
+ int err;
+
+ hmac_tfm = crypto_alloc_shash(HKDF_HMAC_ALG, 0, 0);
+ if (IS_ERR(hmac_tfm)) {
+ fscrypt_warn(NULL, "error allocating " HKDF_HMAC_ALG ": %ld",
+ PTR_ERR(hmac_tfm));
+ return PTR_ERR(hmac_tfm);
+ }
+
+ BUG_ON(crypto_shash_digestsize(hmac_tfm) != sizeof(prk));
+
+ err = hkdf_extract(hmac_tfm, master_key, master_key_size, prk);
+ if (err)
+ goto err_free_tfm;
+
+ err = crypto_shash_setkey(hmac_tfm, prk, sizeof(prk));
+ if (err)
+ goto err_free_tfm;
+
+ hkdf->hmac_tfm = hmac_tfm;
+ goto out;
+
+err_free_tfm:
+ crypto_free_shash(hmac_tfm);
+out:
+ memzero_explicit(prk, sizeof(prk));
+ return err;
+}
+
+/*
+ * HKDF-Expand (RFC 5869 section 2.3). This expands the pseudorandom key, which
+ * was already keyed into 'hkdf->hmac_tfm' by fscrypt_init_hkdf(), into 'okmlen'
+ * bytes of output keying material parameterized by the application-specific
+ * 'info' of length 'infolen' bytes, prefixed with the 'context' byte. This is
+ * thread-safe and may be called by multiple threads in parallel.
+ *
+ * ('context' isn't part of the HKDF specification; it's just a prefix fscrypt
+ * adds to its application-specific info strings to guarantee that it doesn't
+ * accidentally repeat an info string when using HKDF for different purposes.)
+ */
+int fscrypt_hkdf_expand(struct fscrypt_hkdf *hkdf, u8 context,
+ const u8 *info, unsigned int infolen,
+ u8 *okm, unsigned int okmlen)
+{
+ SHASH_DESC_ON_STACK(desc, hkdf->hmac_tfm);
+ unsigned int i;
+ int err;
+ const u8 *prev = NULL;
+ u8 counter = 1;
+ u8 tmp[HKDF_HASHLEN];
+
+ if (WARN_ON(okmlen > 255 * HKDF_HASHLEN))
+ return -EINVAL;
+
+ desc->tfm = hkdf->hmac_tfm;
+ desc->flags = 0;
+
+ for (i = 0; i < okmlen; i += HKDF_HASHLEN) {
+
+ err = crypto_shash_init(desc);
+ if (err)
+ goto out;
+
+ if (prev) {
+ err = crypto_shash_update(desc, prev, HKDF_HASHLEN);
+ if (err)
+ goto out;
+ }
+
+ BUILD_BUG_ON(sizeof(context) != 1);
+ err = crypto_shash_update(desc, &context, 1);
+ if (err)
+ goto out;
+
+ err = crypto_shash_update(desc, info, infolen);
+ if (err)
+ goto out;
+
+ BUILD_BUG_ON(sizeof(counter) != 1);
+ if (okmlen - i < HKDF_HASHLEN) {
+ err = crypto_shash_finup(desc, &counter, 1, tmp);
+ if (err)
+ goto out;
+ memcpy(&okm[i], tmp, okmlen - i);
+ memzero_explicit(tmp, sizeof(tmp));
+ } else {
+ err = crypto_shash_finup(desc, &counter, 1, &okm[i]);
+ if (err)
+ goto out;
+ }
+ counter++;
+ prev = &okm[i];
+ }
+ err = 0;
+out:
+ if (unlikely(err))
+ memzero_explicit(okm, okmlen); /* so caller doesn't need to */
+ shash_desc_zero(desc);
+ return err;
+}
+
+void fscrypt_destroy_hkdf(struct fscrypt_hkdf *hkdf)
+{
+ crypto_free_shash(hkdf->hmac_tfm);
+}
--
2.21.0
^ permalink raw reply related
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