* Re: [PATCH v2 10/20] x86: avoid W^X being broken during modules loading
From: hpa @ 2019-03-07 16:53 UTC (permalink / raw)
To: Borislav Petkov, Nadav Amit
Cc: Rick Edgecombe, Andy Lutomirski, Ingo Molnar, LKML, X86 ML,
Thomas Gleixner, Dave Hansen, Peter Zijlstra, Damian Tometzki,
linux-integrity, LSM List, Andrew Morton, Kernel Hardening,
Linux-MM, Will Deacon, Ard Biesheuvel, Kristen Carlson Accardi,
Dock, Deneen T, Kees Cook, Dave Hansen, Masami Hiramatsu
In-Reply-To: <20190307072947.GA26566@zn.tnic>
On March 6, 2019 11:29:47 PM PST, Borislav Petkov <bp@alien8.de> wrote:
>On Mon, Feb 11, 2019 at 08:42:51PM +0100, Borislav Petkov wrote:
>> On Mon, Feb 11, 2019 at 11:27:03AM -0800, Nadav Amit wrote:
>> > Is there any comment over static_cpu_has()? ;-)
>>
>> Almost:
>>
>> /*
>> * Static testing of CPU features. Used the same as boot_cpu_has().
>> * These will statically patch the target code for additional
>> * performance.
>> */
>> static __always_inline __pure bool _static_cpu_has(u16 bit)
>
>Ok, I guess something like that along with converting the obvious slow
>path callers to boot_cpu_has():
>
>---
>diff --git a/arch/x86/include/asm/cpufeature.h
>b/arch/x86/include/asm/cpufeature.h
>index ce95b8cbd229..e25d11ad7a88 100644
>--- a/arch/x86/include/asm/cpufeature.h
>+++ b/arch/x86/include/asm/cpufeature.h
>@@ -155,9 +155,12 @@ extern void clear_cpu_cap(struct cpuinfo_x86 *c,
>unsigned int bit);
> #else
>
> /*
>- * Static testing of CPU features. Used the same as boot_cpu_has().
>- * These will statically patch the target code for additional
>- * performance.
>+ * Static testing of CPU features. Used the same as boot_cpu_has(). It
>+ * statically patches the target code for additional performance. Use
>+ * static_cpu_has() only in fast paths, where every cycle counts.
>Which
>+ * means that the boot_cpu_has() variant is already fast enough for
>the
>+ * majority of cases and you should stick to using it as it is
>generally
>+ * only two instructions: a RIP-relative MOV and a TEST.
> */
> static __always_inline __pure bool _static_cpu_has(u16 bit)
> {
>diff --git a/arch/x86/include/asm/fpu/internal.h
>b/arch/x86/include/asm/fpu/internal.h
>index fa2c93cb42a2..c525b053b3b3 100644
>--- a/arch/x86/include/asm/fpu/internal.h
>+++ b/arch/x86/include/asm/fpu/internal.h
>@@ -291,7 +291,7 @@ static inline void
>copy_xregs_to_kernel_booting(struct xregs_state *xstate)
>
> WARN_ON(system_state != SYSTEM_BOOTING);
>
>- if (static_cpu_has(X86_FEATURE_XSAVES))
>+ if (boot_cpu_has(X86_FEATURE_XSAVES))
> XSTATE_OP(XSAVES, xstate, lmask, hmask, err);
> else
> XSTATE_OP(XSAVE, xstate, lmask, hmask, err);
>@@ -313,7 +313,7 @@ static inline void
>copy_kernel_to_xregs_booting(struct xregs_state *xstate)
>
> WARN_ON(system_state != SYSTEM_BOOTING);
>
>- if (static_cpu_has(X86_FEATURE_XSAVES))
>+ if (boot_cpu_has(X86_FEATURE_XSAVES))
> XSTATE_OP(XRSTORS, xstate, lmask, hmask, err);
> else
> XSTATE_OP(XRSTOR, xstate, lmask, hmask, err);
>@@ -528,8 +528,7 @@ static inline void fpregs_activate(struct fpu *fpu)
> * - switch_fpu_finish() restores the new state as
> * necessary.
> */
>-static inline void
>-switch_fpu_prepare(struct fpu *old_fpu, int cpu)
>+static inline void switch_fpu_prepare(struct fpu *old_fpu, int cpu)
> {
> if (static_cpu_has(X86_FEATURE_FPU) && old_fpu->initialized) {
> if (!copy_fpregs_to_fpstate(old_fpu))
>diff --git a/arch/x86/kernel/apic/apic_numachip.c
>b/arch/x86/kernel/apic/apic_numachip.c
>index 78778b54f904..a5464b8b6c46 100644
>--- a/arch/x86/kernel/apic/apic_numachip.c
>+++ b/arch/x86/kernel/apic/apic_numachip.c
>@@ -175,7 +175,7 @@ static void fixup_cpu_id(struct cpuinfo_x86 *c, int
>node)
> this_cpu_write(cpu_llc_id, node);
>
> /* Account for nodes per socket in multi-core-module processors */
>- if (static_cpu_has(X86_FEATURE_NODEID_MSR)) {
>+ if (boot_cpu_has(X86_FEATURE_NODEID_MSR)) {
> rdmsrl(MSR_FAM10H_NODE_ID, val);
> nodes = ((val >> 3) & 7) + 1;
> }
>diff --git a/arch/x86/kernel/cpu/aperfmperf.c
>b/arch/x86/kernel/cpu/aperfmperf.c
>index 804c49493938..64d5aec24203 100644
>--- a/arch/x86/kernel/cpu/aperfmperf.c
>+++ b/arch/x86/kernel/cpu/aperfmperf.c
>@@ -83,7 +83,7 @@ unsigned int aperfmperf_get_khz(int cpu)
> if (!cpu_khz)
> return 0;
>
>- if (!static_cpu_has(X86_FEATURE_APERFMPERF))
>+ if (!boot_cpu_has(X86_FEATURE_APERFMPERF))
> return 0;
>
> aperfmperf_snapshot_cpu(cpu, ktime_get(), true);
>@@ -99,7 +99,7 @@ void arch_freq_prepare_all(void)
> if (!cpu_khz)
> return;
>
>- if (!static_cpu_has(X86_FEATURE_APERFMPERF))
>+ if (!boot_cpu_has(X86_FEATURE_APERFMPERF))
> return;
>
> for_each_online_cpu(cpu)
>@@ -115,7 +115,7 @@ unsigned int arch_freq_get_on_cpu(int cpu)
> if (!cpu_khz)
> return 0;
>
>- if (!static_cpu_has(X86_FEATURE_APERFMPERF))
>+ if (!boot_cpu_has(X86_FEATURE_APERFMPERF))
> return 0;
>
> if (aperfmperf_snapshot_cpu(cpu, ktime_get(), true))
>diff --git a/arch/x86/kernel/cpu/common.c
>b/arch/x86/kernel/cpu/common.c
>index cb28e98a0659..95a5faf3a6a0 100644
>--- a/arch/x86/kernel/cpu/common.c
>+++ b/arch/x86/kernel/cpu/common.c
>@@ -1668,7 +1668,7 @@ static void setup_getcpu(int cpu)
> unsigned long cpudata = vdso_encode_cpunode(cpu,
>early_cpu_to_node(cpu));
> struct desc_struct d = { };
>
>- if (static_cpu_has(X86_FEATURE_RDTSCP))
>+ if (boot_cpu_has(X86_FEATURE_RDTSCP))
> write_rdtscp_aux(cpudata);
>
> /* Store CPU and node number in limit. */
>diff --git a/arch/x86/kernel/cpu/mce/inject.c
>b/arch/x86/kernel/cpu/mce/inject.c
>index 8492ef7d9015..3da9a8823e47 100644
>--- a/arch/x86/kernel/cpu/mce/inject.c
>+++ b/arch/x86/kernel/cpu/mce/inject.c
>@@ -528,7 +528,7 @@ static void do_inject(void)
> * only on the node base core. Refer to D18F3x44[NbMcaToMstCpuEn] for
> * Fam10h and later BKDGs.
> */
>- if (static_cpu_has(X86_FEATURE_AMD_DCM) &&
>+ if (boot_cpu_has(X86_FEATURE_AMD_DCM) &&
> b == 4 &&
> boot_cpu_data.x86 < 0x17) {
> toggle_nb_mca_mst_cpu(amd_get_nb_id(cpu));
>diff --git a/arch/x86/kernel/cpu/proc.c b/arch/x86/kernel/cpu/proc.c
>index 2c8522a39ed5..cb2e49810d68 100644
>--- a/arch/x86/kernel/cpu/proc.c
>+++ b/arch/x86/kernel/cpu/proc.c
>@@ -35,11 +35,11 @@ static void show_cpuinfo_misc(struct seq_file *m,
>struct cpuinfo_x86 *c)
> "fpu_exception\t: %s\n"
> "cpuid level\t: %d\n"
> "wp\t\t: yes\n",
>- static_cpu_has_bug(X86_BUG_FDIV) ? "yes" : "no",
>- static_cpu_has_bug(X86_BUG_F00F) ? "yes" : "no",
>- static_cpu_has_bug(X86_BUG_COMA) ? "yes" : "no",
>- static_cpu_has(X86_FEATURE_FPU) ? "yes" : "no",
>- static_cpu_has(X86_FEATURE_FPU) ? "yes" : "no",
>+ boot_cpu_has_bug(X86_BUG_FDIV) ? "yes" : "no",
>+ boot_cpu_has_bug(X86_BUG_F00F) ? "yes" : "no",
>+ boot_cpu_has_bug(X86_BUG_COMA) ? "yes" : "no",
>+ boot_cpu_has(X86_FEATURE_FPU) ? "yes" : "no",
>+ boot_cpu_has(X86_FEATURE_FPU) ? "yes" : "no",
> c->cpuid_level);
> }
> #else
>diff --git a/arch/x86/kernel/ldt.c b/arch/x86/kernel/ldt.c
>index 6135ae8ce036..b2463fcb20a8 100644
>--- a/arch/x86/kernel/ldt.c
>+++ b/arch/x86/kernel/ldt.c
>@@ -113,7 +113,7 @@ static void do_sanity_check(struct mm_struct *mm,
> * tables.
> */
> WARN_ON(!had_kernel_mapping);
>- if (static_cpu_has(X86_FEATURE_PTI))
>+ if (boot_cpu_has(X86_FEATURE_PTI))
> WARN_ON(!had_user_mapping);
> } else {
> /*
>@@ -121,7 +121,7 @@ static void do_sanity_check(struct mm_struct *mm,
> * Sync the pgd to the usermode tables.
> */
> WARN_ON(had_kernel_mapping);
>- if (static_cpu_has(X86_FEATURE_PTI))
>+ if (boot_cpu_has(X86_FEATURE_PTI))
> WARN_ON(had_user_mapping);
> }
> }
>@@ -156,7 +156,7 @@ static void map_ldt_struct_to_user(struct mm_struct
>*mm)
> k_pmd = pgd_to_pmd_walk(k_pgd, LDT_BASE_ADDR);
> u_pmd = pgd_to_pmd_walk(u_pgd, LDT_BASE_ADDR);
>
>- if (static_cpu_has(X86_FEATURE_PTI) && !mm->context.ldt)
>+ if (boot_cpu_has(X86_FEATURE_PTI) && !mm->context.ldt)
> set_pmd(u_pmd, *k_pmd);
> }
>
>@@ -181,7 +181,7 @@ static void map_ldt_struct_to_user(struct mm_struct
>*mm)
> {
> pgd_t *pgd = pgd_offset(mm, LDT_BASE_ADDR);
>
>- if (static_cpu_has(X86_FEATURE_PTI) && !mm->context.ldt)
>+ if (boot_cpu_has(X86_FEATURE_PTI) && !mm->context.ldt)
> set_pgd(kernel_to_user_pgdp(pgd), *pgd);
> }
>
>@@ -208,7 +208,7 @@ map_ldt_struct(struct mm_struct *mm, struct
>ldt_struct *ldt, int slot)
> spinlock_t *ptl;
> int i, nr_pages;
>
>- if (!static_cpu_has(X86_FEATURE_PTI))
>+ if (!boot_cpu_has(X86_FEATURE_PTI))
> return 0;
>
> /*
>@@ -271,7 +271,7 @@ static void unmap_ldt_struct(struct mm_struct *mm,
>struct ldt_struct *ldt)
> return;
>
> /* LDT map/unmap is only required for PTI */
>- if (!static_cpu_has(X86_FEATURE_PTI))
>+ if (!boot_cpu_has(X86_FEATURE_PTI))
> return;
>
> nr_pages = DIV_ROUND_UP(ldt->nr_entries * LDT_ENTRY_SIZE, PAGE_SIZE);
>@@ -311,7 +311,7 @@ static void free_ldt_pgtables(struct mm_struct *mm)
> unsigned long start = LDT_BASE_ADDR;
> unsigned long end = LDT_END_ADDR;
>
>- if (!static_cpu_has(X86_FEATURE_PTI))
>+ if (!boot_cpu_has(X86_FEATURE_PTI))
> return;
>
> tlb_gather_mmu(&tlb, mm, start, end);
>diff --git a/arch/x86/kernel/paravirt.c b/arch/x86/kernel/paravirt.c
>index c0e0101133f3..7bbaa6baf37f 100644
>--- a/arch/x86/kernel/paravirt.c
>+++ b/arch/x86/kernel/paravirt.c
>@@ -121,7 +121,7 @@ DEFINE_STATIC_KEY_TRUE(virt_spin_lock_key);
>
> void __init native_pv_lock_init(void)
> {
>- if (!static_cpu_has(X86_FEATURE_HYPERVISOR))
>+ if (!boot_cpu_has(X86_FEATURE_HYPERVISOR))
> static_branch_disable(&virt_spin_lock_key);
> }
>
>diff --git a/arch/x86/kernel/process.c b/arch/x86/kernel/process.c
>index 58ac7be52c7a..16a7113e91c5 100644
>--- a/arch/x86/kernel/process.c
>+++ b/arch/x86/kernel/process.c
>@@ -236,7 +236,7 @@ static int get_cpuid_mode(void)
>
>static int set_cpuid_mode(struct task_struct *task, unsigned long
>cpuid_enabled)
> {
>- if (!static_cpu_has(X86_FEATURE_CPUID_FAULT))
>+ if (!boot_cpu_has(X86_FEATURE_CPUID_FAULT))
> return -ENODEV;
>
> if (cpuid_enabled)
>@@ -666,7 +666,7 @@ static int prefer_mwait_c1_over_halt(const struct
>cpuinfo_x86 *c)
> if (c->x86_vendor != X86_VENDOR_INTEL)
> return 0;
>
>- if (!cpu_has(c, X86_FEATURE_MWAIT) ||
>static_cpu_has_bug(X86_BUG_MONITOR))
>+ if (!cpu_has(c, X86_FEATURE_MWAIT) ||
>boot_cpu_has_bug(X86_BUG_MONITOR))
> return 0;
>
> return 1;
>diff --git a/arch/x86/kernel/reboot.c b/arch/x86/kernel/reboot.c
>index 725624b6c0c0..d62ebbc5ec78 100644
>--- a/arch/x86/kernel/reboot.c
>+++ b/arch/x86/kernel/reboot.c
>@@ -108,7 +108,7 @@ void __noreturn machine_real_restart(unsigned int
>type)
> write_cr3(real_mode_header->trampoline_pgd);
>
> /* Exiting long mode will fail if CR4.PCIDE is set. */
>- if (static_cpu_has(X86_FEATURE_PCID))
>+ if (boot_cpu_has(X86_FEATURE_PCID))
> cr4_clear_bits(X86_CR4_PCIDE);
> #endif
>
>diff --git a/arch/x86/kernel/vm86_32.c b/arch/x86/kernel/vm86_32.c
>index a092b6b40c6b..6a38717d179c 100644
>--- a/arch/x86/kernel/vm86_32.c
>+++ b/arch/x86/kernel/vm86_32.c
>@@ -369,7 +369,7 @@ static long do_sys_vm86(struct vm86plus_struct
>__user *user_vm86, bool plus)
> preempt_disable();
> tsk->thread.sp0 += 16;
>
>- if (static_cpu_has(X86_FEATURE_SEP)) {
>+ if (boot_cpu_has(X86_FEATURE_SEP)) {
> tsk->thread.sysenter_cs = 0;
> refresh_sysenter_cs(&tsk->thread);
> }
>diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
>index f13a3a24d360..5ed039bf1b58 100644
>--- a/arch/x86/kvm/svm.c
>+++ b/arch/x86/kvm/svm.c
>@@ -835,7 +835,7 @@ static void svm_init_erratum_383(void)
> int err;
> u64 val;
>
>- if (!static_cpu_has_bug(X86_BUG_AMD_TLB_MMATCH))
>+ if (!boot_cpu_has_bug(X86_BUG_AMD_TLB_MMATCH))
> return;
>
> /* Use _safe variants to not break nested virtualization */
>@@ -889,7 +889,7 @@ static int has_svm(void)
> static void svm_hardware_disable(void)
> {
> /* Make sure we clean up behind us */
>- if (static_cpu_has(X86_FEATURE_TSCRATEMSR))
>+ if (boot_cpu_has(X86_FEATURE_TSCRATEMSR))
> wrmsrl(MSR_AMD64_TSC_RATIO, TSC_RATIO_DEFAULT);
>
> cpu_svm_disable();
>@@ -931,7 +931,7 @@ static int svm_hardware_enable(void)
>
> wrmsrl(MSR_VM_HSAVE_PA, page_to_pfn(sd->save_area) << PAGE_SHIFT);
>
>- if (static_cpu_has(X86_FEATURE_TSCRATEMSR)) {
>+ if (boot_cpu_has(X86_FEATURE_TSCRATEMSR)) {
> wrmsrl(MSR_AMD64_TSC_RATIO, TSC_RATIO_DEFAULT);
> __this_cpu_write(current_tsc_ratio, TSC_RATIO_DEFAULT);
> }
>@@ -2247,7 +2247,7 @@ static void svm_vcpu_load(struct kvm_vcpu *vcpu,
>int cpu)
> for (i = 0; i < NR_HOST_SAVE_USER_MSRS; i++)
> rdmsrl(host_save_user_msrs[i], svm->host_user_msrs[i]);
>
>- if (static_cpu_has(X86_FEATURE_TSCRATEMSR)) {
>+ if (boot_cpu_has(X86_FEATURE_TSCRATEMSR)) {
> u64 tsc_ratio = vcpu->arch.tsc_scaling_ratio;
> if (tsc_ratio != __this_cpu_read(current_tsc_ratio)) {
> __this_cpu_write(current_tsc_ratio, tsc_ratio);
>@@ -2255,7 +2255,7 @@ static void svm_vcpu_load(struct kvm_vcpu *vcpu,
>int cpu)
> }
> }
> /* This assumes that the kernel never uses MSR_TSC_AUX */
>- if (static_cpu_has(X86_FEATURE_RDTSCP))
>+ if (boot_cpu_has(X86_FEATURE_RDTSCP))
> wrmsrl(MSR_TSC_AUX, svm->tsc_aux);
>
> if (sd->current_vmcb != svm->vmcb) {
>diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
>index 30a6bcd735ec..0ec24853a0e6 100644
>--- a/arch/x86/kvm/vmx/vmx.c
>+++ b/arch/x86/kvm/vmx/vmx.c
>@@ -6553,7 +6553,7 @@ static void vmx_vcpu_run(struct kvm_vcpu *vcpu)
> if (vcpu->guest_debug & KVM_GUESTDBG_SINGLESTEP)
> vmx_set_interrupt_shadow(vcpu, 0);
>
>- if (static_cpu_has(X86_FEATURE_PKU) &&
>+ if (boot_cpu_has(X86_FEATURE_PKU) &&
> kvm_read_cr4_bits(vcpu, X86_CR4_PKE) &&
> vcpu->arch.pkru != vmx->host_pkru)
> __write_pkru(vcpu->arch.pkru);
>@@ -6633,7 +6633,7 @@ static void vmx_vcpu_run(struct kvm_vcpu *vcpu)
> * back on host, so it is safe to read guest PKRU from current
> * XSAVE.
> */
>- if (static_cpu_has(X86_FEATURE_PKU) &&
>+ if (boot_cpu_has(X86_FEATURE_PKU) &&
> kvm_read_cr4_bits(vcpu, X86_CR4_PKE)) {
> vcpu->arch.pkru = __read_pkru();
> if (vcpu->arch.pkru != vmx->host_pkru)
>diff --git a/arch/x86/mm/dump_pagetables.c
>b/arch/x86/mm/dump_pagetables.c
>index e3cdc85ce5b6..b596ac1eed1c 100644
>--- a/arch/x86/mm/dump_pagetables.c
>+++ b/arch/x86/mm/dump_pagetables.c
>@@ -579,7 +579,7 @@ void ptdump_walk_pgd_level(struct seq_file *m,
>pgd_t *pgd)
>void ptdump_walk_pgd_level_debugfs(struct seq_file *m, pgd_t *pgd, bool
>user)
> {
> #ifdef CONFIG_PAGE_TABLE_ISOLATION
>- if (user && static_cpu_has(X86_FEATURE_PTI))
>+ if (user && boot_cpu_has(X86_FEATURE_PTI))
> pgd = kernel_to_user_pgdp(pgd);
> #endif
> ptdump_walk_pgd_level_core(m, pgd, false, false);
>@@ -592,7 +592,7 @@ void ptdump_walk_user_pgd_level_checkwx(void)
> pgd_t *pgd = INIT_PGD;
>
> if (!(__supported_pte_mask & _PAGE_NX) ||
>- !static_cpu_has(X86_FEATURE_PTI))
>+ !boot_cpu_has(X86_FEATURE_PTI))
> return;
>
> pr_info("x86/mm: Checking user space page tables\n");
>diff --git a/arch/x86/mm/pgtable.c b/arch/x86/mm/pgtable.c
>index 7bd01709a091..3dbf440d4114 100644
>--- a/arch/x86/mm/pgtable.c
>+++ b/arch/x86/mm/pgtable.c
>@@ -190,7 +190,7 @@ static void pgd_dtor(pgd_t *pgd)
>* when PTI is enabled. We need them to map the per-process LDT into the
> * user-space page-table.
> */
>-#define PREALLOCATED_USER_PMDS (static_cpu_has(X86_FEATURE_PTI) ? \
>+#define PREALLOCATED_USER_PMDS (boot_cpu_has(X86_FEATURE_PTI) ? \
> KERNEL_PGD_PTRS : 0)
> #define MAX_PREALLOCATED_USER_PMDS KERNEL_PGD_PTRS
>
>@@ -292,7 +292,7 @@ static void pgd_mop_up_pmds(struct mm_struct *mm,
>pgd_t *pgdp)
>
> #ifdef CONFIG_PAGE_TABLE_ISOLATION
>
>- if (!static_cpu_has(X86_FEATURE_PTI))
>+ if (!boot_cpu_has(X86_FEATURE_PTI))
> return;
>
> pgdp = kernel_to_user_pgdp(pgdp);
>diff --git a/arch/x86/mm/pti.c b/arch/x86/mm/pti.c
>index 4fee5c3003ed..8c9a54ebda60 100644
>--- a/arch/x86/mm/pti.c
>+++ b/arch/x86/mm/pti.c
>@@ -626,7 +626,7 @@ void pti_set_kernel_image_nonglobal(void)
> */
> void __init pti_init(void)
> {
>- if (!static_cpu_has(X86_FEATURE_PTI))
>+ if (!boot_cpu_has(X86_FEATURE_PTI))
> return;
>
> pr_info("enabled\n");
>diff --git a/drivers/cpufreq/amd_freq_sensitivity.c
>b/drivers/cpufreq/amd_freq_sensitivity.c
>index 4ac7c3cf34be..6927a8c0e748 100644
>--- a/drivers/cpufreq/amd_freq_sensitivity.c
>+++ b/drivers/cpufreq/amd_freq_sensitivity.c
>@@ -124,7 +124,7 @@ static int __init amd_freq_sensitivity_init(void)
> PCI_DEVICE_ID_AMD_KERNCZ_SMBUS, NULL);
>
> if (!pcidev) {
>- if (!static_cpu_has(X86_FEATURE_PROC_FEEDBACK))
>+ if (!boot_cpu_has(X86_FEATURE_PROC_FEEDBACK))
> return -ENODEV;
> }
>
>diff --git a/drivers/cpufreq/intel_pstate.c
>b/drivers/cpufreq/intel_pstate.c
>index dd66decf2087..9bbc3dfdebe3 100644
>--- a/drivers/cpufreq/intel_pstate.c
>+++ b/drivers/cpufreq/intel_pstate.c
>@@ -520,7 +520,7 @@ static s16 intel_pstate_get_epb(struct cpudata
>*cpu_data)
> u64 epb;
> int ret;
>
>- if (!static_cpu_has(X86_FEATURE_EPB))
>+ if (!boot_cpu_has(X86_FEATURE_EPB))
> return -ENXIO;
>
> ret = rdmsrl_on_cpu(cpu_data->cpu, MSR_IA32_ENERGY_PERF_BIAS, &epb);
>@@ -534,7 +534,7 @@ static s16 intel_pstate_get_epp(struct cpudata
>*cpu_data, u64 hwp_req_data)
> {
> s16 epp;
>
>- if (static_cpu_has(X86_FEATURE_HWP_EPP)) {
>+ if (boot_cpu_has(X86_FEATURE_HWP_EPP)) {
> /*
> * When hwp_req_data is 0, means that caller didn't read
> * MSR_HWP_REQUEST, so need to read and get EPP.
>@@ -559,7 +559,7 @@ static int intel_pstate_set_epb(int cpu, s16 pref)
> u64 epb;
> int ret;
>
>- if (!static_cpu_has(X86_FEATURE_EPB))
>+ if (!boot_cpu_has(X86_FEATURE_EPB))
> return -ENXIO;
>
> ret = rdmsrl_on_cpu(cpu, MSR_IA32_ENERGY_PERF_BIAS, &epb);
>@@ -607,7 +607,7 @@ static int
>intel_pstate_get_energy_pref_index(struct cpudata *cpu_data)
> if (epp < 0)
> return epp;
>
>- if (static_cpu_has(X86_FEATURE_HWP_EPP)) {
>+ if (boot_cpu_has(X86_FEATURE_HWP_EPP)) {
> if (epp == HWP_EPP_PERFORMANCE)
> return 1;
> if (epp <= HWP_EPP_BALANCE_PERFORMANCE)
>@@ -616,7 +616,7 @@ static int
>intel_pstate_get_energy_pref_index(struct cpudata *cpu_data)
> return 3;
> else
> return 4;
>- } else if (static_cpu_has(X86_FEATURE_EPB)) {
>+ } else if (boot_cpu_has(X86_FEATURE_EPB)) {
> /*
> * Range:
> * 0x00-0x03 : Performance
>@@ -644,7 +644,7 @@ static int
>intel_pstate_set_energy_pref_index(struct cpudata *cpu_data,
>
> mutex_lock(&intel_pstate_limits_lock);
>
>- if (static_cpu_has(X86_FEATURE_HWP_EPP)) {
>+ if (boot_cpu_has(X86_FEATURE_HWP_EPP)) {
> u64 value;
>
> ret = rdmsrl_on_cpu(cpu_data->cpu, MSR_HWP_REQUEST, &value);
>@@ -819,7 +819,7 @@ static void intel_pstate_hwp_set(unsigned int cpu)
> epp = cpu_data->epp_powersave;
> }
> update_epp:
>- if (static_cpu_has(X86_FEATURE_HWP_EPP)) {
>+ if (boot_cpu_has(X86_FEATURE_HWP_EPP)) {
> value &= ~GENMASK_ULL(31, 24);
> value |= (u64)epp << 24;
> } else {
>@@ -844,7 +844,7 @@ static void intel_pstate_hwp_force_min_perf(int
>cpu)
> value |= HWP_MIN_PERF(min_perf);
>
> /* Set EPP/EPB to min */
>- if (static_cpu_has(X86_FEATURE_HWP_EPP))
>+ if (boot_cpu_has(X86_FEATURE_HWP_EPP))
> value |= HWP_ENERGY_PERF_PREFERENCE(HWP_EPP_POWERSAVE);
> else
> intel_pstate_set_epb(cpu, HWP_EPP_BALANCE_POWERSAVE);
>@@ -1191,7 +1191,7 @@ static void __init
>intel_pstate_sysfs_expose_params(void)
> static void intel_pstate_hwp_enable(struct cpudata *cpudata)
> {
> /* First disable HWP notification interrupt as we don't process them
>*/
>- if (static_cpu_has(X86_FEATURE_HWP_NOTIFY))
>+ if (boot_cpu_has(X86_FEATURE_HWP_NOTIFY))
> wrmsrl_on_cpu(cpudata->cpu, MSR_HWP_INTERRUPT, 0x00);
>
> wrmsrl_on_cpu(cpudata->cpu, MSR_PM_ENABLE, 0x1);
>diff --git a/drivers/cpufreq/powernow-k8.c
>b/drivers/cpufreq/powernow-k8.c
>index fb77b39a4ce3..3c12e03fa343 100644
>--- a/drivers/cpufreq/powernow-k8.c
>+++ b/drivers/cpufreq/powernow-k8.c
>@@ -1178,7 +1178,7 @@ static int powernowk8_init(void)
> unsigned int i, supported_cpus = 0;
> int ret;
>
>- if (static_cpu_has(X86_FEATURE_HW_PSTATE)) {
>+ if (boot_cpu_has(X86_FEATURE_HW_PSTATE)) {
> __request_acpi_cpufreq();
> return -ENODEV;
> }
I'm confused here, and as I'm on my phone on an aircraft I can't check the back thread, but am I gathering that this tries to unbreak W^X during module loading by removing functions which use alternatives?
The right thing to do is to apply alternatives before the pages are marked +x-w, like we do with the kernel proper during early boot, if this isn't already the case (sorry again, see above.)
If we *do*, what is the issue here? Although boot_cpu_has() isn't slow (it should in general be possible to reduce to one testb instruction followed by a conditional jump) it seems that "avoiding an alternatives slot" *should* be a *very* weak reason, and seems to me to look like papering over some other problem.
--
Sent from my Android device with K-9 Mail. Please excuse my brevity.
^ permalink raw reply
* Re: [PATCH] x86/cpufeature: Remove __pure attribute to _static_cpu_has()
From: hpa @ 2019-03-07 16:43 UTC (permalink / raw)
To: Borislav Petkov, Nadav Amit
Cc: Rick Edgecombe, Andy Lutomirski, Ingo Molnar, LKML, X86 ML,
Thomas Gleixner, Dave Hansen, Peter Zijlstra, Damian Tometzki,
linux-integrity, LSM List, Andrew Morton, Kernel Hardening,
Linux-MM, Will Deacon, Ard Biesheuvel, Kristen Carlson Accardi,
Dock, Deneen T, Kees Cook, Dave Hansen, Masami Hiramatsu
In-Reply-To: <20190307151036.GD26566@zn.tnic>
On March 7, 2019 7:10:36 AM PST, Borislav Petkov <bp@alien8.de> wrote:
>On Mon, Feb 11, 2019 at 12:32:41PM -0800, Nadav Amit wrote:
>> BTW: the “__pure” attribute is useless when “__always_inline” is
>used.
>> Unless it is intended to be some sort of comment, of course.
>
>---
>From: Borislav Petkov <bp@suse.de>
>Date: Thu, 7 Mar 2019 15:54:51 +0100
>
>__pure is used to make gcc do Common Subexpression Elimination (CSE)
>and thus save subsequent invocations of a function which does a complex
>computation (without side effects). As a simple example:
>
> bool a = _static_cpu_has(x);
> bool b = _static_cpu_has(x);
>
>gets turned into
>
> bool a = _static_cpu_has(x);
> bool b = a;
>
>However, gcc doesn't do CSE with asm()s when those get inlined - like
>it
>is done with _static_cpu_has() - because, for example, the t_yes/t_no
>labels are different for each inlined function body and thus cannot be
>detected as equivalent anymore for the CSE heuristic to hit.
>
>However, this all is beside the point because best it should be avoided
>to have more than one call to _static_cpu_has(X) in the same function
>due to the fact that each such call is an alternatives patch site and
>it
>is simply pointless.
>
>Therefore, drop the __pure attribute as it is not doing anything.
>
>Reported-by: Nadav Amit <nadav.amit@gmail.com>
>Signed-off-by: Borislav Petkov <bp@suse.de>
>Cc: Peter Zijlstra <peterz@infradead.org>
>Cc: x86@kernel.org
>---
> arch/x86/include/asm/cpufeature.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/arch/x86/include/asm/cpufeature.h
>b/arch/x86/include/asm/cpufeature.h
>index e25d11ad7a88..6d6d5cc4302b 100644
>--- a/arch/x86/include/asm/cpufeature.h
>+++ b/arch/x86/include/asm/cpufeature.h
>@@ -162,7 +162,7 @@ extern void clear_cpu_cap(struct cpuinfo_x86 *c,
>unsigned int bit);
>* majority of cases and you should stick to using it as it is generally
> * only two instructions: a RIP-relative MOV and a TEST.
> */
>-static __always_inline __pure bool _static_cpu_has(u16 bit)
>+static __always_inline bool _static_cpu_has(u16 bit)
> {
> asm_volatile_goto("1: jmp 6f\n"
> "2:\n"
Uhm... (a) it is correct, even if the compiler doesn't use it now, it allows the compiler to CSE it in the future; (b) it is documentation; (c) there is an actual bug here: the "volatile" implies a side effect, which in reality is not present, inhibiting CSE.
So the correct fix is to remove "volatile", not remove "__pure".
--
Sent from my Android device with K-9 Mail. Please excuse my brevity.
^ permalink raw reply
* Re: [PATCH 09/27] hibernate: Disable when the kernel is locked down
From: Alan Cox @ 2019-03-07 14:55 UTC (permalink / raw)
To: Matthew Garrett; +Cc: jmorris, linux-security-module, linux-kernel, dhowells
In-Reply-To: <20190306235913.6631-10-matthewgarrett@google.com>
On Wed, 6 Mar 2019 15:58:55 -0800
Matthew Garrett <matthewgarrett@google.com> wrote:
> From: Josh Boyer <jwboyer@fedoraproject.org>
>
> There is currently no way to verify the resume image when returning
> from hibernate. This might compromise the signed modules trust model,
> so until we can work with signed hibernate images we disable it when the
> kernel is locked down.
That one is a bit worrying since whilst the other stuff may be useful in
some business environments, mandatory hibernate not suspend to RAM is a
common corporate IT policy because of concerns about theft and recovery
of memory contents.
Alan
^ permalink raw reply
* Re: [PATCH 02/27] Add a SysRq option to lift kernel lockdown
From: David Howells @ 2019-03-07 15:59 UTC (permalink / raw)
To: Matthew Garrett; +Cc: dhowells, jmorris, linux-security-module, linux-kernel
In-Reply-To: <20190306235913.6631-3-matthewgarrett@google.com>
Matthew Garrett <matthewgarrett@google.com> wrote:
> + /* Ban synthetic events from some sysrq functionality */
> + if ((from == SYSRQ_FROM_PROC || from == SYSRQ_FROM_SYNTHETIC) &&
> + op_p->enable_mask & SYSRQ_DISABLE_USERSPACE)
> + printk("This sysrq operation is disabled from userspace.\n");
> /*
> * Should we check for enabled operations (/proc/sysrq-trigger
> * should not) and is the invoked operation enabled?
> */
> - if (!check_mask || sysrq_on_mask(op_p->enable_mask)) {
> + if (from == SYSRQ_FROM_KERNEL || sysrq_on_mask(op_p->enable_mask)) {
There's some missing logic here. Probably an else is missing, but it seems
more than that.
David
^ permalink raw reply
* [PATCH] x86/cpufeature: Remove __pure attribute to _static_cpu_has()
From: Borislav Petkov @ 2019-03-07 15:10 UTC (permalink / raw)
To: Nadav Amit
Cc: Rick Edgecombe, Andy Lutomirski, Ingo Molnar, LKML, X86 ML,
H. Peter Anvin, Thomas Gleixner, Dave Hansen, Peter Zijlstra,
Damian Tometzki, linux-integrity, LSM List, Andrew Morton,
Kernel Hardening, Linux-MM, Will Deacon, Ard Biesheuvel,
Kristen Carlson Accardi, Dock, Deneen T, Kees Cook, Dave Hansen,
Masami Hiramatsu
In-Reply-To: <A55214F3-CDC0-44C4-AFB6-7E8E23CC6F85@gmail.com>
On Mon, Feb 11, 2019 at 12:32:41PM -0800, Nadav Amit wrote:
> BTW: the “__pure” attribute is useless when “__always_inline” is used.
> Unless it is intended to be some sort of comment, of course.
---
From: Borislav Petkov <bp@suse.de>
Date: Thu, 7 Mar 2019 15:54:51 +0100
__pure is used to make gcc do Common Subexpression Elimination (CSE)
and thus save subsequent invocations of a function which does a complex
computation (without side effects). As a simple example:
bool a = _static_cpu_has(x);
bool b = _static_cpu_has(x);
gets turned into
bool a = _static_cpu_has(x);
bool b = a;
However, gcc doesn't do CSE with asm()s when those get inlined - like it
is done with _static_cpu_has() - because, for example, the t_yes/t_no
labels are different for each inlined function body and thus cannot be
detected as equivalent anymore for the CSE heuristic to hit.
However, this all is beside the point because best it should be avoided
to have more than one call to _static_cpu_has(X) in the same function
due to the fact that each such call is an alternatives patch site and it
is simply pointless.
Therefore, drop the __pure attribute as it is not doing anything.
Reported-by: Nadav Amit <nadav.amit@gmail.com>
Signed-off-by: Borislav Petkov <bp@suse.de>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: x86@kernel.org
---
arch/x86/include/asm/cpufeature.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/x86/include/asm/cpufeature.h b/arch/x86/include/asm/cpufeature.h
index e25d11ad7a88..6d6d5cc4302b 100644
--- a/arch/x86/include/asm/cpufeature.h
+++ b/arch/x86/include/asm/cpufeature.h
@@ -162,7 +162,7 @@ extern void clear_cpu_cap(struct cpuinfo_x86 *c, unsigned int bit);
* majority of cases and you should stick to using it as it is generally
* only two instructions: a RIP-relative MOV and a TEST.
*/
-static __always_inline __pure bool _static_cpu_has(u16 bit)
+static __always_inline bool _static_cpu_has(u16 bit)
{
asm_volatile_goto("1: jmp 6f\n"
"2:\n"
--
2.21.0
--
Regards/Gruss,
Boris.
Good mailing practices for 400: avoid top-posting and trim the reply.
^ permalink raw reply related
* [PATCH 1/1] RFC: initmem: introduce CONFIG_INIT_ALL_MEMORY and CONFIG_INIT_ALL_STACK
From: Alexander Potapenko @ 2019-03-07 13:35 UTC (permalink / raw)
To: yamada.masahiro, jmorris, serge
Cc: linux-security-module, linux-kbuild, ndesaulniers, kcc, dvyukov,
keescook, sspatil
In-Reply-To: <20190307133514.44378-1-glider@google.com>
This patch is a part of a bigger initiative to allow initializing
heap/stack memory in the Linux kernels by default.
The rationale behind doing so is to reduce the severity of bugs caused
by using uninitialized memory.
CONFIG_INIT_ALL_MEMORY is going to be an umbrella config for options
that force heap and stack initialization.
CONFIG_INIT_ALL_STACK turns on stack initialization based on the
-ftrivial-auto-var-init Clang flag.
-ftrivial-auto-var-init is a Clang flag that provides trivial
initializers for uninitialized local variables, variable fields and
padding.
It has three possible values:
pattern - uninitialized locals are filled with a fixed pattern
(mostly 0xAA on 64-bit platforms, see https://reviews.llvm.org/D54604
for more details) likely to cause crashes when uninitialized value is
used;
zero (it's still debated whether this flag makes it to the official
Clang release) - uninitialized locals are filled with zeroes;
uninitialized (default) - uninitialized locals are left intact.
The proposed config builds the kernel with
-ftrivial-auto-var-init=pattern.
Developers have the possibility to opt-out of this feature on a
per-file (by using the INIT_ALL_MEMORY_ Makefile prefix) or per-variable
(by using __attribute__((uninitialized))) basis.
Signed-off-by: Alexander Potapenko <glider@google.com>
Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
Cc: James Morris <jmorris@namei.org>
Cc: "Serge E. Hallyn" <serge@hallyn.com>
Cc: Nick Desaulniers <ndesaulniers@google.com>
Cc: Kostya Serebryany <kcc@google.com>
Cc: Dmitry Vyukov <dvyukov@google.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Sandeep Patil <sspatil@android.com>
Cc: linux-security-module@vger.kernel.org
Cc: linux-kbuild@vger.kernel.org
---
Makefile | 3 ++-
scripts/Makefile.initmem | 17 +++++++++++++++++
scripts/Makefile.lib | 6 ++++++
security/Kconfig | 1 +
security/Kconfig.initmem | 22 ++++++++++++++++++++++
5 files changed, 48 insertions(+), 1 deletion(-)
create mode 100644 scripts/Makefile.initmem
create mode 100644 security/Kconfig.initmem
diff --git a/Makefile b/Makefile
index f070e0d65186..028ca37878fd 100644
--- a/Makefile
+++ b/Makefile
@@ -448,7 +448,7 @@ export HOSTCXX KBUILD_HOSTCXXFLAGS LDFLAGS_MODULE CHECK CHECKFLAGS
export KBUILD_CPPFLAGS NOSTDINC_FLAGS LINUXINCLUDE OBJCOPYFLAGS KBUILD_LDFLAGS
export KBUILD_CFLAGS CFLAGS_KERNEL CFLAGS_MODULE
-export CFLAGS_KASAN CFLAGS_KASAN_NOSANITIZE CFLAGS_UBSAN
+export CFLAGS_KASAN CFLAGS_KASAN_NOSANITIZE CFLAGS_UBSAN CFLAGS_INITMEM
export KBUILD_AFLAGS AFLAGS_KERNEL AFLAGS_MODULE
export KBUILD_AFLAGS_MODULE KBUILD_CFLAGS_MODULE KBUILD_LDFLAGS_MODULE
export KBUILD_AFLAGS_KERNEL KBUILD_CFLAGS_KERNEL
@@ -840,6 +840,7 @@ KBUILD_ARFLAGS := $(call ar-option,D)
include scripts/Makefile.kasan
include scripts/Makefile.extrawarn
include scripts/Makefile.ubsan
+include scripts/Makefile.initmem
# Add any arch overrides and user supplied CPPFLAGS, AFLAGS and CFLAGS as the
# last assignments
diff --git a/scripts/Makefile.initmem b/scripts/Makefile.initmem
new file mode 100644
index 000000000000..f49be398f2c1
--- /dev/null
+++ b/scripts/Makefile.initmem
@@ -0,0 +1,17 @@
+ifdef CONFIG_INIT_ALL_MEMORY
+
+# Clang's -ftrivial-auto-var-init=pattern flag initializes the
+# uninitialized parts of local variables (including fields and padding)
+# with a fixed pattern (0xAA in most cases).
+ifdef CONFIG_INIT_ALL_STACK
+ CFLAGS_INITMEM := -ftrivial-auto-var-init=pattern
+endif
+
+ifeq ($(call cc-option, $(CFLAGS_INITMEM) -Werror),)
+ ifneq ($(CONFIG_COMPILE_TEST),y)
+ $(warning Cannot use CONFIG_INIT_ALL_MEMORY: \
+ -ftrivial-auto-var-init is not supported by compiler)
+ endif
+endif
+
+endif
diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
index 12b88d09c3a4..53d18fd15c79 100644
--- a/scripts/Makefile.lib
+++ b/scripts/Makefile.lib
@@ -131,6 +131,12 @@ _c_flags += $(if $(patsubst n%,, \
$(CFLAGS_UBSAN))
endif
+ifeq ($(CONFIG_INIT_ALL_MEMORY),y)
+_c_flags += $(if $(patsubst n%,, \
+ $(INIT_ALL_MEMORY_$(basetarget).o)$(INIT_ALL_MEMORY)y), \
+ $(CFLAGS_INITMEM))
+endif
+
ifeq ($(CONFIG_KCOV),y)
_c_flags += $(if $(patsubst n%,, \
$(KCOV_INSTRUMENT_$(basetarget).o)$(KCOV_INSTRUMENT)$(CONFIG_KCOV_INSTRUMENT_ALL)), \
diff --git a/security/Kconfig b/security/Kconfig
index e4fe2f3c2c65..cc12a39424dd 100644
--- a/security/Kconfig
+++ b/security/Kconfig
@@ -230,6 +230,7 @@ config STATIC_USERMODEHELPER_PATH
If you wish for all usermode helper programs to be disabled,
specify an empty string here (i.e. "").
+source "security/Kconfig.initmem"
source "security/selinux/Kconfig"
source "security/smack/Kconfig"
source "security/tomoyo/Kconfig"
diff --git a/security/Kconfig.initmem b/security/Kconfig.initmem
new file mode 100644
index 000000000000..5ac3cf3e7f88
--- /dev/null
+++ b/security/Kconfig.initmem
@@ -0,0 +1,22 @@
+menu "Initialize all memory"
+
+config INIT_ALL_MEMORY
+ bool "Initialize all memory"
+ default n
+ help
+ Enforce memory initialization to mitigate infoleaks and make
+ the control-flow bugs depending on uninitialized values more
+ deterministic.
+
+if INIT_ALL_MEMORY
+
+config INIT_ALL_STACK
+ bool "Initialize all stack"
+ depends on INIT_ALL_MEMORY
+ default y
+ help
+ Initialize uninitialized stack data with a 0xAA pattern.
+ This config option only supports Clang builds at the moment.
+
+endif # INIT_ALL_MEMORY
+endmenu
--
2.21.0.352.gf09ad66450-goog
^ permalink raw reply related
* [PATCH 0/1] RFC: introduce CONFIG_INIT_ALL_MEMORY
From: Alexander Potapenko @ 2019-03-07 13:35 UTC (permalink / raw)
To: yamada.masahiro, jmorris, serge
Cc: linux-security-module, linux-kbuild, ndesaulniers, kcc, dvyukov,
keescook, sspatil
This patch is a part of a bigger initiative to allow initializing
heap/stack memory in the Linux kernels by default.
The rationale behind doing so is to reduce the severity of bugs caused
by using uninitialized memory.
Over the last two years KMSAN (https://github.com/google/kmsan/) has
found more than a hundred bugs running in a really moderate setup (orders
of magnitude less CPU/months than KASAN). Some of those bugs led to
information leaks if uninitialized memory was copied to the userspace,
other could cause DoS because of subverted control flow.
A lot more bugs remain uncovered, so we want to provide the distros and OS
vendors with a last resort measure to mitigate such bugs.
Our plan is to introduce configuration flags to force initialization of
stack and heap variables with a fixed pattern.
This is going to render information leaks inefficient (as we'll only leak
pattern data) and make uses of uninitialized values in conditions more
deterministic and discoverable.
The stack instrumentation part is only implemented in Clang at the moment
(see https://reviews.llvm.org/D54604 ; there's also a GCC feature request
for a similar flag: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87210).
The heap initialization part (still work in progress) is going to be
compiler-agnostic.
It'll be possible to opt-out from stack initalization by annotating local
variables as knowingly initialized, or by disabling the feature for a
single translation unit.
Opting out for a heap allocation will be done using a special GFP flag.
Alexander Potapenko (1):
initmem: introduce CONFIG_INIT_ALL_MEMORY and CONFIG_INIT_ALL_STACK
Makefile | 3 ++-
scripts/Makefile.initmem | 17 +++++++++++++++++
scripts/Makefile.lib | 6 ++++++
security/Kconfig | 1 +
security/Kconfig.initmem | 22 ++++++++++++++++++++++
5 files changed, 48 insertions(+), 1 deletion(-)
create mode 100644 scripts/Makefile.initmem
create mode 100644 security/Kconfig.initmem
--
2.21.0.352.gf09ad66450-goog
^ permalink raw reply
* RE: [RFC PATCH 0/2] Create CAAM HW key in linux keyring and use in dmcrypt
From: Franck Lenormand @ 2019-03-07 13:17 UTC (permalink / raw)
To: David Howells
Cc: linux-kernel@vger.kernel.org,
linux-security-module@vger.kernel.org, keyrings@vger.kernel.org,
Horia Geanta, Silvano Di Ninno, agk@redhat.com,
snitzer@redhat.com, dm-devel@redhat.com, jmorris@namei.org,
serge@hallyn.com
In-Reply-To: <11177.1551893395@warthog.procyon.org.uk>
> -----Original Message-----
> From: David Howells <dhowells@redhat.com>
> Sent: Wednesday, March 6, 2019 6:30 PM
> To: Franck Lenormand <franck.lenormand@nxp.com>
> Cc: dhowells@redhat.com; linux-kernel@vger.kernel.org; linux-security-
> module@vger.kernel.org; keyrings@vger.kernel.org; Horia Geanta
> <horia.geanta@nxp.com>; Silvano Di Ninno <silvano.dininno@nxp.com>;
> agk@redhat.com; snitzer@redhat.com; dm-devel@redhat.com;
> jmorris@namei.org; serge@hallyn.com
> Subject: Re: [RFC PATCH 0/2] Create CAAM HW key in linux keyring and use in
> dmcrypt
>
> Franck LENORMAND <franck.lenormand@nxp.com> wrote:
>
> > The capacity to generate or load keys already available in the Linux
> > key retention service does not allows to exploit CAAM capabilities
> > hence we need to create a new key_type. The new key type "caam_tk"
> allows to:
> > - Create a black key from random
> > - Create a black key from a red key
> > - Load a black blob to retrieve the black key
>
> Is it possible that this could be done through an existing key type, such as the
> asymmetric, trusted or encrypted key typed?
>
> David
Hello David,
I didn't know about asymmetric key type so I looked it up, from my
observation, it would not be possible to use it for the caam_tk as
we must perform operations on the data provided.
The name " asymmetric " is also misleading for the use we would have.
The trusted and encrypted does not provides the necessary
callbacks to do what we would need or require huge modifications.
I would like, for this series to focus on the change related to
dm-crypt. In effect, it is currently not possible to pass a key
from the asymmetric key type to it.
Franck
^ permalink raw reply
* RE: [RFC PATCH 0/2] Create CAAM HW key in linux keyring and use in dmcrypt
From: Franck Lenormand @ 2019-03-07 13:02 UTC (permalink / raw)
To: Jan Lübbe, linux-kernel@vger.kernel.org,
linux-security-module@vger.kernel.org, keyrings@vger.kernel.org
Cc: Horia Geanta, Silvano Di Ninno, agk@redhat.com,
snitzer@redhat.com, dm-devel@redhat.com, dhowells@redhat.com,
jmorris@namei.org, serge@hallyn.com, David Gstir
In-Reply-To: <1551890866.5086.125.camel@pengutronix.de>
> -----Original Message-----
> From: Jan Lübbe <jlu@pengutronix.de>
> Sent: Wednesday, March 6, 2019 5:48 PM
> To: Franck Lenormand <franck.lenormand@nxp.com>; linux-
> kernel@vger.kernel.org; linux-security-module@vger.kernel.org;
> keyrings@vger.kernel.org
> Cc: Horia Geanta <horia.geanta@nxp.com>; Silvano Di Ninno
> <silvano.dininno@nxp.com>; agk@redhat.com; snitzer@redhat.com; dm-
> devel@redhat.com; dhowells@redhat.com; jmorris@namei.org;
> serge@hallyn.com; David Gstir <david@sigma-star.at>
> Subject: Re: [RFC PATCH 0/2] Create CAAM HW key in linux keyring and use in
> dmcrypt
>
> Hi Franck,
>
> thanks for working on this!
>
> On Fri, 2019-03-01 at 17:09 +0100, Franck LENORMAND wrote:
> > The creation of such structures and its use was not exposed to
> > userspace so it was complicated to use and required custom
> > development. We would like to ease this using interface which are known
> and used:
> > - Linux key retention service : Allow to generate or load keys in a
> > keyring which can be used by applications.
> > - dm-crypt : device mapper allowing to encrypt data.
> >
> > The capacity to generate or load keys already available in the Linux
> > key retention service does not allows to exploit CAAM capabilities
> > hence we need to create a new key_type. The new key type "caam_tk"
> allows to:
> > - Create a black key from random
> > - Create a black key from a red key
> > - Load a black blob to retrieve the black key
>
> On 2018-07-23, Udit Agarwal <udit.agarwal@nxp.com> sent a series which
> seems related to this:
> [PATCH v2 1/2] security/keys/secure_key: Adds the secure key support based
> on CAAM.
> [PATCH v2 2/2] encrypted_keys: Adds support for secure key-type as master
> key.
>
> Is this series intended to continue that work and cover the same uses- cases?
>
> If I remember correctly, the CAAM also supports marking blobs to allow or
> disallow exporting the encapsulated key from the hardware. Or is this
> unneeded and we could encrypt/decrypt other (less critical) key material
> against the tk(cbc(aes)) CAAM key via the keyring mechanisms?
>
> Best regards,
> Jan
> --
> Pengutronix e.K. | |
> Industrial Linux Solutions |
> https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fww
> w.pengutronix.de%2F&data=02%7C01%7Cfranck.lenormand%40nxp.co
> m%7C29ab89e99f9040e30aa908d6a2537967%7C686ea1d3bc2b4c6fa92cd99c
> 5c301635%7C0%7C0%7C636874876732153093&sdata=byKD4SmYfzMTs
> FK6cX5L81%2B9hxxqTbnOLf7%2BAJgXylU%3D&reserved=0 |
> Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
> Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
Hello Jan,
The current series is aimed to change which key type is acceptable in dm-crypt. Without
this change, the key type cannot be integrated properly.
About the key type implementation:
The implementation of the key type is an improvement related to the work of Udit Agarwal.
The transform tk(cbc(aes)) is intended to receive key material whose format can change
hence the "tagging" to handle it properly. It could, for example, handle plain keys, black keys or blobs as key material and this material would be stored in keyring.
Regards,
Franck
^ permalink raw reply
* [PATCH 1/1] smack: removal of global rule list
From: Vishal Goel @ 2019-03-07 11:25 UTC (permalink / raw)
To: casey, linux-security-module, linux-kernel
Cc: pankaj.m, a.sahrawat, Vishal Goel
In-Reply-To: <CGME20190307113608epcas5p1be102a5a4592055ab3c97df8a8339d98@epcas5p1.samsung.com>
In this patch, global rule list has been removed. Now all
smack rules will be read using "smack_known_list". This list contains
all the smack labels and internally each smack label structure
maintains the list of smack rules corresponding to that smack label.
So there is no need to maintain extra list.
1) Small Memory Optimization
For eg. if there are 20000 rules, then it will save 625KB(20000*32),
which is critical for small embedded systems.
2) Reducing the time taken in writing rules on load/load2 interface
3) Since global rule list is just used to read the rules, so there
will be no performance impact on system
Signed-off-by: Vishal Goel <vishal.goel@samsung.com>
Signed-off-by: Amit Sahrawat <a.sahrawat@samsung.com>
---
security/smack/smackfs.c | 53 ++++++++++++++----------------------------------
1 file changed, 15 insertions(+), 38 deletions(-)
diff --git a/security/smack/smackfs.c b/security/smack/smackfs.c
index f6482e5..2a8a1f5 100644
--- a/security/smack/smackfs.c
+++ b/security/smack/smackfs.c
@@ -67,7 +67,6 @@ enum smk_inos {
/*
* List locks
*/
-static DEFINE_MUTEX(smack_master_list_lock);
static DEFINE_MUTEX(smack_cipso_lock);
static DEFINE_MUTEX(smack_ambient_lock);
static DEFINE_MUTEX(smk_net4addr_lock);
@@ -134,15 +133,7 @@ enum smk_inos {
/*
* Rule lists are maintained for each label.
- * This master list is just for reading /smack/load and /smack/load2.
*/
-struct smack_master_list {
- struct list_head list;
- struct smack_rule *smk_rule;
-};
-
-static LIST_HEAD(smack_rule_list);
-
struct smack_parsed_rule {
struct smack_known *smk_subject;
struct smack_known *smk_object;
@@ -211,7 +202,6 @@ static void smk_netlabel_audit_set(struct netlbl_audit *nap)
* @srp: the rule to add or replace
* @rule_list: the list of rules
* @rule_lock: the rule list lock
- * @global: if non-zero, indicates a global rule
*
* Looks through the current subject/object/access list for
* the subject/object pair and replaces the access that was
@@ -223,10 +213,9 @@ static void smk_netlabel_audit_set(struct netlbl_audit *nap)
*/
static int smk_set_access(struct smack_parsed_rule *srp,
struct list_head *rule_list,
- struct mutex *rule_lock, int global)
+ struct mutex *rule_lock)
{
struct smack_rule *sp;
- struct smack_master_list *smlp;
int found = 0;
int rc = 0;
@@ -258,22 +247,6 @@ static int smk_set_access(struct smack_parsed_rule *srp,
sp->smk_access = srp->smk_access1 & ~srp->smk_access2;
list_add_rcu(&sp->list, rule_list);
- /*
- * If this is a global as opposed to self and a new rule
- * it needs to get added for reporting.
- */
- if (global) {
- mutex_unlock(rule_lock);
- smlp = kzalloc(sizeof(*smlp), GFP_KERNEL);
- if (smlp != NULL) {
- smlp->smk_rule = sp;
- mutex_lock(&smack_master_list_lock);
- list_add_rcu(&smlp->list, &smack_rule_list);
- mutex_unlock(&smack_master_list_lock);
- } else
- rc = -ENOMEM;
- return rc;
- }
}
out:
@@ -540,9 +513,9 @@ static ssize_t smk_write_rules_list(struct file *file, const char __user *buf,
if (rule_list == NULL)
rc = smk_set_access(&rule, &rule.smk_subject->smk_rules,
- &rule.smk_subject->smk_rules_lock, 1);
+ &rule.smk_subject->smk_rules_lock);
else
- rc = smk_set_access(&rule, rule_list, rule_lock, 0);
+ rc = smk_set_access(&rule, rule_list, rule_lock);
if (rc)
goto out;
@@ -636,21 +609,23 @@ static void smk_rule_show(struct seq_file *s, struct smack_rule *srp, int max)
static void *load2_seq_start(struct seq_file *s, loff_t *pos)
{
- return smk_seq_start(s, pos, &smack_rule_list);
+ return smk_seq_start(s, pos, &smack_known_list);
}
static void *load2_seq_next(struct seq_file *s, void *v, loff_t *pos)
{
- return smk_seq_next(s, v, pos, &smack_rule_list);
+ return smk_seq_next(s, v, pos, &smack_known_list);
}
static int load_seq_show(struct seq_file *s, void *v)
{
struct list_head *list = v;
- struct smack_master_list *smlp =
- list_entry_rcu(list, struct smack_master_list, list);
+ struct smack_rule *srp;
+ struct smack_known *skp =
+ list_entry_rcu(list, struct smack_known, list);
- smk_rule_show(s, smlp->smk_rule, SMK_LABELLEN);
+ list_for_each_entry_rcu(srp, &skp->smk_rules, list)
+ smk_rule_show(s, srp, SMK_LABELLEN);
return 0;
}
@@ -2352,10 +2327,12 @@ static ssize_t smk_write_access(struct file *file, const char __user *buf,
static int load2_seq_show(struct seq_file *s, void *v)
{
struct list_head *list = v;
- struct smack_master_list *smlp =
- list_entry_rcu(list, struct smack_master_list, list);
+ struct smack_rule *srp;
+ struct smack_known *skp =
+ list_entry_rcu(list, struct smack_known, list);
- smk_rule_show(s, smlp->smk_rule, SMK_LONGLABEL);
+ list_for_each_entry_rcu(srp, &skp->smk_rules, list)
+ smk_rule_show(s, srp, SMK_LONGLABEL);
return 0;
}
--
1.9.1
^ permalink raw reply related
* [PATCH 1/1] Smack :- In this patch, global rule list has been removed. Now all smack rules will be read using "smack_known_list". This list contains all the smack labels and internally each smack label structure maintains the list of smack rules corresponding to that smack label. So there is no need to maintain extra list.
From: Vishal Goel @ 2019-03-07 9:23 UTC (permalink / raw)
To: casey, linux-security-module, linux-kernel
Cc: pankaj.m, a.sahrawat, Vishal Goel
In-Reply-To: <CGME20190307093333epcas5p25cc50478585532ea2f7c4a1275b04279@epcas5p2.samsung.com>
1) Small Memory Optimization
For eg. if there are 20000 rules, then it will save 625KB(20000*32),
which is critical for small embedded systems.
2) Reducing the time taken in writing rules on load/load2 interface
3) Since global rule list is just used to read the rules, so there
will be no performance impact on system
Signed-off-by: Vishal Goel <vishal.goel@samsung.com>
Signed-off-by: Amit Sahrawat <a.sahrawat@samsung.com>
---
security/smack/smackfs.c | 53 ++++++++++++++----------------------------------
1 file changed, 15 insertions(+), 38 deletions(-)
diff --git a/security/smack/smackfs.c b/security/smack/smackfs.c
index f6482e5..2a8a1f5 100644
--- a/security/smack/smackfs.c
+++ b/security/smack/smackfs.c
@@ -67,7 +67,6 @@ enum smk_inos {
/*
* List locks
*/
-static DEFINE_MUTEX(smack_master_list_lock);
static DEFINE_MUTEX(smack_cipso_lock);
static DEFINE_MUTEX(smack_ambient_lock);
static DEFINE_MUTEX(smk_net4addr_lock);
@@ -134,15 +133,7 @@ enum smk_inos {
/*
* Rule lists are maintained for each label.
- * This master list is just for reading /smack/load and /smack/load2.
*/
-struct smack_master_list {
- struct list_head list;
- struct smack_rule *smk_rule;
-};
-
-static LIST_HEAD(smack_rule_list);
-
struct smack_parsed_rule {
struct smack_known *smk_subject;
struct smack_known *smk_object;
@@ -211,7 +202,6 @@ static void smk_netlabel_audit_set(struct netlbl_audit *nap)
* @srp: the rule to add or replace
* @rule_list: the list of rules
* @rule_lock: the rule list lock
- * @global: if non-zero, indicates a global rule
*
* Looks through the current subject/object/access list for
* the subject/object pair and replaces the access that was
@@ -223,10 +213,9 @@ static void smk_netlabel_audit_set(struct netlbl_audit *nap)
*/
static int smk_set_access(struct smack_parsed_rule *srp,
struct list_head *rule_list,
- struct mutex *rule_lock, int global)
+ struct mutex *rule_lock)
{
struct smack_rule *sp;
- struct smack_master_list *smlp;
int found = 0;
int rc = 0;
@@ -258,22 +247,6 @@ static int smk_set_access(struct smack_parsed_rule *srp,
sp->smk_access = srp->smk_access1 & ~srp->smk_access2;
list_add_rcu(&sp->list, rule_list);
- /*
- * If this is a global as opposed to self and a new rule
- * it needs to get added for reporting.
- */
- if (global) {
- mutex_unlock(rule_lock);
- smlp = kzalloc(sizeof(*smlp), GFP_KERNEL);
- if (smlp != NULL) {
- smlp->smk_rule = sp;
- mutex_lock(&smack_master_list_lock);
- list_add_rcu(&smlp->list, &smack_rule_list);
- mutex_unlock(&smack_master_list_lock);
- } else
- rc = -ENOMEM;
- return rc;
- }
}
out:
@@ -540,9 +513,9 @@ static ssize_t smk_write_rules_list(struct file *file, const char __user *buf,
if (rule_list == NULL)
rc = smk_set_access(&rule, &rule.smk_subject->smk_rules,
- &rule.smk_subject->smk_rules_lock, 1);
+ &rule.smk_subject->smk_rules_lock);
else
- rc = smk_set_access(&rule, rule_list, rule_lock, 0);
+ rc = smk_set_access(&rule, rule_list, rule_lock);
if (rc)
goto out;
@@ -636,21 +609,23 @@ static void smk_rule_show(struct seq_file *s, struct smack_rule *srp, int max)
static void *load2_seq_start(struct seq_file *s, loff_t *pos)
{
- return smk_seq_start(s, pos, &smack_rule_list);
+ return smk_seq_start(s, pos, &smack_known_list);
}
static void *load2_seq_next(struct seq_file *s, void *v, loff_t *pos)
{
- return smk_seq_next(s, v, pos, &smack_rule_list);
+ return smk_seq_next(s, v, pos, &smack_known_list);
}
static int load_seq_show(struct seq_file *s, void *v)
{
struct list_head *list = v;
- struct smack_master_list *smlp =
- list_entry_rcu(list, struct smack_master_list, list);
+ struct smack_rule *srp;
+ struct smack_known *skp =
+ list_entry_rcu(list, struct smack_known, list);
- smk_rule_show(s, smlp->smk_rule, SMK_LABELLEN);
+ list_for_each_entry_rcu(srp, &skp->smk_rules, list)
+ smk_rule_show(s, srp, SMK_LABELLEN);
return 0;
}
@@ -2352,10 +2327,12 @@ static ssize_t smk_write_access(struct file *file, const char __user *buf,
static int load2_seq_show(struct seq_file *s, void *v)
{
struct list_head *list = v;
- struct smack_master_list *smlp =
- list_entry_rcu(list, struct smack_master_list, list);
+ struct smack_rule *srp;
+ struct smack_known *skp =
+ list_entry_rcu(list, struct smack_known, list);
- smk_rule_show(s, smlp->smk_rule, SMK_LONGLABEL);
+ list_for_each_entry_rcu(srp, &skp->smk_rules, list)
+ smk_rule_show(s, srp, SMK_LONGLABEL);
return 0;
}
--
1.9.1
^ permalink raw reply related
* Re: [PATCH v7 0/7] Allow initializing the kernfs node's secctx based on its parent
From: Ondrej Mosnacek @ 2019-03-07 9:01 UTC (permalink / raw)
To: Casey Schaufler
Cc: selinux, Paul Moore, Stephen Smalley, Linux Security Module list,
Tejun Heo, Serge E . Hallyn, Greg Kroah-Hartman, James Morris,
linux-fsdevel, cgroups
In-Reply-To: <7f92ac61-c72b-ab31-c757-c2ac1bcf7b08@schaufler-ca.com>
On Wed, Mar 6, 2019 at 7:04 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
> On 3/6/2019 7:54 AM, Ondrej Mosnacek wrote:
> > On Fri, Feb 22, 2019 at 3:57 PM Ondrej Mosnacek <omosnace@redhat.com> wrote:
> >> TL;DR:
> >> This series adds a new security hook that allows to initialize the security
> >> context of kernfs properly, taking into account the parent context (and
> >> possibly other attributes). Kernfs nodes require special handling here, since
> >> they are not bound to specific inodes/superblocks, but instead represent the
> >> backing tree structure that is used to build the VFS tree when the kernfs
> >> tree is mounted.
> >>
> >> Changes in v7:
> >> - simplify the new security hook's interface
> >> - rather than trying to extract kernfs data into common structures, just
> >> pass the kernfs nodes themselves and add helper functions to
> >> <linux/kernfs.h> for accessing their security xattrs
> >> - in case other LSMs need more kernfs node attributes than the file mode
> >> (uid/gid/...), they can simply add new helpers to <linux/kernfs.h> as
> >> needed
> >> - refactor "kernfs: use simple_xattrs for security attributes" to keep using
> >> a single common simple_xattrs structure
> >> - turns out having two separate simple_xattrs wouldn't work right (see
> >> the definition of simple_xattr_list() in fs/xattr.c)
> >> - drop unnecessary initializations from inode_doinit_use_xattr()
> >> - move the IOP_XATTR check out of inode_doinit_use_xattr()
> >> - add two kernfs cleanup patches
> >> - these could be applied independently, but the rest of the patches depend on
> >> them, so I'd rather they stay bundled with the rest to avoid cross-tree
> >> conflicts
> >>
> >> v6: https://lore.kernel.org/selinux/20190214095015.16032-1-omosnace@redhat.com/T/
> >> Changes in v6:
> >> - remove copy-pasted duplicate macro definition
> >>
> >> v5: https://lore.kernel.org/selinux/20190205110638.30782-1-omosnace@redhat.com/T/
> >> Changes in v5:
> >> - fix misplaced semicolon detected by 0day robot
> >>
> >> v4: https://lore.kernel.org/selinux/20190205085915.5183-1-omosnace@redhat.com/T/
> >> Changes in v4:
> >> - reorder and rename hook arguments
> >> - avoid allocating kernfs_iattrs unless needed
> >>
> >> v3: https://lore.kernel.org/selinux/20190130114150.27807-1-omosnace@redhat.com/T/
> >> Changes in v3:
> >> - rename the hook to "kernfs_init_security"
> >> - change the hook interface to simply pass pointers to struct iattr and
> >> struct simple_xattrs of both the new node and its parent
> >> - add full security xattr support to kernfs (and fixup SELinux behavior
> >> to handle it properly)
> >>
> >> v2: https://lore.kernel.org/selinux/20190109162830.8309-1-omosnace@redhat.com/T/
> >> Changes in v2:
> >> - add docstring for the new hook in union security_list_options
> >> - initialize *ctx to NULL and *ctxlen to 0 in case the hook is not
> >> implemented
> >>
> >> v1: https://lore.kernel.org/selinux/20190109091028.24485-1-omosnace@redhat.com/T/
> >>
> >> The kernfs nodes initially do not store any security context and rely on
> >> the LSM to assign some default context to inodes created over them. Kernfs
> >> inodes, however, allow setting an explicit context via the *setxattr(2)
> >> syscalls, in which case the context is stored inside the kernfs node's
> >> internal structure.
> >>
> >> SELinux (and possibly other LSMs) initialize the context of newly created
> >> FS objects based on the parent object's context (usually the child inherits
> >> the parent's context, unless the policy dictates otherwise). This is done
> >> by hooking the creation of the new inode corresponding to the newly created
> >> file/directory via security_inode_init_security() (most filesystems always
> >> create a fresh inode when a new FS object is created). However, kernfs nodes
> >> can be created "behind the scenes" while the filesystem is not mounted
> >> anywhere and thus no inodes can exist for them yet.
> >>
> >> Therefore, to allow maintaining similar behavior for kernfs nodes, a new
> >> LSM hook is needed, which will allow initializing the kernfs node's
> >> security context based on its own attributes and those of the parent's
> >> node.
> >>
> >> The main motivation for this change is that the userspace users of cgroupfs
> >> (which is built on kernfs) expect the usual security context inheritance
> >> to work under SELinux (see [1] and [2]). This functionality is required for
> >> better confinement of containers under SELinux.
> >>
> >> Patch 1/7 simplifies the kernfs_iattrs structure and patch 2/7 optimizes
> >> kernfs to not allocate kernfs_iattrs when getting the value of an xattr.
> >>
> >> Patch 3/7 changes SELinux to fetch security context from extended
> >> attributes on kernfs filesystems, falling back to genfs-defined context
> >> if that fails. Without this patch the 4/7 would be a regression for
> >> SELinux (due to the removal of ...notifysecctx() call.
> >>
> >> Patch 4/7 implements full security xattr support in kernfs using
> >> simple_xattrs; patch 5/7 adds the new LSM hook; patch 6/7 implements the
> >> new hook in SELinux; and patch 7/7 modifies kernfs to call the new hook
> >> on new node creation.
> >>
> >> Testing:
> >> - passed the reproducer from the commit message of the last patch
> >> - passed SELinux testsuite on Fedora Rawhide (x86_64) when applied on top
> >> of current Rawhide kernel (5.0.0-0.rc7.git2.1) [3]
> >> - including the new proposed selinux-testsuite subtest [4] (adapted
> >> from the reproducer)
> >>
> >> [1] https://github.com/SELinuxProject/selinux-kernel/issues/39
> >> [2] https://bugzilla.redhat.com/show_bug.cgi?id=1553803
> >> [3] https://koji.fedoraproject.org/koji/taskinfo?taskID=32963825
> >> [4] https://github.com/SELinuxProject/selinux-testsuite/pull/48
> >>
> >> Ondrej Mosnacek (7):
> >> kernfs: clean up struct kernfs_iattrs
> >> kernfs: do not alloc iattrs in kernfs_xattr_get
> >> selinux: try security xattr after genfs for kernfs filesystems
> >> kernfs: use simple_xattrs for security attributes
> >> LSM: add new hook for kernfs node initialization
> >> selinux: implement the kernfs_init_security hook
> >> kernfs: initialize security of newly created nodes
> >>
> >> fs/kernfs/dir.c | 28 ++--
> >> fs/kernfs/inode.c | 166 +++++++++------------
> >> fs/kernfs/kernfs-internal.h | 8 +-
> >> fs/kernfs/symlink.c | 4 +-
> >> include/linux/kernfs.h | 15 ++
> >> include/linux/lsm_hooks.h | 13 ++
> >> include/linux/security.h | 9 ++
> >> security/security.c | 6 +
> >> security/selinux/hooks.c | 223 +++++++++++++++++++---------
> >> security/selinux/include/security.h | 1 +
> >> 10 files changed, 290 insertions(+), 183 deletions(-)
> >>
> >> --
> >> 2.20.1
> > Ping about this series... Casey, are you OK with this new version?
>
> I'm still not wildly enthusiastic about it, but I can't
> offer a better solution right now. You can add my
>
> Acked-by: Casey Schaufler <casey@schaufler-ca.com>
Thanks!
--
Ondrej Mosnacek <omosnace at redhat dot com>
Associate Software Engineer, Security Technologies
Red Hat, Inc.
^ permalink raw reply
* Re: [PATCH v7 0/7] Allow initializing the kernfs node's secctx based on its parent
From: Ondrej Mosnacek @ 2019-03-07 9:00 UTC (permalink / raw)
To: Paul Moore
Cc: selinux, Stephen Smalley, Linux Security Module list, Tejun Heo,
Casey Schaufler, Serge E . Hallyn, Greg Kroah-Hartman,
James Morris, linux-fsdevel, cgroups
In-Reply-To: <CAHC9VhShXZasD8d3nF4PwaL38gkq3hKDbvRivHX5_oHeBROFKQ@mail.gmail.com>
On Wed, Mar 6, 2019 at 5:20 PM Paul Moore <paul@paul-moore.com> wrote:
> On Wed, Mar 6, 2019 at 10:54 AM Ondrej Mosnacek <omosnace@redhat.com> wrote:
> > On Fri, Feb 22, 2019 at 3:57 PM Ondrej Mosnacek <omosnace@redhat.com> wrote:
> > > [...]
> >
> > Ping about this series... Casey, are you OK with this new version?
>
> We've got two weeks, well one and a half weeks, to decide as the
> earliest I would merge this would be after the merge window closes :)
Sure, I just wanted to know early if there would be any more changes needed.
--
Ondrej Mosnacek <omosnace at redhat dot com>
Associate Software Engineer, Security Technologies
Red Hat, Inc.
^ permalink raw reply
* Re: [PATCH v2 10/20] x86: avoid W^X being broken during modules loading
From: Borislav Petkov @ 2019-03-07 7:29 UTC (permalink / raw)
To: Nadav Amit
Cc: Rick Edgecombe, Andy Lutomirski, Ingo Molnar, LKML, X86 ML,
H. Peter Anvin, Thomas Gleixner, Dave Hansen, Peter Zijlstra,
Damian Tometzki, linux-integrity, LSM List, Andrew Morton,
Kernel Hardening, Linux-MM, Will Deacon, Ard Biesheuvel,
Kristen Carlson Accardi, Dock, Deneen T, Kees Cook, Dave Hansen,
Masami Hiramatsu
In-Reply-To: <20190211194251.GS19618@zn.tnic>
On Mon, Feb 11, 2019 at 08:42:51PM +0100, Borislav Petkov wrote:
> On Mon, Feb 11, 2019 at 11:27:03AM -0800, Nadav Amit wrote:
> > Is there any comment over static_cpu_has()? ;-)
>
> Almost:
>
> /*
> * Static testing of CPU features. Used the same as boot_cpu_has().
> * These will statically patch the target code for additional
> * performance.
> */
> static __always_inline __pure bool _static_cpu_has(u16 bit)
Ok, I guess something like that along with converting the obvious slow
path callers to boot_cpu_has():
---
diff --git a/arch/x86/include/asm/cpufeature.h b/arch/x86/include/asm/cpufeature.h
index ce95b8cbd229..e25d11ad7a88 100644
--- a/arch/x86/include/asm/cpufeature.h
+++ b/arch/x86/include/asm/cpufeature.h
@@ -155,9 +155,12 @@ extern void clear_cpu_cap(struct cpuinfo_x86 *c, unsigned int bit);
#else
/*
- * Static testing of CPU features. Used the same as boot_cpu_has().
- * These will statically patch the target code for additional
- * performance.
+ * Static testing of CPU features. Used the same as boot_cpu_has(). It
+ * statically patches the target code for additional performance. Use
+ * static_cpu_has() only in fast paths, where every cycle counts. Which
+ * means that the boot_cpu_has() variant is already fast enough for the
+ * majority of cases and you should stick to using it as it is generally
+ * only two instructions: a RIP-relative MOV and a TEST.
*/
static __always_inline __pure bool _static_cpu_has(u16 bit)
{
diff --git a/arch/x86/include/asm/fpu/internal.h b/arch/x86/include/asm/fpu/internal.h
index fa2c93cb42a2..c525b053b3b3 100644
--- a/arch/x86/include/asm/fpu/internal.h
+++ b/arch/x86/include/asm/fpu/internal.h
@@ -291,7 +291,7 @@ static inline void copy_xregs_to_kernel_booting(struct xregs_state *xstate)
WARN_ON(system_state != SYSTEM_BOOTING);
- if (static_cpu_has(X86_FEATURE_XSAVES))
+ if (boot_cpu_has(X86_FEATURE_XSAVES))
XSTATE_OP(XSAVES, xstate, lmask, hmask, err);
else
XSTATE_OP(XSAVE, xstate, lmask, hmask, err);
@@ -313,7 +313,7 @@ static inline void copy_kernel_to_xregs_booting(struct xregs_state *xstate)
WARN_ON(system_state != SYSTEM_BOOTING);
- if (static_cpu_has(X86_FEATURE_XSAVES))
+ if (boot_cpu_has(X86_FEATURE_XSAVES))
XSTATE_OP(XRSTORS, xstate, lmask, hmask, err);
else
XSTATE_OP(XRSTOR, xstate, lmask, hmask, err);
@@ -528,8 +528,7 @@ static inline void fpregs_activate(struct fpu *fpu)
* - switch_fpu_finish() restores the new state as
* necessary.
*/
-static inline void
-switch_fpu_prepare(struct fpu *old_fpu, int cpu)
+static inline void switch_fpu_prepare(struct fpu *old_fpu, int cpu)
{
if (static_cpu_has(X86_FEATURE_FPU) && old_fpu->initialized) {
if (!copy_fpregs_to_fpstate(old_fpu))
diff --git a/arch/x86/kernel/apic/apic_numachip.c b/arch/x86/kernel/apic/apic_numachip.c
index 78778b54f904..a5464b8b6c46 100644
--- a/arch/x86/kernel/apic/apic_numachip.c
+++ b/arch/x86/kernel/apic/apic_numachip.c
@@ -175,7 +175,7 @@ static void fixup_cpu_id(struct cpuinfo_x86 *c, int node)
this_cpu_write(cpu_llc_id, node);
/* Account for nodes per socket in multi-core-module processors */
- if (static_cpu_has(X86_FEATURE_NODEID_MSR)) {
+ if (boot_cpu_has(X86_FEATURE_NODEID_MSR)) {
rdmsrl(MSR_FAM10H_NODE_ID, val);
nodes = ((val >> 3) & 7) + 1;
}
diff --git a/arch/x86/kernel/cpu/aperfmperf.c b/arch/x86/kernel/cpu/aperfmperf.c
index 804c49493938..64d5aec24203 100644
--- a/arch/x86/kernel/cpu/aperfmperf.c
+++ b/arch/x86/kernel/cpu/aperfmperf.c
@@ -83,7 +83,7 @@ unsigned int aperfmperf_get_khz(int cpu)
if (!cpu_khz)
return 0;
- if (!static_cpu_has(X86_FEATURE_APERFMPERF))
+ if (!boot_cpu_has(X86_FEATURE_APERFMPERF))
return 0;
aperfmperf_snapshot_cpu(cpu, ktime_get(), true);
@@ -99,7 +99,7 @@ void arch_freq_prepare_all(void)
if (!cpu_khz)
return;
- if (!static_cpu_has(X86_FEATURE_APERFMPERF))
+ if (!boot_cpu_has(X86_FEATURE_APERFMPERF))
return;
for_each_online_cpu(cpu)
@@ -115,7 +115,7 @@ unsigned int arch_freq_get_on_cpu(int cpu)
if (!cpu_khz)
return 0;
- if (!static_cpu_has(X86_FEATURE_APERFMPERF))
+ if (!boot_cpu_has(X86_FEATURE_APERFMPERF))
return 0;
if (aperfmperf_snapshot_cpu(cpu, ktime_get(), true))
diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index cb28e98a0659..95a5faf3a6a0 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -1668,7 +1668,7 @@ static void setup_getcpu(int cpu)
unsigned long cpudata = vdso_encode_cpunode(cpu, early_cpu_to_node(cpu));
struct desc_struct d = { };
- if (static_cpu_has(X86_FEATURE_RDTSCP))
+ if (boot_cpu_has(X86_FEATURE_RDTSCP))
write_rdtscp_aux(cpudata);
/* Store CPU and node number in limit. */
diff --git a/arch/x86/kernel/cpu/mce/inject.c b/arch/x86/kernel/cpu/mce/inject.c
index 8492ef7d9015..3da9a8823e47 100644
--- a/arch/x86/kernel/cpu/mce/inject.c
+++ b/arch/x86/kernel/cpu/mce/inject.c
@@ -528,7 +528,7 @@ static void do_inject(void)
* only on the node base core. Refer to D18F3x44[NbMcaToMstCpuEn] for
* Fam10h and later BKDGs.
*/
- if (static_cpu_has(X86_FEATURE_AMD_DCM) &&
+ if (boot_cpu_has(X86_FEATURE_AMD_DCM) &&
b == 4 &&
boot_cpu_data.x86 < 0x17) {
toggle_nb_mca_mst_cpu(amd_get_nb_id(cpu));
diff --git a/arch/x86/kernel/cpu/proc.c b/arch/x86/kernel/cpu/proc.c
index 2c8522a39ed5..cb2e49810d68 100644
--- a/arch/x86/kernel/cpu/proc.c
+++ b/arch/x86/kernel/cpu/proc.c
@@ -35,11 +35,11 @@ static void show_cpuinfo_misc(struct seq_file *m, struct cpuinfo_x86 *c)
"fpu_exception\t: %s\n"
"cpuid level\t: %d\n"
"wp\t\t: yes\n",
- static_cpu_has_bug(X86_BUG_FDIV) ? "yes" : "no",
- static_cpu_has_bug(X86_BUG_F00F) ? "yes" : "no",
- static_cpu_has_bug(X86_BUG_COMA) ? "yes" : "no",
- static_cpu_has(X86_FEATURE_FPU) ? "yes" : "no",
- static_cpu_has(X86_FEATURE_FPU) ? "yes" : "no",
+ boot_cpu_has_bug(X86_BUG_FDIV) ? "yes" : "no",
+ boot_cpu_has_bug(X86_BUG_F00F) ? "yes" : "no",
+ boot_cpu_has_bug(X86_BUG_COMA) ? "yes" : "no",
+ boot_cpu_has(X86_FEATURE_FPU) ? "yes" : "no",
+ boot_cpu_has(X86_FEATURE_FPU) ? "yes" : "no",
c->cpuid_level);
}
#else
diff --git a/arch/x86/kernel/ldt.c b/arch/x86/kernel/ldt.c
index 6135ae8ce036..b2463fcb20a8 100644
--- a/arch/x86/kernel/ldt.c
+++ b/arch/x86/kernel/ldt.c
@@ -113,7 +113,7 @@ static void do_sanity_check(struct mm_struct *mm,
* tables.
*/
WARN_ON(!had_kernel_mapping);
- if (static_cpu_has(X86_FEATURE_PTI))
+ if (boot_cpu_has(X86_FEATURE_PTI))
WARN_ON(!had_user_mapping);
} else {
/*
@@ -121,7 +121,7 @@ static void do_sanity_check(struct mm_struct *mm,
* Sync the pgd to the usermode tables.
*/
WARN_ON(had_kernel_mapping);
- if (static_cpu_has(X86_FEATURE_PTI))
+ if (boot_cpu_has(X86_FEATURE_PTI))
WARN_ON(had_user_mapping);
}
}
@@ -156,7 +156,7 @@ static void map_ldt_struct_to_user(struct mm_struct *mm)
k_pmd = pgd_to_pmd_walk(k_pgd, LDT_BASE_ADDR);
u_pmd = pgd_to_pmd_walk(u_pgd, LDT_BASE_ADDR);
- if (static_cpu_has(X86_FEATURE_PTI) && !mm->context.ldt)
+ if (boot_cpu_has(X86_FEATURE_PTI) && !mm->context.ldt)
set_pmd(u_pmd, *k_pmd);
}
@@ -181,7 +181,7 @@ static void map_ldt_struct_to_user(struct mm_struct *mm)
{
pgd_t *pgd = pgd_offset(mm, LDT_BASE_ADDR);
- if (static_cpu_has(X86_FEATURE_PTI) && !mm->context.ldt)
+ if (boot_cpu_has(X86_FEATURE_PTI) && !mm->context.ldt)
set_pgd(kernel_to_user_pgdp(pgd), *pgd);
}
@@ -208,7 +208,7 @@ map_ldt_struct(struct mm_struct *mm, struct ldt_struct *ldt, int slot)
spinlock_t *ptl;
int i, nr_pages;
- if (!static_cpu_has(X86_FEATURE_PTI))
+ if (!boot_cpu_has(X86_FEATURE_PTI))
return 0;
/*
@@ -271,7 +271,7 @@ static void unmap_ldt_struct(struct mm_struct *mm, struct ldt_struct *ldt)
return;
/* LDT map/unmap is only required for PTI */
- if (!static_cpu_has(X86_FEATURE_PTI))
+ if (!boot_cpu_has(X86_FEATURE_PTI))
return;
nr_pages = DIV_ROUND_UP(ldt->nr_entries * LDT_ENTRY_SIZE, PAGE_SIZE);
@@ -311,7 +311,7 @@ static void free_ldt_pgtables(struct mm_struct *mm)
unsigned long start = LDT_BASE_ADDR;
unsigned long end = LDT_END_ADDR;
- if (!static_cpu_has(X86_FEATURE_PTI))
+ if (!boot_cpu_has(X86_FEATURE_PTI))
return;
tlb_gather_mmu(&tlb, mm, start, end);
diff --git a/arch/x86/kernel/paravirt.c b/arch/x86/kernel/paravirt.c
index c0e0101133f3..7bbaa6baf37f 100644
--- a/arch/x86/kernel/paravirt.c
+++ b/arch/x86/kernel/paravirt.c
@@ -121,7 +121,7 @@ DEFINE_STATIC_KEY_TRUE(virt_spin_lock_key);
void __init native_pv_lock_init(void)
{
- if (!static_cpu_has(X86_FEATURE_HYPERVISOR))
+ if (!boot_cpu_has(X86_FEATURE_HYPERVISOR))
static_branch_disable(&virt_spin_lock_key);
}
diff --git a/arch/x86/kernel/process.c b/arch/x86/kernel/process.c
index 58ac7be52c7a..16a7113e91c5 100644
--- a/arch/x86/kernel/process.c
+++ b/arch/x86/kernel/process.c
@@ -236,7 +236,7 @@ static int get_cpuid_mode(void)
static int set_cpuid_mode(struct task_struct *task, unsigned long cpuid_enabled)
{
- if (!static_cpu_has(X86_FEATURE_CPUID_FAULT))
+ if (!boot_cpu_has(X86_FEATURE_CPUID_FAULT))
return -ENODEV;
if (cpuid_enabled)
@@ -666,7 +666,7 @@ static int prefer_mwait_c1_over_halt(const struct cpuinfo_x86 *c)
if (c->x86_vendor != X86_VENDOR_INTEL)
return 0;
- if (!cpu_has(c, X86_FEATURE_MWAIT) || static_cpu_has_bug(X86_BUG_MONITOR))
+ if (!cpu_has(c, X86_FEATURE_MWAIT) || boot_cpu_has_bug(X86_BUG_MONITOR))
return 0;
return 1;
diff --git a/arch/x86/kernel/reboot.c b/arch/x86/kernel/reboot.c
index 725624b6c0c0..d62ebbc5ec78 100644
--- a/arch/x86/kernel/reboot.c
+++ b/arch/x86/kernel/reboot.c
@@ -108,7 +108,7 @@ void __noreturn machine_real_restart(unsigned int type)
write_cr3(real_mode_header->trampoline_pgd);
/* Exiting long mode will fail if CR4.PCIDE is set. */
- if (static_cpu_has(X86_FEATURE_PCID))
+ if (boot_cpu_has(X86_FEATURE_PCID))
cr4_clear_bits(X86_CR4_PCIDE);
#endif
diff --git a/arch/x86/kernel/vm86_32.c b/arch/x86/kernel/vm86_32.c
index a092b6b40c6b..6a38717d179c 100644
--- a/arch/x86/kernel/vm86_32.c
+++ b/arch/x86/kernel/vm86_32.c
@@ -369,7 +369,7 @@ static long do_sys_vm86(struct vm86plus_struct __user *user_vm86, bool plus)
preempt_disable();
tsk->thread.sp0 += 16;
- if (static_cpu_has(X86_FEATURE_SEP)) {
+ if (boot_cpu_has(X86_FEATURE_SEP)) {
tsk->thread.sysenter_cs = 0;
refresh_sysenter_cs(&tsk->thread);
}
diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index f13a3a24d360..5ed039bf1b58 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -835,7 +835,7 @@ static void svm_init_erratum_383(void)
int err;
u64 val;
- if (!static_cpu_has_bug(X86_BUG_AMD_TLB_MMATCH))
+ if (!boot_cpu_has_bug(X86_BUG_AMD_TLB_MMATCH))
return;
/* Use _safe variants to not break nested virtualization */
@@ -889,7 +889,7 @@ static int has_svm(void)
static void svm_hardware_disable(void)
{
/* Make sure we clean up behind us */
- if (static_cpu_has(X86_FEATURE_TSCRATEMSR))
+ if (boot_cpu_has(X86_FEATURE_TSCRATEMSR))
wrmsrl(MSR_AMD64_TSC_RATIO, TSC_RATIO_DEFAULT);
cpu_svm_disable();
@@ -931,7 +931,7 @@ static int svm_hardware_enable(void)
wrmsrl(MSR_VM_HSAVE_PA, page_to_pfn(sd->save_area) << PAGE_SHIFT);
- if (static_cpu_has(X86_FEATURE_TSCRATEMSR)) {
+ if (boot_cpu_has(X86_FEATURE_TSCRATEMSR)) {
wrmsrl(MSR_AMD64_TSC_RATIO, TSC_RATIO_DEFAULT);
__this_cpu_write(current_tsc_ratio, TSC_RATIO_DEFAULT);
}
@@ -2247,7 +2247,7 @@ static void svm_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
for (i = 0; i < NR_HOST_SAVE_USER_MSRS; i++)
rdmsrl(host_save_user_msrs[i], svm->host_user_msrs[i]);
- if (static_cpu_has(X86_FEATURE_TSCRATEMSR)) {
+ if (boot_cpu_has(X86_FEATURE_TSCRATEMSR)) {
u64 tsc_ratio = vcpu->arch.tsc_scaling_ratio;
if (tsc_ratio != __this_cpu_read(current_tsc_ratio)) {
__this_cpu_write(current_tsc_ratio, tsc_ratio);
@@ -2255,7 +2255,7 @@ static void svm_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
}
}
/* This assumes that the kernel never uses MSR_TSC_AUX */
- if (static_cpu_has(X86_FEATURE_RDTSCP))
+ if (boot_cpu_has(X86_FEATURE_RDTSCP))
wrmsrl(MSR_TSC_AUX, svm->tsc_aux);
if (sd->current_vmcb != svm->vmcb) {
diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index 30a6bcd735ec..0ec24853a0e6 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -6553,7 +6553,7 @@ static void vmx_vcpu_run(struct kvm_vcpu *vcpu)
if (vcpu->guest_debug & KVM_GUESTDBG_SINGLESTEP)
vmx_set_interrupt_shadow(vcpu, 0);
- if (static_cpu_has(X86_FEATURE_PKU) &&
+ if (boot_cpu_has(X86_FEATURE_PKU) &&
kvm_read_cr4_bits(vcpu, X86_CR4_PKE) &&
vcpu->arch.pkru != vmx->host_pkru)
__write_pkru(vcpu->arch.pkru);
@@ -6633,7 +6633,7 @@ static void vmx_vcpu_run(struct kvm_vcpu *vcpu)
* back on host, so it is safe to read guest PKRU from current
* XSAVE.
*/
- if (static_cpu_has(X86_FEATURE_PKU) &&
+ if (boot_cpu_has(X86_FEATURE_PKU) &&
kvm_read_cr4_bits(vcpu, X86_CR4_PKE)) {
vcpu->arch.pkru = __read_pkru();
if (vcpu->arch.pkru != vmx->host_pkru)
diff --git a/arch/x86/mm/dump_pagetables.c b/arch/x86/mm/dump_pagetables.c
index e3cdc85ce5b6..b596ac1eed1c 100644
--- a/arch/x86/mm/dump_pagetables.c
+++ b/arch/x86/mm/dump_pagetables.c
@@ -579,7 +579,7 @@ void ptdump_walk_pgd_level(struct seq_file *m, pgd_t *pgd)
void ptdump_walk_pgd_level_debugfs(struct seq_file *m, pgd_t *pgd, bool user)
{
#ifdef CONFIG_PAGE_TABLE_ISOLATION
- if (user && static_cpu_has(X86_FEATURE_PTI))
+ if (user && boot_cpu_has(X86_FEATURE_PTI))
pgd = kernel_to_user_pgdp(pgd);
#endif
ptdump_walk_pgd_level_core(m, pgd, false, false);
@@ -592,7 +592,7 @@ void ptdump_walk_user_pgd_level_checkwx(void)
pgd_t *pgd = INIT_PGD;
if (!(__supported_pte_mask & _PAGE_NX) ||
- !static_cpu_has(X86_FEATURE_PTI))
+ !boot_cpu_has(X86_FEATURE_PTI))
return;
pr_info("x86/mm: Checking user space page tables\n");
diff --git a/arch/x86/mm/pgtable.c b/arch/x86/mm/pgtable.c
index 7bd01709a091..3dbf440d4114 100644
--- a/arch/x86/mm/pgtable.c
+++ b/arch/x86/mm/pgtable.c
@@ -190,7 +190,7 @@ static void pgd_dtor(pgd_t *pgd)
* when PTI is enabled. We need them to map the per-process LDT into the
* user-space page-table.
*/
-#define PREALLOCATED_USER_PMDS (static_cpu_has(X86_FEATURE_PTI) ? \
+#define PREALLOCATED_USER_PMDS (boot_cpu_has(X86_FEATURE_PTI) ? \
KERNEL_PGD_PTRS : 0)
#define MAX_PREALLOCATED_USER_PMDS KERNEL_PGD_PTRS
@@ -292,7 +292,7 @@ static void pgd_mop_up_pmds(struct mm_struct *mm, pgd_t *pgdp)
#ifdef CONFIG_PAGE_TABLE_ISOLATION
- if (!static_cpu_has(X86_FEATURE_PTI))
+ if (!boot_cpu_has(X86_FEATURE_PTI))
return;
pgdp = kernel_to_user_pgdp(pgdp);
diff --git a/arch/x86/mm/pti.c b/arch/x86/mm/pti.c
index 4fee5c3003ed..8c9a54ebda60 100644
--- a/arch/x86/mm/pti.c
+++ b/arch/x86/mm/pti.c
@@ -626,7 +626,7 @@ void pti_set_kernel_image_nonglobal(void)
*/
void __init pti_init(void)
{
- if (!static_cpu_has(X86_FEATURE_PTI))
+ if (!boot_cpu_has(X86_FEATURE_PTI))
return;
pr_info("enabled\n");
diff --git a/drivers/cpufreq/amd_freq_sensitivity.c b/drivers/cpufreq/amd_freq_sensitivity.c
index 4ac7c3cf34be..6927a8c0e748 100644
--- a/drivers/cpufreq/amd_freq_sensitivity.c
+++ b/drivers/cpufreq/amd_freq_sensitivity.c
@@ -124,7 +124,7 @@ static int __init amd_freq_sensitivity_init(void)
PCI_DEVICE_ID_AMD_KERNCZ_SMBUS, NULL);
if (!pcidev) {
- if (!static_cpu_has(X86_FEATURE_PROC_FEEDBACK))
+ if (!boot_cpu_has(X86_FEATURE_PROC_FEEDBACK))
return -ENODEV;
}
diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
index dd66decf2087..9bbc3dfdebe3 100644
--- a/drivers/cpufreq/intel_pstate.c
+++ b/drivers/cpufreq/intel_pstate.c
@@ -520,7 +520,7 @@ static s16 intel_pstate_get_epb(struct cpudata *cpu_data)
u64 epb;
int ret;
- if (!static_cpu_has(X86_FEATURE_EPB))
+ if (!boot_cpu_has(X86_FEATURE_EPB))
return -ENXIO;
ret = rdmsrl_on_cpu(cpu_data->cpu, MSR_IA32_ENERGY_PERF_BIAS, &epb);
@@ -534,7 +534,7 @@ static s16 intel_pstate_get_epp(struct cpudata *cpu_data, u64 hwp_req_data)
{
s16 epp;
- if (static_cpu_has(X86_FEATURE_HWP_EPP)) {
+ if (boot_cpu_has(X86_FEATURE_HWP_EPP)) {
/*
* When hwp_req_data is 0, means that caller didn't read
* MSR_HWP_REQUEST, so need to read and get EPP.
@@ -559,7 +559,7 @@ static int intel_pstate_set_epb(int cpu, s16 pref)
u64 epb;
int ret;
- if (!static_cpu_has(X86_FEATURE_EPB))
+ if (!boot_cpu_has(X86_FEATURE_EPB))
return -ENXIO;
ret = rdmsrl_on_cpu(cpu, MSR_IA32_ENERGY_PERF_BIAS, &epb);
@@ -607,7 +607,7 @@ static int intel_pstate_get_energy_pref_index(struct cpudata *cpu_data)
if (epp < 0)
return epp;
- if (static_cpu_has(X86_FEATURE_HWP_EPP)) {
+ if (boot_cpu_has(X86_FEATURE_HWP_EPP)) {
if (epp == HWP_EPP_PERFORMANCE)
return 1;
if (epp <= HWP_EPP_BALANCE_PERFORMANCE)
@@ -616,7 +616,7 @@ static int intel_pstate_get_energy_pref_index(struct cpudata *cpu_data)
return 3;
else
return 4;
- } else if (static_cpu_has(X86_FEATURE_EPB)) {
+ } else if (boot_cpu_has(X86_FEATURE_EPB)) {
/*
* Range:
* 0x00-0x03 : Performance
@@ -644,7 +644,7 @@ static int intel_pstate_set_energy_pref_index(struct cpudata *cpu_data,
mutex_lock(&intel_pstate_limits_lock);
- if (static_cpu_has(X86_FEATURE_HWP_EPP)) {
+ if (boot_cpu_has(X86_FEATURE_HWP_EPP)) {
u64 value;
ret = rdmsrl_on_cpu(cpu_data->cpu, MSR_HWP_REQUEST, &value);
@@ -819,7 +819,7 @@ static void intel_pstate_hwp_set(unsigned int cpu)
epp = cpu_data->epp_powersave;
}
update_epp:
- if (static_cpu_has(X86_FEATURE_HWP_EPP)) {
+ if (boot_cpu_has(X86_FEATURE_HWP_EPP)) {
value &= ~GENMASK_ULL(31, 24);
value |= (u64)epp << 24;
} else {
@@ -844,7 +844,7 @@ static void intel_pstate_hwp_force_min_perf(int cpu)
value |= HWP_MIN_PERF(min_perf);
/* Set EPP/EPB to min */
- if (static_cpu_has(X86_FEATURE_HWP_EPP))
+ if (boot_cpu_has(X86_FEATURE_HWP_EPP))
value |= HWP_ENERGY_PERF_PREFERENCE(HWP_EPP_POWERSAVE);
else
intel_pstate_set_epb(cpu, HWP_EPP_BALANCE_POWERSAVE);
@@ -1191,7 +1191,7 @@ static void __init intel_pstate_sysfs_expose_params(void)
static void intel_pstate_hwp_enable(struct cpudata *cpudata)
{
/* First disable HWP notification interrupt as we don't process them */
- if (static_cpu_has(X86_FEATURE_HWP_NOTIFY))
+ if (boot_cpu_has(X86_FEATURE_HWP_NOTIFY))
wrmsrl_on_cpu(cpudata->cpu, MSR_HWP_INTERRUPT, 0x00);
wrmsrl_on_cpu(cpudata->cpu, MSR_PM_ENABLE, 0x1);
diff --git a/drivers/cpufreq/powernow-k8.c b/drivers/cpufreq/powernow-k8.c
index fb77b39a4ce3..3c12e03fa343 100644
--- a/drivers/cpufreq/powernow-k8.c
+++ b/drivers/cpufreq/powernow-k8.c
@@ -1178,7 +1178,7 @@ static int powernowk8_init(void)
unsigned int i, supported_cpus = 0;
int ret;
- if (static_cpu_has(X86_FEATURE_HW_PSTATE)) {
+ if (boot_cpu_has(X86_FEATURE_HW_PSTATE)) {
__request_acpi_cpufreq();
return -ENODEV;
}
--
Regards/Gruss,
Boris.
Good mailing practices for 400: avoid top-posting and trim the reply.
^ permalink raw reply related
* Re: [PULL REQUEST] Kernel lockdown patches for 5.2
From: Matthew Garrett @ 2019-03-07 4:24 UTC (permalink / raw)
To: Mimi Zohar; +Cc: jmorris, LSM List, Linux Kernel Mailing List, David Howells
In-Reply-To: <1551930990.31706.279.camel@linux.ibm.com>
On Wed, Mar 6, 2019 at 7:56 PM Mimi Zohar <zohar@linux.ibm.com> wrote:
> The kexec and kernel modules patches in this patch set continues to
> ignore IMA. This patch set should up front either provide an
> alternative solution to coordinate the different signature
> verification methods or rely on the architecture specific policy for
> that coordination.
Hi Mimi,
I'm working on a patch for this at the moment which can then be added
to either patchset. Is there a tree that contains the proposed Power
architecture policy? I want to make sure I don't accidentally end up
depending on anything x86.
^ permalink raw reply
* Re: [PULL REQUEST] Kernel lockdown patches for 5.2
From: Mimi Zohar @ 2019-03-07 3:56 UTC (permalink / raw)
To: Matthew Garrett, jmorris; +Cc: linux-security-module, linux-kernel, dhowells
In-Reply-To: <20190306235913.6631-1-matthewgarrett@google.com>
On Wed, 2019-03-06 at 15:58 -0800, Matthew Garrett wrote:
> 3) The integration with IMA has been dropped for now. IMA is in the
> process of adding support for architecture-specific policies that will
> interact correctly with the lockdown feature, and a followup patch will
> integrate that so we don't end up with an ordering dependency on the
> merge
The architecture specific policy is an attempt to coordinate between
the different signature verification methods (eg. PE and IMA kexec
kernel image signatures, appended and IMA kernel module signatures).
The coordination between these signature verification methods is
independent of the "lockdown" feature.
To prevent requiring multiple signature verifications, an IMA policy
rule(s) is defined only if either KEXEC_VERIFY_SIG or MODULE_SIG is
not enabled.
The kexec and kernel modules patches in this patch set continues to
ignore IMA. This patch set should up front either provide an
alternative solution to coordinate the different signature
verification methods or rely on the architecture specific policy for
that coordination.
Mimi
^ permalink raw reply
* Re: [PATCH 02/27] Add a SysRq option to lift kernel lockdown
From: Matthew Garrett @ 2019-03-07 0:12 UTC (permalink / raw)
To: Randy Dunlap; +Cc: jmorris, LSM List, Linux Kernel Mailing List, David Howells
In-Reply-To: <7bcd6149-332d-15f8-31e1-ef0a6b7f496e@infradead.org>
On Wed, Mar 6, 2019 at 4:10 PM Randy Dunlap <rdunlap@infradead.org> wrote:
>
> On 3/6/19 3:58 PM, Matthew Garrett wrote:
> > From: Kyle McMartin <kyle@redhat.com>
> >
> > Make an option to provide a sysrq key that will lift the kernel lockdown,
> > thereby allowing the running kernel image to be accessed and modified.
>
> You still need to document this in Documentation/admin-guide/sysrq.rst,
> like I mentioned last week.
Hm.. On reflection this patch doesn't make much sense without the
automatic lockdown enable functionality, so I'll just drop it from the
patchset instead. Thanks!
^ permalink raw reply
* Re: [PATCH 02/27] Add a SysRq option to lift kernel lockdown
From: Randy Dunlap @ 2019-03-07 0:09 UTC (permalink / raw)
To: Matthew Garrett, jmorris; +Cc: linux-security-module, linux-kernel, dhowells
In-Reply-To: <20190306235913.6631-3-matthewgarrett@google.com>
On 3/6/19 3:58 PM, Matthew Garrett wrote:
> From: Kyle McMartin <kyle@redhat.com>
>
> Make an option to provide a sysrq key that will lift the kernel lockdown,
> thereby allowing the running kernel image to be accessed and modified.
You still need to document this in Documentation/admin-guide/sysrq.rst,
like I mentioned last week.
> On x86 this is triggered with SysRq+x, but this key may not be available on
> all arches, so it is set by setting LOCKDOWN_LIFT_KEY in asm/setup.h.
> Since this macro must be defined in an arch to be able to use this facility
> for that arch, the Kconfig option is restricted to arches that support it.
>
> Signed-off-by: Kyle McMartin <kyle@redhat.com>
> Signed-off-by: David Howells <dhowells@redhat.com>
> cc: x86@kernel.org
> Signed-off-by: Matthew Garrett <matthewgarrett@google.com>
> ---
> arch/x86/include/asm/setup.h | 2 ++
> drivers/input/misc/uinput.c | 1 +
> drivers/tty/sysrq.c | 19 ++++++++++-----
> include/linux/input.h | 5 ++++
> include/linux/sysrq.h | 8 +++++-
> kernel/debug/kdb/kdb_main.c | 2 +-
> security/Kconfig | 9 +++++++
> security/lock_down.c | 47 ++++++++++++++++++++++++++++++++++++
> 8 files changed, 85 insertions(+), 8 deletions(-)
>
> diff --git a/arch/x86/include/asm/setup.h b/arch/x86/include/asm/setup.h
> index ed8ec011a9fd..8daf633a5347 100644
> --- a/arch/x86/include/asm/setup.h
> +++ b/arch/x86/include/asm/setup.h
> @@ -9,6 +9,8 @@
> #include <linux/linkage.h>
> #include <asm/page_types.h>
>
> +#define LOCKDOWN_LIFT_KEY 'x'
> +
> #ifdef __i386__
>
> #include <linux/pfn.h>
> diff --git a/drivers/input/misc/uinput.c b/drivers/input/misc/uinput.c
> index 8ec483e8688b..c2a77dc73fa0 100644
> --- a/drivers/input/misc/uinput.c
> +++ b/drivers/input/misc/uinput.c
> @@ -365,6 +365,7 @@ static int uinput_create_device(struct uinput_device *udev)
> dev->flush = uinput_dev_flush;
> }
>
> + dev->flags |= INPUTDEV_FLAGS_SYNTHETIC;
> dev->event = uinput_dev_event;
>
> input_set_drvdata(udev->dev, udev);
> diff --git a/drivers/tty/sysrq.c b/drivers/tty/sysrq.c
> index 1f03078ec352..0a05d336008e 100644
> --- a/drivers/tty/sysrq.c
> +++ b/drivers/tty/sysrq.c
> @@ -480,6 +480,7 @@ static struct sysrq_key_op *sysrq_key_table[36] = {
> /* x: May be registered on mips for TLB dump */
> /* x: May be registered on ppc/powerpc for xmon */
> /* x: May be registered on sparc64 for global PMU dump */
> + /* x: May be registered on x86_64 for disabling secure boot */
> NULL, /* x */
> /* y: May be registered on sparc64 for global register dump */
> NULL, /* y */
> @@ -523,7 +524,7 @@ static void __sysrq_put_key_op(int key, struct sysrq_key_op *op_p)
> sysrq_key_table[i] = op_p;
> }
>
> -void __handle_sysrq(int key, bool check_mask)
> +void __handle_sysrq(int key, unsigned int from)
> {
> struct sysrq_key_op *op_p;
> int orig_log_level;
> @@ -543,11 +544,15 @@ void __handle_sysrq(int key, bool check_mask)
>
> op_p = __sysrq_get_key_op(key);
> if (op_p) {
> + /* Ban synthetic events from some sysrq functionality */
> + if ((from == SYSRQ_FROM_PROC || from == SYSRQ_FROM_SYNTHETIC) &&
> + op_p->enable_mask & SYSRQ_DISABLE_USERSPACE)
> + printk("This sysrq operation is disabled from userspace.\n");
> /*
> * Should we check for enabled operations (/proc/sysrq-trigger
> * should not) and is the invoked operation enabled?
> */
> - if (!check_mask || sysrq_on_mask(op_p->enable_mask)) {
> + if (from == SYSRQ_FROM_KERNEL || sysrq_on_mask(op_p->enable_mask)) {
> pr_cont("%s\n", op_p->action_msg);
> console_loglevel = orig_log_level;
> op_p->handler(key);
> @@ -579,7 +584,7 @@ void __handle_sysrq(int key, bool check_mask)
> void handle_sysrq(int key)
> {
> if (sysrq_on())
> - __handle_sysrq(key, true);
> + __handle_sysrq(key, SYSRQ_FROM_KERNEL);
> }
> EXPORT_SYMBOL(handle_sysrq);
>
> @@ -659,7 +664,7 @@ static void sysrq_do_reset(struct timer_list *t)
> static void sysrq_handle_reset_request(struct sysrq_state *state)
> {
> if (state->reset_requested)
> - __handle_sysrq(sysrq_xlate[KEY_B], false);
> + __handle_sysrq(sysrq_xlate[KEY_B], SYSRQ_FROM_KERNEL);
>
> if (sysrq_reset_downtime_ms)
> mod_timer(&state->keyreset_timer,
> @@ -812,8 +817,10 @@ static bool sysrq_handle_keypress(struct sysrq_state *sysrq,
>
> default:
> if (sysrq->active && value && value != 2) {
> + int from = sysrq->handle.dev->flags & INPUTDEV_FLAGS_SYNTHETIC ?
> + SYSRQ_FROM_SYNTHETIC : 0;
> sysrq->need_reinject = false;
> - __handle_sysrq(sysrq_xlate[code], true);
> + __handle_sysrq(sysrq_xlate[code], from);
> }
> break;
> }
> @@ -1096,7 +1103,7 @@ static ssize_t write_sysrq_trigger(struct file *file, const char __user *buf,
>
> if (get_user(c, buf))
> return -EFAULT;
> - __handle_sysrq(c, false);
> + __handle_sysrq(c, SYSRQ_FROM_PROC);
> }
>
> return count;
> diff --git a/include/linux/input.h b/include/linux/input.h
> index 7c7516eb7d76..38cd0ea72c37 100644
> --- a/include/linux/input.h
> +++ b/include/linux/input.h
> @@ -42,6 +42,7 @@ struct input_value {
> * @phys: physical path to the device in the system hierarchy
> * @uniq: unique identification code for the device (if device has it)
> * @id: id of the device (struct input_id)
> + * @flags: input device flags (SYNTHETIC, etc.)
> * @propbit: bitmap of device properties and quirks
> * @evbit: bitmap of types of events supported by the device (EV_KEY,
> * EV_REL, etc.)
> @@ -124,6 +125,8 @@ struct input_dev {
> const char *uniq;
> struct input_id id;
>
> + unsigned int flags;
> +
> unsigned long propbit[BITS_TO_LONGS(INPUT_PROP_CNT)];
>
> unsigned long evbit[BITS_TO_LONGS(EV_CNT)];
> @@ -190,6 +193,8 @@ struct input_dev {
> };
> #define to_input_dev(d) container_of(d, struct input_dev, dev)
>
> +#define INPUTDEV_FLAGS_SYNTHETIC 0x000000001
> +
> /*
> * Verify that we are in sync with input_device_id mod_devicetable.h #defines
> */
> diff --git a/include/linux/sysrq.h b/include/linux/sysrq.h
> index 8c71874e8485..7de1f08b60a9 100644
> --- a/include/linux/sysrq.h
> +++ b/include/linux/sysrq.h
> @@ -29,6 +29,8 @@
> #define SYSRQ_ENABLE_BOOT 0x0080
> #define SYSRQ_ENABLE_RTNICE 0x0100
>
> +#define SYSRQ_DISABLE_USERSPACE 0x00010000
> +
> struct sysrq_key_op {
> void (*handler)(int);
> char *help_msg;
> @@ -43,8 +45,12 @@ struct sysrq_key_op {
> * are available -- else NULL's).
> */
>
> +#define SYSRQ_FROM_KERNEL 0x0001
> +#define SYSRQ_FROM_PROC 0x0002
> +#define SYSRQ_FROM_SYNTHETIC 0x0004
> +
> void handle_sysrq(int key);
> -void __handle_sysrq(int key, bool check_mask);
> +void __handle_sysrq(int key, unsigned int from);
> int register_sysrq_key(int key, struct sysrq_key_op *op);
> int unregister_sysrq_key(int key, struct sysrq_key_op *op);
> struct sysrq_key_op *__sysrq_get_key_op(int key);
> diff --git a/kernel/debug/kdb/kdb_main.c b/kernel/debug/kdb/kdb_main.c
> index 82a3b32a7cfc..efee1abf5e8e 100644
> --- a/kernel/debug/kdb/kdb_main.c
> +++ b/kernel/debug/kdb/kdb_main.c
> @@ -1981,7 +1981,7 @@ static int kdb_sr(int argc, const char **argv)
> return KDB_ARGCOUNT;
>
> kdb_trap_printk++;
> - __handle_sysrq(*argv[1], check_mask);
> + __handle_sysrq(*argv[1], check_mask ? SYSRQ_FROM_KERNEL : 0);
> kdb_trap_printk--;
>
> return 0;
> diff --git a/security/Kconfig b/security/Kconfig
> index 47dc3403b5af..8346eb883336 100644
> --- a/security/Kconfig
> +++ b/security/Kconfig
> @@ -244,6 +244,15 @@ config LOCK_DOWN_KERNEL_FORCE
> help
> Enable the kernel lock down functionality automatically at boot.
>
> +config ALLOW_LOCKDOWN_LIFT_BY_SYSRQ
> + bool "Allow the kernel lockdown to be lifted by SysRq"
> + depends on LOCK_DOWN_KERNEL
> + depends on MAGIC_SYSRQ
> + depends on X86
> + help
> + Allow the lockdown on a kernel to be lifted, by pressing a SysRq key
> + combination on a wired keyboard.
> +
> source "security/selinux/Kconfig"
> source "security/smack/Kconfig"
> source "security/tomoyo/Kconfig"
> diff --git a/security/lock_down.c b/security/lock_down.c
> index 13a8228c1034..cfbc2c39712b 100644
> --- a/security/lock_down.c
> +++ b/security/lock_down.c
> @@ -11,8 +11,14 @@
>
> #include <linux/security.h>
> #include <linux/export.h>
> +#include <linux/sysrq.h>
> +#include <asm/setup.h>
>
> +#ifdef CONFIG_ALLOW_LOCKDOWN_LIFT_BY_SYSRQ
> +static __read_mostly bool kernel_locked_down;
> +#else
> static __ro_after_init bool kernel_locked_down;
> +#endif
>
> /*
> * Put the kernel into lock-down mode.
> @@ -57,3 +63,44 @@ bool __kernel_is_locked_down(const char *what, bool first)
> return kernel_locked_down;
> }
> EXPORT_SYMBOL(__kernel_is_locked_down);
> +
> +#ifdef CONFIG_ALLOW_LOCKDOWN_LIFT_BY_SYSRQ
> +
> +/*
> + * Take the kernel out of lockdown mode.
> + */
> +static void lift_kernel_lockdown(void)
> +{
> + pr_notice("Lifting lockdown\n");
> + kernel_locked_down = false;
> +}
> +
> +/*
> + * Allow lockdown to be lifted by pressing something like SysRq+x (and not by
> + * echoing the appropriate letter into the sysrq-trigger file).
> + */
> +static void sysrq_handle_lockdown_lift(int key)
> +{
> + if (kernel_locked_down)
> + lift_kernel_lockdown();
> +}
> +
> +static struct sysrq_key_op lockdown_lift_sysrq_op = {
> + .handler = sysrq_handle_lockdown_lift,
> + .help_msg = "unSB(x)",
> + .action_msg = "Disabling Secure Boot restrictions",
> + .enable_mask = SYSRQ_DISABLE_USERSPACE,
> +};
> +
> +static int __init lockdown_lift_sysrq(void)
> +{
> + if (kernel_locked_down) {
> + lockdown_lift_sysrq_op.help_msg[5] = LOCKDOWN_LIFT_KEY;
> + register_sysrq_key(LOCKDOWN_LIFT_KEY, &lockdown_lift_sysrq_op);
> + }
> + return 0;
> +}
> +
> +late_initcall(lockdown_lift_sysrq);
> +
> +#endif /* CONFIG_ALLOW_LOCKDOWN_LIFT_BY_SYSRQ */
>
--
~Randy
^ permalink raw reply
* [PATCH 01/27] Add the ability to lock down access to the running kernel image
From: Matthew Garrett @ 2019-03-06 23:58 UTC (permalink / raw)
To: jmorris; +Cc: linux-security-module, linux-kernel, dhowells
In-Reply-To: <20190306235913.6631-1-matthewgarrett@google.com>
From: David Howells <dhowells@redhat.com>
Provide a single call to allow kernel code to determine whether the system
should be locked down, thereby disallowing various accesses that might
allow the running kernel image to be changed including the loading of
modules that aren't validly signed with a key we recognise, fiddling with
MSR registers and disallowing hibernation.
Signed-off-by: David Howells <dhowells@redhat.com>
Acked-by: James Morris <james.l.morris@oracle.com>
Signed-off-by: Matthew Garrett <matthewgarrett@google.com>
---
include/linux/kernel.h | 17 ++++++++++++
include/linux/security.h | 9 +++++-
security/Kconfig | 15 ++++++++++
security/Makefile | 3 ++
security/lock_down.c | 59 ++++++++++++++++++++++++++++++++++++++++
5 files changed, 102 insertions(+), 1 deletion(-)
create mode 100644 security/lock_down.c
diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index 8f0e68e250a7..833bf32ce4e6 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -340,6 +340,23 @@ static inline void refcount_error_report(struct pt_regs *regs, const char *err)
{ }
#endif
+#ifdef CONFIG_LOCK_DOWN_KERNEL
+extern bool __kernel_is_locked_down(const char *what, bool first);
+#else
+static inline bool __kernel_is_locked_down(const char *what, bool first)
+{
+ return false;
+}
+#endif
+
+#define kernel_is_locked_down(what) \
+ ({ \
+ static bool message_given; \
+ bool locked_down = __kernel_is_locked_down(what, !message_given); \
+ message_given = true; \
+ locked_down; \
+ })
+
/* Internal, do not use. */
int __must_check _kstrtoul(const char *s, unsigned int base, unsigned long *res);
int __must_check _kstrtol(const char *s, unsigned int base, long *res);
diff --git a/include/linux/security.h b/include/linux/security.h
index 13537a49ae97..b290946341a4 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -1798,5 +1798,12 @@ static inline void security_bpf_prog_free(struct bpf_prog_aux *aux)
#endif /* CONFIG_SECURITY */
#endif /* CONFIG_BPF_SYSCALL */
-#endif /* ! __LINUX_SECURITY_H */
+#ifdef CONFIG_LOCK_DOWN_KERNEL
+extern void __init init_lockdown(void);
+#else
+static inline void __init init_lockdown(void)
+{
+}
+#endif
+#endif /* ! __LINUX_SECURITY_H */
diff --git a/security/Kconfig b/security/Kconfig
index 1d6463fb1450..47dc3403b5af 100644
--- a/security/Kconfig
+++ b/security/Kconfig
@@ -229,6 +229,21 @@ config STATIC_USERMODEHELPER_PATH
If you wish for all usermode helper programs to be disabled,
specify an empty string here (i.e. "").
+config LOCK_DOWN_KERNEL
+ bool "Allow the kernel to be 'locked down'"
+ help
+ Allow the kernel to be locked down. If lockdown support is enabled
+ and activated, the kernel will impose additional restrictions
+ intended to prevent uid 0 from being able to modify the running
+ kernel. This may break userland applications that rely on low-level
+ access to hardware.
+
+config LOCK_DOWN_KERNEL_FORCE
+ bool "Enable kernel lockdown mode automatically"
+ depends on LOCK_DOWN_KERNEL
+ help
+ Enable the kernel lock down functionality automatically at boot.
+
source "security/selinux/Kconfig"
source "security/smack/Kconfig"
source "security/tomoyo/Kconfig"
diff --git a/security/Makefile b/security/Makefile
index c598b904938f..5ff090149c88 100644
--- a/security/Makefile
+++ b/security/Makefile
@@ -32,3 +32,6 @@ obj-$(CONFIG_CGROUP_DEVICE) += device_cgroup.o
# Object integrity file lists
subdir-$(CONFIG_INTEGRITY) += integrity
obj-$(CONFIG_INTEGRITY) += integrity/
+
+# Allow the kernel to be locked down
+obj-$(CONFIG_LOCK_DOWN_KERNEL) += lock_down.o
diff --git a/security/lock_down.c b/security/lock_down.c
new file mode 100644
index 000000000000..13a8228c1034
--- /dev/null
+++ b/security/lock_down.c
@@ -0,0 +1,59 @@
+/* Lock down the kernel
+ *
+ * Copyright (C) 2016 Red Hat, Inc. All Rights Reserved.
+ * Written by David Howells (dhowells@redhat.com)
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public Licence
+ * as published by the Free Software Foundation; either version
+ * 2 of the Licence, or (at your option) any later version.
+ */
+
+#include <linux/security.h>
+#include <linux/export.h>
+
+static __ro_after_init bool kernel_locked_down;
+
+/*
+ * Put the kernel into lock-down mode.
+ */
+static void __init lock_kernel_down(const char *where)
+{
+ if (!kernel_locked_down) {
+ kernel_locked_down = true;
+ pr_notice("Kernel is locked down from %s; see man kernel_lockdown.7\n",
+ where);
+ }
+}
+
+static int __init lockdown_param(char *ignored)
+{
+ lock_kernel_down("command line");
+ return 0;
+}
+
+early_param("lockdown", lockdown_param);
+
+/*
+ * Lock the kernel down from very early in the arch setup. This must happen
+ * prior to things like ACPI being initialised.
+ */
+void __init init_lockdown(void)
+{
+#ifdef CONFIG_LOCK_DOWN_FORCE
+ lock_kernel_down("Kernel configuration");
+#endif
+}
+
+/**
+ * kernel_is_locked_down - Find out if the kernel is locked down
+ * @what: Tag to use in notice generated if lockdown is in effect
+ */
+bool __kernel_is_locked_down(const char *what, bool first)
+{
+ if (what && first && kernel_locked_down)
+ pr_notice("Lockdown: %s is restricted; see man kernel_lockdown.7\n",
+ what);
+ return kernel_locked_down;
+}
+EXPORT_SYMBOL(__kernel_is_locked_down);
--
2.21.0.352.gf09ad66450-goog
^ permalink raw reply related
* [PATCH 02/27] Add a SysRq option to lift kernel lockdown
From: Matthew Garrett @ 2019-03-06 23:58 UTC (permalink / raw)
To: jmorris; +Cc: linux-security-module, linux-kernel, dhowells
In-Reply-To: <20190306235913.6631-1-matthewgarrett@google.com>
From: Kyle McMartin <kyle@redhat.com>
Make an option to provide a sysrq key that will lift the kernel lockdown,
thereby allowing the running kernel image to be accessed and modified.
On x86 this is triggered with SysRq+x, but this key may not be available on
all arches, so it is set by setting LOCKDOWN_LIFT_KEY in asm/setup.h.
Since this macro must be defined in an arch to be able to use this facility
for that arch, the Kconfig option is restricted to arches that support it.
Signed-off-by: Kyle McMartin <kyle@redhat.com>
Signed-off-by: David Howells <dhowells@redhat.com>
cc: x86@kernel.org
Signed-off-by: Matthew Garrett <matthewgarrett@google.com>
---
arch/x86/include/asm/setup.h | 2 ++
drivers/input/misc/uinput.c | 1 +
drivers/tty/sysrq.c | 19 ++++++++++-----
include/linux/input.h | 5 ++++
include/linux/sysrq.h | 8 +++++-
kernel/debug/kdb/kdb_main.c | 2 +-
security/Kconfig | 9 +++++++
security/lock_down.c | 47 ++++++++++++++++++++++++++++++++++++
8 files changed, 85 insertions(+), 8 deletions(-)
diff --git a/arch/x86/include/asm/setup.h b/arch/x86/include/asm/setup.h
index ed8ec011a9fd..8daf633a5347 100644
--- a/arch/x86/include/asm/setup.h
+++ b/arch/x86/include/asm/setup.h
@@ -9,6 +9,8 @@
#include <linux/linkage.h>
#include <asm/page_types.h>
+#define LOCKDOWN_LIFT_KEY 'x'
+
#ifdef __i386__
#include <linux/pfn.h>
diff --git a/drivers/input/misc/uinput.c b/drivers/input/misc/uinput.c
index 8ec483e8688b..c2a77dc73fa0 100644
--- a/drivers/input/misc/uinput.c
+++ b/drivers/input/misc/uinput.c
@@ -365,6 +365,7 @@ static int uinput_create_device(struct uinput_device *udev)
dev->flush = uinput_dev_flush;
}
+ dev->flags |= INPUTDEV_FLAGS_SYNTHETIC;
dev->event = uinput_dev_event;
input_set_drvdata(udev->dev, udev);
diff --git a/drivers/tty/sysrq.c b/drivers/tty/sysrq.c
index 1f03078ec352..0a05d336008e 100644
--- a/drivers/tty/sysrq.c
+++ b/drivers/tty/sysrq.c
@@ -480,6 +480,7 @@ static struct sysrq_key_op *sysrq_key_table[36] = {
/* x: May be registered on mips for TLB dump */
/* x: May be registered on ppc/powerpc for xmon */
/* x: May be registered on sparc64 for global PMU dump */
+ /* x: May be registered on x86_64 for disabling secure boot */
NULL, /* x */
/* y: May be registered on sparc64 for global register dump */
NULL, /* y */
@@ -523,7 +524,7 @@ static void __sysrq_put_key_op(int key, struct sysrq_key_op *op_p)
sysrq_key_table[i] = op_p;
}
-void __handle_sysrq(int key, bool check_mask)
+void __handle_sysrq(int key, unsigned int from)
{
struct sysrq_key_op *op_p;
int orig_log_level;
@@ -543,11 +544,15 @@ void __handle_sysrq(int key, bool check_mask)
op_p = __sysrq_get_key_op(key);
if (op_p) {
+ /* Ban synthetic events from some sysrq functionality */
+ if ((from == SYSRQ_FROM_PROC || from == SYSRQ_FROM_SYNTHETIC) &&
+ op_p->enable_mask & SYSRQ_DISABLE_USERSPACE)
+ printk("This sysrq operation is disabled from userspace.\n");
/*
* Should we check for enabled operations (/proc/sysrq-trigger
* should not) and is the invoked operation enabled?
*/
- if (!check_mask || sysrq_on_mask(op_p->enable_mask)) {
+ if (from == SYSRQ_FROM_KERNEL || sysrq_on_mask(op_p->enable_mask)) {
pr_cont("%s\n", op_p->action_msg);
console_loglevel = orig_log_level;
op_p->handler(key);
@@ -579,7 +584,7 @@ void __handle_sysrq(int key, bool check_mask)
void handle_sysrq(int key)
{
if (sysrq_on())
- __handle_sysrq(key, true);
+ __handle_sysrq(key, SYSRQ_FROM_KERNEL);
}
EXPORT_SYMBOL(handle_sysrq);
@@ -659,7 +664,7 @@ static void sysrq_do_reset(struct timer_list *t)
static void sysrq_handle_reset_request(struct sysrq_state *state)
{
if (state->reset_requested)
- __handle_sysrq(sysrq_xlate[KEY_B], false);
+ __handle_sysrq(sysrq_xlate[KEY_B], SYSRQ_FROM_KERNEL);
if (sysrq_reset_downtime_ms)
mod_timer(&state->keyreset_timer,
@@ -812,8 +817,10 @@ static bool sysrq_handle_keypress(struct sysrq_state *sysrq,
default:
if (sysrq->active && value && value != 2) {
+ int from = sysrq->handle.dev->flags & INPUTDEV_FLAGS_SYNTHETIC ?
+ SYSRQ_FROM_SYNTHETIC : 0;
sysrq->need_reinject = false;
- __handle_sysrq(sysrq_xlate[code], true);
+ __handle_sysrq(sysrq_xlate[code], from);
}
break;
}
@@ -1096,7 +1103,7 @@ static ssize_t write_sysrq_trigger(struct file *file, const char __user *buf,
if (get_user(c, buf))
return -EFAULT;
- __handle_sysrq(c, false);
+ __handle_sysrq(c, SYSRQ_FROM_PROC);
}
return count;
diff --git a/include/linux/input.h b/include/linux/input.h
index 7c7516eb7d76..38cd0ea72c37 100644
--- a/include/linux/input.h
+++ b/include/linux/input.h
@@ -42,6 +42,7 @@ struct input_value {
* @phys: physical path to the device in the system hierarchy
* @uniq: unique identification code for the device (if device has it)
* @id: id of the device (struct input_id)
+ * @flags: input device flags (SYNTHETIC, etc.)
* @propbit: bitmap of device properties and quirks
* @evbit: bitmap of types of events supported by the device (EV_KEY,
* EV_REL, etc.)
@@ -124,6 +125,8 @@ struct input_dev {
const char *uniq;
struct input_id id;
+ unsigned int flags;
+
unsigned long propbit[BITS_TO_LONGS(INPUT_PROP_CNT)];
unsigned long evbit[BITS_TO_LONGS(EV_CNT)];
@@ -190,6 +193,8 @@ struct input_dev {
};
#define to_input_dev(d) container_of(d, struct input_dev, dev)
+#define INPUTDEV_FLAGS_SYNTHETIC 0x000000001
+
/*
* Verify that we are in sync with input_device_id mod_devicetable.h #defines
*/
diff --git a/include/linux/sysrq.h b/include/linux/sysrq.h
index 8c71874e8485..7de1f08b60a9 100644
--- a/include/linux/sysrq.h
+++ b/include/linux/sysrq.h
@@ -29,6 +29,8 @@
#define SYSRQ_ENABLE_BOOT 0x0080
#define SYSRQ_ENABLE_RTNICE 0x0100
+#define SYSRQ_DISABLE_USERSPACE 0x00010000
+
struct sysrq_key_op {
void (*handler)(int);
char *help_msg;
@@ -43,8 +45,12 @@ struct sysrq_key_op {
* are available -- else NULL's).
*/
+#define SYSRQ_FROM_KERNEL 0x0001
+#define SYSRQ_FROM_PROC 0x0002
+#define SYSRQ_FROM_SYNTHETIC 0x0004
+
void handle_sysrq(int key);
-void __handle_sysrq(int key, bool check_mask);
+void __handle_sysrq(int key, unsigned int from);
int register_sysrq_key(int key, struct sysrq_key_op *op);
int unregister_sysrq_key(int key, struct sysrq_key_op *op);
struct sysrq_key_op *__sysrq_get_key_op(int key);
diff --git a/kernel/debug/kdb/kdb_main.c b/kernel/debug/kdb/kdb_main.c
index 82a3b32a7cfc..efee1abf5e8e 100644
--- a/kernel/debug/kdb/kdb_main.c
+++ b/kernel/debug/kdb/kdb_main.c
@@ -1981,7 +1981,7 @@ static int kdb_sr(int argc, const char **argv)
return KDB_ARGCOUNT;
kdb_trap_printk++;
- __handle_sysrq(*argv[1], check_mask);
+ __handle_sysrq(*argv[1], check_mask ? SYSRQ_FROM_KERNEL : 0);
kdb_trap_printk--;
return 0;
diff --git a/security/Kconfig b/security/Kconfig
index 47dc3403b5af..8346eb883336 100644
--- a/security/Kconfig
+++ b/security/Kconfig
@@ -244,6 +244,15 @@ config LOCK_DOWN_KERNEL_FORCE
help
Enable the kernel lock down functionality automatically at boot.
+config ALLOW_LOCKDOWN_LIFT_BY_SYSRQ
+ bool "Allow the kernel lockdown to be lifted by SysRq"
+ depends on LOCK_DOWN_KERNEL
+ depends on MAGIC_SYSRQ
+ depends on X86
+ help
+ Allow the lockdown on a kernel to be lifted, by pressing a SysRq key
+ combination on a wired keyboard.
+
source "security/selinux/Kconfig"
source "security/smack/Kconfig"
source "security/tomoyo/Kconfig"
diff --git a/security/lock_down.c b/security/lock_down.c
index 13a8228c1034..cfbc2c39712b 100644
--- a/security/lock_down.c
+++ b/security/lock_down.c
@@ -11,8 +11,14 @@
#include <linux/security.h>
#include <linux/export.h>
+#include <linux/sysrq.h>
+#include <asm/setup.h>
+#ifdef CONFIG_ALLOW_LOCKDOWN_LIFT_BY_SYSRQ
+static __read_mostly bool kernel_locked_down;
+#else
static __ro_after_init bool kernel_locked_down;
+#endif
/*
* Put the kernel into lock-down mode.
@@ -57,3 +63,44 @@ bool __kernel_is_locked_down(const char *what, bool first)
return kernel_locked_down;
}
EXPORT_SYMBOL(__kernel_is_locked_down);
+
+#ifdef CONFIG_ALLOW_LOCKDOWN_LIFT_BY_SYSRQ
+
+/*
+ * Take the kernel out of lockdown mode.
+ */
+static void lift_kernel_lockdown(void)
+{
+ pr_notice("Lifting lockdown\n");
+ kernel_locked_down = false;
+}
+
+/*
+ * Allow lockdown to be lifted by pressing something like SysRq+x (and not by
+ * echoing the appropriate letter into the sysrq-trigger file).
+ */
+static void sysrq_handle_lockdown_lift(int key)
+{
+ if (kernel_locked_down)
+ lift_kernel_lockdown();
+}
+
+static struct sysrq_key_op lockdown_lift_sysrq_op = {
+ .handler = sysrq_handle_lockdown_lift,
+ .help_msg = "unSB(x)",
+ .action_msg = "Disabling Secure Boot restrictions",
+ .enable_mask = SYSRQ_DISABLE_USERSPACE,
+};
+
+static int __init lockdown_lift_sysrq(void)
+{
+ if (kernel_locked_down) {
+ lockdown_lift_sysrq_op.help_msg[5] = LOCKDOWN_LIFT_KEY;
+ register_sysrq_key(LOCKDOWN_LIFT_KEY, &lockdown_lift_sysrq_op);
+ }
+ return 0;
+}
+
+late_initcall(lockdown_lift_sysrq);
+
+#endif /* CONFIG_ALLOW_LOCKDOWN_LIFT_BY_SYSRQ */
--
2.21.0.352.gf09ad66450-goog
^ permalink raw reply related
* [PATCH 03/27] Enforce module signatures if the kernel is locked down
From: Matthew Garrett @ 2019-03-06 23:58 UTC (permalink / raw)
To: jmorris; +Cc: linux-security-module, linux-kernel, dhowells
In-Reply-To: <20190306235913.6631-1-matthewgarrett@google.com>
From: David Howells <dhowells@redhat.com>
If the kernel is locked down, require that all modules have valid
signatures that we can verify.
I have adjusted the errors generated:
(1) If there's no signature (ENODATA) or we can't check it (ENOPKG,
ENOKEY), then:
(a) If signatures are enforced then EKEYREJECTED is returned.
(b) If there's no signature or we can't check it, but the kernel is
locked down then EPERM is returned (this is then consistent with
other lockdown cases).
(2) If the signature is unparseable (EBADMSG, EINVAL), the signature fails
the check (EKEYREJECTED) or a system error occurs (eg. ENOMEM), we
return the error we got.
Note that the X.509 code doesn't check for key expiry as the RTC might not
be valid or might not have been transferred to the kernel's clock yet.
[Modified by Matthew Garrett to remove the IMA integration. This will
be replaced with integration with the IMA architecture policy
patchset.]
Signed-off-by: David Howells <dhowells@redhat.com>
Reviewed-by: Jiri Bohac <jbohac@suse.cz>
cc: "Lee, Chun-Yi" <jlee@suse.com>
cc: James Morris <james.l.morris@oracle.com>
Signed-off-by: Matthew Garrett <matthewgarrett@google.com>
---
kernel/module.c | 39 ++++++++++++++++++++++++++++++++-------
1 file changed, 32 insertions(+), 7 deletions(-)
diff --git a/kernel/module.c b/kernel/module.c
index 2ad1b5239910..9a377c6ea200 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -2767,8 +2767,9 @@ static inline void kmemleak_load_module(const struct module *mod,
#ifdef CONFIG_MODULE_SIG
static int module_sig_check(struct load_info *info, int flags)
{
- int err = -ENOKEY;
+ int err = -ENODATA;
const unsigned long markerlen = sizeof(MODULE_SIG_STRING) - 1;
+ const char *reason;
const void *mod = info->hdr;
/*
@@ -2783,16 +2784,40 @@ static int module_sig_check(struct load_info *info, int flags)
err = mod_verify_sig(mod, info);
}
- if (!err) {
+ switch (err) {
+ case 0:
info->sig_ok = true;
return 0;
- }
- /* Not having a signature is only an error if we're strict. */
- if (err == -ENOKEY && !is_module_sig_enforced())
- err = 0;
+ /* We don't permit modules to be loaded into trusted kernels
+ * without a valid signature on them, but if we're not
+ * enforcing, certain errors are non-fatal.
+ */
+ case -ENODATA:
+ reason = "Loading of unsigned module";
+ goto decide;
+ case -ENOPKG:
+ reason = "Loading of module with unsupported crypto";
+ goto decide;
+ case -ENOKEY:
+ reason = "Loading of module with unavailable key";
+ decide:
+ if (is_module_sig_enforced()) {
+ pr_notice("%s is rejected\n", reason);
+ return -EKEYREJECTED;
+ }
- return err;
+ if (kernel_is_locked_down(reason))
+ return -EPERM;
+ return 0;
+
+ /* All other errors are fatal, including nomem, unparseable
+ * signatures and signature check failures - even if signatures
+ * aren't required.
+ */
+ default:
+ return err;
+ }
}
#else /* !CONFIG_MODULE_SIG */
static int module_sig_check(struct load_info *info, int flags)
--
2.21.0.352.gf09ad66450-goog
^ permalink raw reply related
* [PATCH 04/27] Restrict /dev/{mem,kmem,port} when the kernel is locked down
From: Matthew Garrett @ 2019-03-06 23:58 UTC (permalink / raw)
To: jmorris; +Cc: linux-security-module, linux-kernel, dhowells
In-Reply-To: <20190306235913.6631-1-matthewgarrett@google.com>
From: Matthew Garrett <mjg59@srcf.ucam.org>
Allowing users to read and write to core kernel memory makes it possible
for the kernel to be subverted, avoiding module loading restrictions, and
also to steal cryptographic information.
Disallow /dev/mem and /dev/kmem from being opened this when the kernel has
been locked down to prevent this.
Also disallow /dev/port from being opened to prevent raw ioport access and
thus DMA from being used to accomplish the same thing.
Signed-off-by: Matthew Garrett <mjg59@srcf.ucam.org>
Signed-off-by: David Howells <dhowells@redhat.com>
Reviewed-by: "Lee, Chun-Yi" <jlee@suse.com>
Signed-off-by: Matthew Garrett <matthewgarrett@google.com>
---
drivers/char/mem.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/char/mem.c b/drivers/char/mem.c
index b08dc50f9f26..0a2f2e75d5f4 100644
--- a/drivers/char/mem.c
+++ b/drivers/char/mem.c
@@ -786,6 +786,8 @@ static loff_t memory_lseek(struct file *file, loff_t offset, int orig)
static int open_port(struct inode *inode, struct file *filp)
{
+ if (kernel_is_locked_down("/dev/mem,kmem,port"))
+ return -EPERM;
return capable(CAP_SYS_RAWIO) ? 0 : -EPERM;
}
--
2.21.0.352.gf09ad66450-goog
^ permalink raw reply related
* [PATCH 06/27] Copy secure_boot flag in boot params across kexec reboot
From: Matthew Garrett @ 2019-03-06 23:58 UTC (permalink / raw)
To: jmorris; +Cc: linux-security-module, linux-kernel, dhowells
In-Reply-To: <20190306235913.6631-1-matthewgarrett@google.com>
From: Dave Young <dyoung@redhat.com>
Kexec reboot in case secure boot being enabled does not keep the secure
boot mode in new kernel, so later one can load unsigned kernel via legacy
kexec_load. In this state, the system is missing the protections provided
by secure boot.
Adding a patch to fix this by retain the secure_boot flag in original
kernel.
secure_boot flag in boot_params is set in EFI stub, but kexec bypasses the
stub. Fixing this issue by copying secure_boot flag across kexec reboot.
Signed-off-by: Dave Young <dyoung@redhat.com>
Signed-off-by: David Howells <dhowells@redhat.com>
Reviewed-by: "Lee, Chun-Yi" <jlee@suse.com>
cc: kexec@lists.infradead.org
Signed-off-by: Matthew Garrett <matthewgarrett@google.com>
---
arch/x86/kernel/kexec-bzimage64.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/x86/kernel/kexec-bzimage64.c b/arch/x86/kernel/kexec-bzimage64.c
index 278cd07228dd..d49554b948fd 100644
--- a/arch/x86/kernel/kexec-bzimage64.c
+++ b/arch/x86/kernel/kexec-bzimage64.c
@@ -179,6 +179,7 @@ setup_efi_state(struct boot_params *params, unsigned long params_load_addr,
if (efi_enabled(EFI_OLD_MEMMAP))
return 0;
+ params->secure_boot = boot_params.secure_boot;
ei->efi_loader_signature = current_ei->efi_loader_signature;
ei->efi_systab = current_ei->efi_systab;
ei->efi_systab_hi = current_ei->efi_systab_hi;
--
2.21.0.352.gf09ad66450-goog
^ permalink raw reply related
* [PATCH 07/27] kexec_file: split KEXEC_VERIFY_SIG into KEXEC_SIG and KEXEC_SIG_FORCE
From: Matthew Garrett @ 2019-03-06 23:58 UTC (permalink / raw)
To: jmorris; +Cc: linux-security-module, linux-kernel, dhowells
In-Reply-To: <20190306235913.6631-1-matthewgarrett@google.com>
From: Jiri Bohac <jbohac@suse.cz>
This is a preparatory patch for kexec_file_load() lockdown. A locked down
kernel needs to prevent unsigned kernel images from being loaded with
kexec_file_load(). Currently, the only way to force the signature
verification is compiling with KEXEC_VERIFY_SIG. This prevents loading
usigned images even when the kernel is not locked down at runtime.
This patch splits KEXEC_VERIFY_SIG into KEXEC_SIG and KEXEC_SIG_FORCE.
Analogous to the MODULE_SIG and MODULE_SIG_FORCE for modules, KEXEC_SIG
turns on the signature verification but allows unsigned images to be
loaded. KEXEC_SIG_FORCE disallows images without a valid signature.
[Modified by David Howells such that:
(1) verify_pefile_signature() differentiates between no-signature and
sig-didn't-match in its returned errors.
(2) kexec fails with EKEYREJECTED and logs an appropriate message if
signature checking is enforced and an signature is not found, uses
unsupported crypto or has no matching key.
(3) kexec fails with EKEYREJECTED if there is a signature for which we
have a key, but signature doesn't match - even if in non-forcing mode.
(4) kexec fails with EBADMSG or some other error if there is a signature
which cannot be parsed - even if in non-forcing mode.
(5) kexec fails with ELIBBAD if the PE file cannot be parsed to extract
the signature - even if in non-forcing mode.
]
Signed-off-by: Jiri Bohac <jbohac@suse.cz>
Signed-off-by: David Howells <dhowells@redhat.com>
Reviewed-by: Jiri Bohac <jbohac@suse.cz>
cc: Matthew Garrett <mjg59@srcf.ucam.org>
cc: Chun-Yi Lee <jlee@suse.com>
cc: kexec@lists.infradead.org
Signed-off-by: Matthew Garrett <matthewgarrett@google.com>
---
arch/x86/Kconfig | 20 ++++++++---
crypto/asymmetric_keys/verify_pefile.c | 4 ++-
include/linux/kexec.h | 4 +--
kernel/kexec_file.c | 48 ++++++++++++++++++++++----
4 files changed, 61 insertions(+), 15 deletions(-)
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 4b4a7f32b68e..735d04a4b18f 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -2016,20 +2016,30 @@ config KEXEC_FILE
config ARCH_HAS_KEXEC_PURGATORY
def_bool KEXEC_FILE
-config KEXEC_VERIFY_SIG
+config KEXEC_SIG
bool "Verify kernel signature during kexec_file_load() syscall"
depends on KEXEC_FILE
---help---
- This option makes kernel signature verification mandatory for
- the kexec_file_load() syscall.
- In addition to that option, you need to enable signature
+ This option makes the kexec_file_load() syscall check for a valid
+ signature of the kernel image. The image can still be loaded without
+ a valid signature unless you also enable KEXEC_SIG_FORCE, though if
+ there's a signature that we can check, then it must be valid.
+
+ In addition to this option, you need to enable signature
verification for the corresponding kernel image type being
loaded in order for this to work.
+config KEXEC_SIG_FORCE
+ bool "Require a valid signature in kexec_file_load() syscall"
+ depends on KEXEC_SIG
+ ---help---
+ This option makes kernel signature verification mandatory for
+ the kexec_file_load() syscall.
+
config KEXEC_BZIMAGE_VERIFY_SIG
bool "Enable bzImage signature verification support"
- depends on KEXEC_VERIFY_SIG
+ depends on KEXEC_SIG
depends on SIGNED_PE_FILE_VERIFICATION
select SYSTEM_TRUSTED_KEYRING
---help---
diff --git a/crypto/asymmetric_keys/verify_pefile.c b/crypto/asymmetric_keys/verify_pefile.c
index d178650fd524..4473cea1e877 100644
--- a/crypto/asymmetric_keys/verify_pefile.c
+++ b/crypto/asymmetric_keys/verify_pefile.c
@@ -100,7 +100,7 @@ static int pefile_parse_binary(const void *pebuf, unsigned int pelen,
if (!ddir->certs.virtual_address || !ddir->certs.size) {
pr_debug("Unsigned PE binary\n");
- return -EKEYREJECTED;
+ return -ENODATA;
}
chkaddr(ctx->header_size, ddir->certs.virtual_address,
@@ -408,6 +408,8 @@ static int pefile_digest_pe(const void *pebuf, unsigned int pelen,
* (*) 0 if at least one signature chain intersects with the keys in the trust
* keyring, or:
*
+ * (*) -ENODATA if there is no signature present.
+ *
* (*) -ENOPKG if a suitable crypto module couldn't be found for a check on a
* chain.
*
diff --git a/include/linux/kexec.h b/include/linux/kexec.h
index b9b1bc5f9669..58b27c7bdc2b 100644
--- a/include/linux/kexec.h
+++ b/include/linux/kexec.h
@@ -125,7 +125,7 @@ typedef void *(kexec_load_t)(struct kimage *image, char *kernel_buf,
unsigned long cmdline_len);
typedef int (kexec_cleanup_t)(void *loader_data);
-#ifdef CONFIG_KEXEC_VERIFY_SIG
+#ifdef CONFIG_KEXEC_SIG
typedef int (kexec_verify_sig_t)(const char *kernel_buf,
unsigned long kernel_len);
#endif
@@ -134,7 +134,7 @@ struct kexec_file_ops {
kexec_probe_t *probe;
kexec_load_t *load;
kexec_cleanup_t *cleanup;
-#ifdef CONFIG_KEXEC_VERIFY_SIG
+#ifdef CONFIG_KEXEC_SIG
kexec_verify_sig_t *verify_sig;
#endif
};
diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
index f1d0e00a3971..67f3a866eabe 100644
--- a/kernel/kexec_file.c
+++ b/kernel/kexec_file.c
@@ -90,7 +90,7 @@ int __weak arch_kimage_file_post_load_cleanup(struct kimage *image)
return kexec_image_post_load_cleanup_default(image);
}
-#ifdef CONFIG_KEXEC_VERIFY_SIG
+#ifdef CONFIG_KEXEC_SIG
static int kexec_image_verify_sig_default(struct kimage *image, void *buf,
unsigned long buf_len)
{
@@ -188,7 +188,8 @@ kimage_file_prepare_segments(struct kimage *image, int kernel_fd, int initrd_fd,
const char __user *cmdline_ptr,
unsigned long cmdline_len, unsigned flags)
{
- int ret = 0;
+ const char *reason;
+ int ret;
void *ldata;
loff_t size;
@@ -207,15 +208,48 @@ kimage_file_prepare_segments(struct kimage *image, int kernel_fd, int initrd_fd,
if (ret)
goto out;
-#ifdef CONFIG_KEXEC_VERIFY_SIG
+#ifdef CONFIG_KEXEC_SIG
ret = arch_kexec_kernel_verify_sig(image, image->kernel_buf,
image->kernel_buf_len);
- if (ret) {
- pr_debug("kernel signature verification failed.\n");
+#else
+ ret = -ENODATA;
+#endif
+
+ switch (ret) {
+ case 0:
+ break;
+
+ /* Certain verification errors are non-fatal if we're not
+ * checking errors, provided we aren't mandating that there
+ * must be a valid signature.
+ */
+ case -ENODATA:
+ reason = "kexec of unsigned image";
+ goto decide;
+ case -ENOPKG:
+ reason = "kexec of image with unsupported crypto";
+ goto decide;
+ case -ENOKEY:
+ reason = "kexec of image with unavailable key";
+ decide:
+ if (IS_ENABLED(CONFIG_KEXEC_SIG_FORCE)) {
+ pr_notice("%s rejected\n", reason);
+ ret = -EKEYREJECTED;
+ goto out;
+ }
+
+ ret = 0;
+ break;
+
+ /* All other errors are fatal, including nomem, unparseable
+ * signatures and signature check failures - even if signatures
+ * aren't required.
+ */
+ default:
+ pr_notice("kernel signature verification failed (%d).\n", ret);
goto out;
}
- pr_debug("kernel signature verification successful.\n");
-#endif
+
/* It is possible that there no initramfs is being loaded */
if (!(flags & KEXEC_FILE_NO_INITRAMFS)) {
ret = kernel_read_file_from_fd(initrd_fd, &image->initrd_buf,
--
2.21.0.352.gf09ad66450-goog
^ permalink raw reply related
* [PATCH 09/27] hibernate: Disable when the kernel is locked down
From: Matthew Garrett @ 2019-03-06 23:58 UTC (permalink / raw)
To: jmorris; +Cc: linux-security-module, linux-kernel, dhowells
In-Reply-To: <20190306235913.6631-1-matthewgarrett@google.com>
From: Josh Boyer <jwboyer@fedoraproject.org>
There is currently no way to verify the resume image when returning
from hibernate. This might compromise the signed modules trust model,
so until we can work with signed hibernate images we disable it when the
kernel is locked down.
Signed-off-by: Josh Boyer <jwboyer@fedoraproject.org>
Signed-off-by: David Howells <dhowells@redhat.com>
Reviewed-by: "Lee, Chun-Yi" <jlee@suse.com>
cc: linux-pm@vger.kernel.org
Signed-off-by: Matthew Garrett <matthewgarrett@google.com>
---
kernel/power/hibernate.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/power/hibernate.c b/kernel/power/hibernate.c
index abef759de7c8..802795becb88 100644
--- a/kernel/power/hibernate.c
+++ b/kernel/power/hibernate.c
@@ -70,7 +70,7 @@ static const struct platform_hibernation_ops *hibernation_ops;
bool hibernation_available(void)
{
- return (nohibernate == 0);
+ return nohibernate == 0 && !kernel_is_locked_down("Hibernation");
}
/**
--
2.21.0.352.gf09ad66450-goog
^ 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