Skip to content

Commit

Permalink
Kbuild: rename CC_STACKPROTECTOR[_STRONG] config variables
Browse files Browse the repository at this point in the history
The changes to automatically test for working stack protector compiler
support in the Kconfig files removed the special STACKPROTECTOR_AUTO
option that picked the strongest stack protector that the compiler
supported.

That was all a nice cleanup - it makes no sense to have the AUTO case
now that the Kconfig phase can just determine the compiler support
directly.

HOWEVER.

It also meant that doing "make oldconfig" would now _disable_ the strong
stackprotector if you had AUTO enabled, because in a legacy config file,
the sane stack protector configuration would look like

  CONFIG_HAVE_CC_STACKPROTECTOR=y
  # CONFIG_CC_STACKPROTECTOR_NONE is not set
  # CONFIG_CC_STACKPROTECTOR_REGULAR is not set
  # CONFIG_CC_STACKPROTECTOR_STRONG is not set
  CONFIG_CC_STACKPROTECTOR_AUTO=y

and when you ran this through "make oldconfig" with the Kbuild changes,
it would ask you about the regular CONFIG_CC_STACKPROTECTOR (that had
been renamed from CONFIG_CC_STACKPROTECTOR_REGULAR to just
CONFIG_CC_STACKPROTECTOR), but it would think that the STRONG version
used to be disabled (because it was really enabled by AUTO), and would
disable it in the new config, resulting in:

  CONFIG_HAVE_CC_STACKPROTECTOR=y
  CONFIG_CC_HAS_STACKPROTECTOR_NONE=y
  CONFIG_CC_STACKPROTECTOR=y
  # CONFIG_CC_STACKPROTECTOR_STRONG is not set
  CONFIG_CC_HAS_SANE_STACKPROTECTOR=y

That's dangerously subtle - people could suddenly find themselves with
the weaker stack protector setup without even realizing.

The solution here is to just rename not just the old RECULAR stack
protector option, but also the strong one.  This does that by just
removing the CC_ prefix entirely for the user choices, because it really
is not about the compiler support (the compiler support now instead
automatially impacts _visibility_ of the options to users).

This results in "make oldconfig" actually asking the user for their
choice, so that we don't have any silent subtle security model changes.
The end result would generally look like this:

  CONFIG_HAVE_CC_STACKPROTECTOR=y
  CONFIG_CC_HAS_STACKPROTECTOR_NONE=y
  CONFIG_STACKPROTECTOR=y
  CONFIG_STACKPROTECTOR_STRONG=y
  CONFIG_CC_HAS_SANE_STACKPROTECTOR=y

where the "CC_" versions really are about internal compiler
infrastructure, not the user selections.

Acked-by: Masahiro Yamada <[email protected]>
Signed-off-by: Linus Torvalds <[email protected]>
  • Loading branch information
torvalds committed Jun 14, 2018
1 parent be779f0 commit 050e9ba
Show file tree
Hide file tree
Showing 33 changed files with 39 additions and 39 deletions.
2 changes: 1 addition & 1 deletion Documentation/kbuild/kconfig-language.txt
Original file line number Diff line number Diff line change
Expand Up @@ -480,7 +480,7 @@ There are several features that need compiler support. The recommended way
to describe the dependency on the compiler feature is to use "depends on"
followed by a test macro.

config CC_STACKPROTECTOR
config STACKPROTECTOR
bool "Stack Protector buffer overflow detection"
depends on $(cc-option,-fstack-protector)
...
Expand Down
2 changes: 1 addition & 1 deletion Documentation/security/self-protection.rst
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ The classic stack buffer overflow involves writing past the expected end
of a variable stored on the stack, ultimately writing a controlled value
to the stack frame's stored return address. The most widely used defense
is the presence of a stack canary between the stack variables and the
return address (``CONFIG_CC_STACKPROTECTOR``), which is verified just before
return address (``CONFIG_STACKPROTECTOR``), which is verified just before
the function returns. Other defenses include things like shadow stacks.

Stack depth overflow
Expand Down
4 changes: 2 additions & 2 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -687,8 +687,8 @@ KBUILD_CFLAGS += $(call cc-option,-Wframe-larger-than=${CONFIG_FRAME_WARN})
endif

stackp-flags-$(CONFIG_CC_HAS_STACKPROTECTOR_NONE) := -fno-stack-protector
stackp-flags-$(CONFIG_CC_STACKPROTECTOR) := -fstack-protector
stackp-flags-$(CONFIG_CC_STACKPROTECTOR_STRONG) := -fstack-protector-strong
stackp-flags-$(CONFIG_STACKPROTECTOR) := -fstack-protector
stackp-flags-$(CONFIG_STACKPROTECTOR_STRONG) := -fstack-protector-strong

KBUILD_CFLAGS += $(stackp-flags-y)

Expand Down
6 changes: 3 additions & 3 deletions arch/Kconfig
Original file line number Diff line number Diff line change
Expand Up @@ -558,7 +558,7 @@ config HAVE_CC_STACKPROTECTOR
config CC_HAS_STACKPROTECTOR_NONE
def_bool $(cc-option,-fno-stack-protector)

config CC_STACKPROTECTOR
config STACKPROTECTOR
bool "Stack Protector buffer overflow detection"
depends on HAVE_CC_STACKPROTECTOR
depends on $(cc-option,-fstack-protector)
Expand All @@ -582,9 +582,9 @@ config CC_STACKPROTECTOR
about 3% of all kernel functions, which increases kernel code size
by about 0.3%.

config CC_STACKPROTECTOR_STRONG
config STACKPROTECTOR_STRONG
bool "Strong Stack Protector"
depends on CC_STACKPROTECTOR
depends on STACKPROTECTOR
depends on $(cc-option,-fstack-protector-strong)
default y
help
Expand Down
2 changes: 1 addition & 1 deletion arch/arm/kernel/asm-offsets.c
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@
int main(void)
{
DEFINE(TSK_ACTIVE_MM, offsetof(struct task_struct, active_mm));
#ifdef CONFIG_CC_STACKPROTECTOR
#ifdef CONFIG_STACKPROTECTOR
DEFINE(TSK_STACK_CANARY, offsetof(struct task_struct, stack_canary));
#endif
BLANK();
Expand Down
4 changes: 2 additions & 2 deletions arch/arm/kernel/entry-armv.S
Original file line number Diff line number Diff line change
Expand Up @@ -791,7 +791,7 @@ ENTRY(__switch_to)
ldr r6, [r2, #TI_CPU_DOMAIN]
#endif
switch_tls r1, r4, r5, r3, r7
#if defined(CONFIG_CC_STACKPROTECTOR) && !defined(CONFIG_SMP)
#if defined(CONFIG_STACKPROTECTOR) && !defined(CONFIG_SMP)
ldr r7, [r2, #TI_TASK]
ldr r8, =__stack_chk_guard
.if (TSK_STACK_CANARY > IMM12_MASK)
Expand All @@ -807,7 +807,7 @@ ENTRY(__switch_to)
ldr r0, =thread_notify_head
mov r1, #THREAD_NOTIFY_SWITCH
bl atomic_notifier_call_chain
#if defined(CONFIG_CC_STACKPROTECTOR) && !defined(CONFIG_SMP)
#if defined(CONFIG_STACKPROTECTOR) && !defined(CONFIG_SMP)
str r7, [r8]
#endif
THUMB( mov ip, r4 )
Expand Down
2 changes: 1 addition & 1 deletion arch/arm/kernel/process.c
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@
#include <asm/tls.h>
#include <asm/vdso.h>

#ifdef CONFIG_CC_STACKPROTECTOR
#ifdef CONFIG_STACKPROTECTOR
#include <linux/stackprotector.h>
unsigned long __stack_chk_guard __read_mostly;
EXPORT_SYMBOL(__stack_chk_guard);
Expand Down
2 changes: 1 addition & 1 deletion arch/arm64/kernel/process.c
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@
#include <asm/processor.h>
#include <asm/stacktrace.h>

#ifdef CONFIG_CC_STACKPROTECTOR
#ifdef CONFIG_STACKPROTECTOR
#include <linux/stackprotector.h>
unsigned long __stack_chk_guard __read_mostly;
EXPORT_SYMBOL(__stack_chk_guard);
Expand Down
2 changes: 1 addition & 1 deletion arch/mips/kernel/asm-offsets.c
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ void output_task_defines(void)
OFFSET(TASK_FLAGS, task_struct, flags);
OFFSET(TASK_MM, task_struct, mm);
OFFSET(TASK_PID, task_struct, pid);
#if defined(CONFIG_CC_STACKPROTECTOR)
#if defined(CONFIG_STACKPROTECTOR)
OFFSET(TASK_STACK_CANARY, task_struct, stack_canary);
#endif
DEFINE(TASK_STRUCT_SIZE, sizeof(struct task_struct));
Expand Down
2 changes: 1 addition & 1 deletion arch/mips/kernel/octeon_switch.S
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@
#endif
3:

#if defined(CONFIG_CC_STACKPROTECTOR) && !defined(CONFIG_SMP)
#if defined(CONFIG_STACKPROTECTOR) && !defined(CONFIG_SMP)
PTR_LA t8, __stack_chk_guard
LONG_L t9, TASK_STACK_CANARY(a1)
LONG_S t9, 0(t8)
Expand Down
2 changes: 1 addition & 1 deletion arch/mips/kernel/process.c
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ int copy_thread_tls(unsigned long clone_flags, unsigned long usp,
return 0;
}

#ifdef CONFIG_CC_STACKPROTECTOR
#ifdef CONFIG_STACKPROTECTOR
#include <linux/stackprotector.h>
unsigned long __stack_chk_guard __read_mostly;
EXPORT_SYMBOL(__stack_chk_guard);
Expand Down
2 changes: 1 addition & 1 deletion arch/mips/kernel/r2300_switch.S
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ LEAF(resume)
cpu_save_nonscratch a0
sw ra, THREAD_REG31(a0)

#if defined(CONFIG_CC_STACKPROTECTOR) && !defined(CONFIG_SMP)
#if defined(CONFIG_STACKPROTECTOR) && !defined(CONFIG_SMP)
PTR_LA t8, __stack_chk_guard
LONG_L t9, TASK_STACK_CANARY(a1)
LONG_S t9, 0(t8)
Expand Down
2 changes: 1 addition & 1 deletion arch/mips/kernel/r4k_switch.S
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
cpu_save_nonscratch a0
LONG_S ra, THREAD_REG31(a0)

#if defined(CONFIG_CC_STACKPROTECTOR) && !defined(CONFIG_SMP)
#if defined(CONFIG_STACKPROTECTOR) && !defined(CONFIG_SMP)
PTR_LA t8, __stack_chk_guard
LONG_L t9, TASK_STACK_CANARY(a1)
LONG_S t9, 0(t8)
Expand Down
2 changes: 1 addition & 1 deletion arch/sh/kernel/process.c
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
struct kmem_cache *task_xstate_cachep = NULL;
unsigned int xstate_size;

#ifdef CONFIG_CC_STACKPROTECTOR
#ifdef CONFIG_STACKPROTECTOR
unsigned long __stack_chk_guard __read_mostly;
EXPORT_SYMBOL(__stack_chk_guard);
#endif
Expand Down
2 changes: 1 addition & 1 deletion arch/sh/kernel/process_32.c
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@ __switch_to(struct task_struct *prev, struct task_struct *next)
{
struct thread_struct *next_t = &next->thread;

#if defined(CONFIG_CC_STACKPROTECTOR) && !defined(CONFIG_SMP)
#if defined(CONFIG_STACKPROTECTOR) && !defined(CONFIG_SMP)
__stack_chk_guard = next->stack_canary;
#endif

Expand Down
2 changes: 1 addition & 1 deletion arch/x86/entry/entry_32.S
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,7 @@ ENTRY(__switch_to_asm)
movl %esp, TASK_threadsp(%eax)
movl TASK_threadsp(%edx), %esp

#ifdef CONFIG_CC_STACKPROTECTOR
#ifdef CONFIG_STACKPROTECTOR
movl TASK_stack_canary(%edx), %ebx
movl %ebx, PER_CPU_VAR(stack_canary)+stack_canary_offset
#endif
Expand Down
2 changes: 1 addition & 1 deletion arch/x86/entry/entry_64.S
Original file line number Diff line number Diff line change
Expand Up @@ -357,7 +357,7 @@ ENTRY(__switch_to_asm)
movq %rsp, TASK_threadsp(%rdi)
movq TASK_threadsp(%rsi), %rsp

#ifdef CONFIG_CC_STACKPROTECTOR
#ifdef CONFIG_STACKPROTECTOR
movq TASK_stack_canary(%rsi), %rbx
movq %rbx, PER_CPU_VAR(irq_stack_union)+stack_canary_offset
#endif
Expand Down
2 changes: 1 addition & 1 deletion arch/x86/include/asm/processor.h
Original file line number Diff line number Diff line change
Expand Up @@ -412,7 +412,7 @@ extern asmlinkage void ignore_sysret(void);
void save_fsgs_for_kvm(void);
#endif
#else /* X86_64 */
#ifdef CONFIG_CC_STACKPROTECTOR
#ifdef CONFIG_STACKPROTECTOR
/*
* Make sure stack canary segment base is cached-aligned:
* "For Intel Atom processors, avoid non zero segment base address
Expand Down
2 changes: 1 addition & 1 deletion arch/x86/include/asm/segment.h
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@
# define __KERNEL_PERCPU 0
#endif

#ifdef CONFIG_CC_STACKPROTECTOR
#ifdef CONFIG_STACKPROTECTOR
# define __KERNEL_STACK_CANARY (GDT_ENTRY_STACK_CANARY*8)
#else
# define __KERNEL_STACK_CANARY 0
Expand Down
6 changes: 3 additions & 3 deletions arch/x86/include/asm/stackprotector.h
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@
#ifndef _ASM_STACKPROTECTOR_H
#define _ASM_STACKPROTECTOR_H 1

#ifdef CONFIG_CC_STACKPROTECTOR
#ifdef CONFIG_STACKPROTECTOR

#include <asm/tsc.h>
#include <asm/processor.h>
Expand Down Expand Up @@ -105,7 +105,7 @@ static inline void load_stack_canary_segment(void)
#endif
}

#else /* CC_STACKPROTECTOR */
#else /* STACKPROTECTOR */

#define GDT_STACK_CANARY_INIT

Expand All @@ -121,5 +121,5 @@ static inline void load_stack_canary_segment(void)
#endif
}

#endif /* CC_STACKPROTECTOR */
#endif /* STACKPROTECTOR */
#endif /* _ASM_STACKPROTECTOR_H */
2 changes: 1 addition & 1 deletion arch/x86/kernel/asm-offsets.c
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
void common(void) {
BLANK();
OFFSET(TASK_threadsp, task_struct, thread.sp);
#ifdef CONFIG_CC_STACKPROTECTOR
#ifdef CONFIG_STACKPROTECTOR
OFFSET(TASK_stack_canary, task_struct, stack_canary);
#endif

Expand Down
2 changes: 1 addition & 1 deletion arch/x86/kernel/asm-offsets_32.c
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ void foo(void)
DEFINE(TSS_sysenter_sp0, offsetof(struct cpu_entry_area, tss.x86_tss.sp0) -
offsetofend(struct cpu_entry_area, entry_stack_page.stack));

#ifdef CONFIG_CC_STACKPROTECTOR
#ifdef CONFIG_STACKPROTECTOR
BLANK();
OFFSET(stack_canary_offset, stack_canary, canary);
#endif
Expand Down
2 changes: 1 addition & 1 deletion arch/x86/kernel/asm-offsets_64.c
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ int main(void)
OFFSET(TSS_sp1, tss_struct, x86_tss.sp1);
BLANK();

#ifdef CONFIG_CC_STACKPROTECTOR
#ifdef CONFIG_STACKPROTECTOR
DEFINE(stack_canary_offset, offsetof(union irq_stack_union, stack_canary));
BLANK();
#endif
Expand Down
2 changes: 1 addition & 1 deletion arch/x86/kernel/cpu/common.c
Original file line number Diff line number Diff line change
Expand Up @@ -1599,7 +1599,7 @@ DEFINE_PER_CPU(unsigned long, cpu_current_top_of_stack) =
(unsigned long)&init_thread_union + THREAD_SIZE;
EXPORT_PER_CPU_SYMBOL(cpu_current_top_of_stack);

#ifdef CONFIG_CC_STACKPROTECTOR
#ifdef CONFIG_STACKPROTECTOR
DEFINE_PER_CPU_ALIGNED(struct stack_canary, stack_canary);
#endif

Expand Down
2 changes: 1 addition & 1 deletion arch/x86/kernel/head_32.S
Original file line number Diff line number Diff line change
Expand Up @@ -375,7 +375,7 @@ ENDPROC(startup_32_smp)
*/
__INIT
setup_once:
#ifdef CONFIG_CC_STACKPROTECTOR
#ifdef CONFIG_STACKPROTECTOR
/*
* Configure the stack canary. The linker can't handle this by
* relocation. Manually set base address in stack canary
Expand Down
2 changes: 1 addition & 1 deletion arch/xtensa/kernel/asm-offsets.c
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ int main(void)
DEFINE(TASK_PID, offsetof (struct task_struct, pid));
DEFINE(TASK_THREAD, offsetof (struct task_struct, thread));
DEFINE(TASK_THREAD_INFO, offsetof (struct task_struct, stack));
#ifdef CONFIG_CC_STACKPROTECTOR
#ifdef CONFIG_STACKPROTECTOR
DEFINE(TASK_STACK_CANARY, offsetof(struct task_struct, stack_canary));
#endif
DEFINE(TASK_STRUCT_SIZE, sizeof (struct task_struct));
Expand Down
2 changes: 1 addition & 1 deletion arch/xtensa/kernel/entry.S
Original file line number Diff line number Diff line change
Expand Up @@ -1971,7 +1971,7 @@ ENTRY(_switch_to)
s32i a1, a2, THREAD_SP # save stack pointer
#endif

#if defined(CONFIG_CC_STACKPROTECTOR) && !defined(CONFIG_SMP)
#if defined(CONFIG_STACKPROTECTOR) && !defined(CONFIG_SMP)
movi a6, __stack_chk_guard
l32i a8, a3, TASK_STACK_CANARY
s32i a8, a6, 0
Expand Down
2 changes: 1 addition & 1 deletion arch/xtensa/kernel/process.c
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ void (*pm_power_off)(void) = NULL;
EXPORT_SYMBOL(pm_power_off);


#ifdef CONFIG_CC_STACKPROTECTOR
#ifdef CONFIG_STACKPROTECTOR
#include <linux/stackprotector.h>
unsigned long __stack_chk_guard __read_mostly;
EXPORT_SYMBOL(__stack_chk_guard);
Expand Down
2 changes: 1 addition & 1 deletion include/linux/sched.h
Original file line number Diff line number Diff line change
Expand Up @@ -742,7 +742,7 @@ struct task_struct {
pid_t pid;
pid_t tgid;

#ifdef CONFIG_CC_STACKPROTECTOR
#ifdef CONFIG_STACKPROTECTOR
/* Canary value for the -fstack-protector GCC feature: */
unsigned long stack_canary;
#endif
Expand Down
2 changes: 1 addition & 1 deletion include/linux/stackprotector.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
#include <linux/sched.h>
#include <linux/random.h>

#ifdef CONFIG_CC_STACKPROTECTOR
#ifdef CONFIG_STACKPROTECTOR
# include <asm/stackprotector.h>
#else
static inline void boot_init_stack_canary(void)
Expand Down
2 changes: 1 addition & 1 deletion kernel/configs/android-recommended.config
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ CONFIG_BLK_DEV_DM=y
CONFIG_BLK_DEV_LOOP=y
CONFIG_BLK_DEV_RAM=y
CONFIG_BLK_DEV_RAM_SIZE=8192
CONFIG_CC_STACKPROTECTOR_STRONG=y
CONFIG_STACKPROTECTOR_STRONG=y
CONFIG_COMPACTION=y
CONFIG_CPU_SW_DOMAIN_PAN=y
CONFIG_DM_CRYPT=y
Expand Down
2 changes: 1 addition & 1 deletion kernel/fork.c
Original file line number Diff line number Diff line change
Expand Up @@ -811,7 +811,7 @@ static struct task_struct *dup_task_struct(struct task_struct *orig, int node)
clear_tsk_need_resched(tsk);
set_task_stack_end_magic(tsk);

#ifdef CONFIG_CC_STACKPROTECTOR
#ifdef CONFIG_STACKPROTECTOR
tsk->stack_canary = get_random_canary();
#endif

Expand Down
2 changes: 1 addition & 1 deletion kernel/panic.c
Original file line number Diff line number Diff line change
Expand Up @@ -623,7 +623,7 @@ static __init int register_warn_debugfs(void)
device_initcall(register_warn_debugfs);
#endif

#ifdef CONFIG_CC_STACKPROTECTOR
#ifdef CONFIG_STACKPROTECTOR

/*
* Called when gcc's -fstack-protector feature is used, and
Expand Down

0 comments on commit 050e9ba

Please sign in to comment.