[PATCH v2 0/5] stackprotector: ascii armor the stack canary

[PATCH v2 0/5] stackprotector: ascii armor the stack canary

[riel]

Zero out the first byte of the stack canary value on 64 bit systems,
in order to mitigate unterminated C string overflows.

The null byte both prevents C string functions from reading the
canary, and from writing it if the canary value were guessed or
obtained through some other means.
    
Reducing the entropy by 8 bits is acceptable on 64-bit systems,
which will still have 56 bits of entropy left, but not on 32
bit systems, so the "ascii armor" canary is only implemented on
64-bit systems.

Inspired by the "ascii armor" code in execshield and Daniel Micay's
linux-hardened tree.

Also see https://github.com/thestinger/linux-hardened/

v2:
 - improve changelogs
 - address Ingo's coding style comments

[PATCH v2 0/5] stackprotector: ascii armor the stack canary

[riel]

Zero out the first byte of the stack canary value on 64 bit systems,
in order to mitigate unterminated C string overflows.

The null byte both prevents C string functions from reading the
canary, and from writing it if the canary value were guessed or
obtained through some other means.
    
Reducing the entropy by 8 bits is acceptable on 64-bit systems,
which will still have 56 bits of entropy left, but not on 32
bit systems, so the "ascii armor" canary is only implemented on
64-bit systems.

Inspired by the "ascii armor" code in execshield and Daniel Micay's
linux-hardened tree.

Also see https://github.com/thestinger/linux-hardened/

v2:
 - improve changelogs
 - address Ingo's coding style comments

[PATCH v2 0/5] stackprotector: ascii armor the stack canary

[riel at redhat.com]

Zero out the first byte of the stack canary value on 64 bit systems,
in order to mitigate unterminated C string overflows.

The null byte both prevents C string functions from reading the
canary, and from writing it if the canary value were guessed or
obtained through some other means.
    
Reducing the entropy by 8 bits is acceptable on 64-bit systems,
which will still have 56 bits of entropy left, but not on 32
bit systems, so the "ascii armor" canary is only implemented on
64-bit systems.

Inspired by the "ascii armor" code in execshield and Daniel Micay's
linux-hardened tree.

Also see https://github.com/thestinger/linux-hardened/

v2:
 - improve changelogs
 - address Ingo's coding style comments

[kernel-hardening] [PATCH v2 0/5] stackprotector: ascii armor the stack canary

[riel]

Zero out the first byte of the stack canary value on 64 bit systems,
in order to mitigate unterminated C string overflows.

The null byte both prevents C string functions from reading the
canary, and from writing it if the canary value were guessed or
obtained through some other means.
    
Reducing the entropy by 8 bits is acceptable on 64-bit systems,
which will still have 56 bits of entropy left, but not on 32
bit systems, so the "ascii armor" canary is only implemented on
64-bit systems.

Inspired by the "ascii armor" code in execshield and Daniel Micay's
linux-hardened tree.

Also see https://github.com/thestinger/linux-hardened/

v2:
 - improve changelogs
 - address Ingo's coding style comments

[PATCH 1/5] random,stackprotect: introduce get_random_canary function

[riel]

From: Rik van Riel <riel@redhat.com>

Introduce the get_random_canary function, which provides a random
unsigned long canary value with the first byte zeroed out on 64
bit architectures, in order to mitigate non-terminated C string
overflows.

The null byte both prevents C string functions from reading the
canary, and from writing it if the canary value were guessed or
obtained through some other means.

Reducing the entropy by 8 bits is acceptable on 64-bit systems,
which will still have 56 bits of entropy left, but not on 32
bit systems, so the "ascii armor" canary is only implemented on
64-bit systems.

Inspired by the "ascii armor" code in the old execshield patches,
and Daniel Micay's linux-hardened tree.

Signed-off-by: Rik van Riel <riel@redhat.com>
---
 include/linux/random.h | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/include/linux/random.h b/include/linux/random.h
index ed5c3838780d..1fa0dc880bd7 100644
--- a/include/linux/random.h
+++ b/include/linux/random.h
@@ -57,6 +57,27 @@ static inline unsigned long get_random_long(void)
 #endif
 }
 
+/*
+ * On 64-bit architectures, protect against non-terminated C string overflows
+ * by zeroing out the first byte of the canary; this leaves 56 bits of entropy.
+ */
+#ifdef CONFIG_64BIT
+# ifdef __LITTLE_ENDIAN
+#  define CANARY_MASK 0xffffffffffffff00UL
+# else /* big endian, 64 bits: */
+#  define CANARY_MASK 0x00ffffffffffffffUL
+# endif
+#else /* 32 bits: */
+# define CANARY_MASK 0xffffffffUL
+#endif
+
+static inline unsigned long get_random_canary(void)
+{
+	unsigned long val = get_random_long();
+
+	return val & CANARY_MASK;
+}
+
 unsigned long randomize_page(unsigned long start, unsigned long range);
 
 u32 prandom_u32(void);
-- 
2.9.3

[PATCH 1/5] random,stackprotect: introduce get_random_canary function

[riel]

From: Rik van Riel <riel@redhat.com>

Introduce the get_random_canary function, which provides a random
unsigned long canary value with the first byte zeroed out on 64
bit architectures, in order to mitigate non-terminated C string
overflows.

The null byte both prevents C string functions from reading the
canary, and from writing it if the canary value were guessed or
obtained through some other means.

Reducing the entropy by 8 bits is acceptable on 64-bit systems,
which will still have 56 bits of entropy left, but not on 32
bit systems, so the "ascii armor" canary is only implemented on
64-bit systems.

Inspired by the "ascii armor" code in the old execshield patches,
and Daniel Micay's linux-hardened tree.

Signed-off-by: Rik van Riel <riel@redhat.com>
---
 include/linux/random.h | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/include/linux/random.h b/include/linux/random.h
index ed5c3838780d..1fa0dc880bd7 100644
--- a/include/linux/random.h
+++ b/include/linux/random.h
@@ -57,6 +57,27 @@ static inline unsigned long get_random_long(void)
 #endif
 }
 
+/*
+ * On 64-bit architectures, protect against non-terminated C string overflows
+ * by zeroing out the first byte of the canary; this leaves 56 bits of entropy.
+ */
+#ifdef CONFIG_64BIT
+# ifdef __LITTLE_ENDIAN
+#  define CANARY_MASK 0xffffffffffffff00UL
+# else /* big endian, 64 bits: */
+#  define CANARY_MASK 0x00ffffffffffffffUL
+# endif
+#else /* 32 bits: */
+# define CANARY_MASK 0xffffffffUL
+#endif
+
+static inline unsigned long get_random_canary(void)
+{
+	unsigned long val = get_random_long();
+
+	return val & CANARY_MASK;
+}
+
 unsigned long randomize_page(unsigned long start, unsigned long range);
 
 u32 prandom_u32(void);
-- 
2.9.3

[PATCH 1/5] random, stackprotect: introduce get_random_canary function

[riel at redhat.com]

From: Rik van Riel <riel@redhat.com>

Introduce the get_random_canary function, which provides a random
unsigned long canary value with the first byte zeroed out on 64
bit architectures, in order to mitigate non-terminated C string
overflows.

The null byte both prevents C string functions from reading the
canary, and from writing it if the canary value were guessed or
obtained through some other means.

Reducing the entropy by 8 bits is acceptable on 64-bit systems,
which will still have 56 bits of entropy left, but not on 32
bit systems, so the "ascii armor" canary is only implemented on
64-bit systems.

Inspired by the "ascii armor" code in the old execshield patches,
and Daniel Micay's linux-hardened tree.

Signed-off-by: Rik van Riel <riel@redhat.com>
---
 include/linux/random.h | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/include/linux/random.h b/include/linux/random.h
index ed5c3838780d..1fa0dc880bd7 100644
--- a/include/linux/random.h
+++ b/include/linux/random.h
@@ -57,6 +57,27 @@ static inline unsigned long get_random_long(void)
 #endif
 }
 
+/*
+ * On 64-bit architectures, protect against non-terminated C string overflows
+ * by zeroing out the first byte of the canary; this leaves 56 bits of entropy.
+ */
+#ifdef CONFIG_64BIT
+# ifdef __LITTLE_ENDIAN
+#  define CANARY_MASK 0xffffffffffffff00UL
+# else /* big endian, 64 bits: */
+#  define CANARY_MASK 0x00ffffffffffffffUL
+# endif
+#else /* 32 bits: */
+# define CANARY_MASK 0xffffffffUL
+#endif
+
+static inline unsigned long get_random_canary(void)
+{
+	unsigned long val = get_random_long();
+
+	return val & CANARY_MASK;
+}
+
 unsigned long randomize_page(unsigned long start, unsigned long range);
 
 u32 prandom_u32(void);
-- 
2.9.3

[kernel-hardening] [PATCH 1/5] random,stackprotect: introduce get_random_canary function

[riel]

From: Rik van Riel <riel@redhat.com>

Introduce the get_random_canary function, which provides a random
unsigned long canary value with the first byte zeroed out on 64
bit architectures, in order to mitigate non-terminated C string
overflows.

The null byte both prevents C string functions from reading the
canary, and from writing it if the canary value were guessed or
obtained through some other means.

Reducing the entropy by 8 bits is acceptable on 64-bit systems,
which will still have 56 bits of entropy left, but not on 32
bit systems, so the "ascii armor" canary is only implemented on
64-bit systems.

Inspired by the "ascii armor" code in the old execshield patches,
and Daniel Micay's linux-hardened tree.

Signed-off-by: Rik van Riel <riel@redhat.com>
---
 include/linux/random.h | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/include/linux/random.h b/include/linux/random.h
index ed5c3838780d..1fa0dc880bd7 100644
--- a/include/linux/random.h
+++ b/include/linux/random.h
@@ -57,6 +57,27 @@ static inline unsigned long get_random_long(void)
 #endif
 }
 
+/*
+ * On 64-bit architectures, protect against non-terminated C string overflows
+ * by zeroing out the first byte of the canary; this leaves 56 bits of entropy.
+ */
+#ifdef CONFIG_64BIT
+# ifdef __LITTLE_ENDIAN
+#  define CANARY_MASK 0xffffffffffffff00UL
+# else /* big endian, 64 bits: */
+#  define CANARY_MASK 0x00ffffffffffffffUL
+# endif
+#else /* 32 bits: */
+# define CANARY_MASK 0xffffffffUL
+#endif
+
+static inline unsigned long get_random_canary(void)
+{
+	unsigned long val = get_random_long();
+
+	return val & CANARY_MASK;
+}
+
 unsigned long randomize_page(unsigned long start, unsigned long range);
 
 u32 prandom_u32(void);
-- 
2.9.3

[PATCH 2/5] fork,random: use get_random_canary to set tsk->stack_canary

[riel]

From: Rik van Riel <riel@redhat.com>

Use the ascii-armor canary to prevent unterminated C string overflows
from being able to successfully overwrite the canary, even if they
somehow obtain the canary value.

Inspired by execshield ascii-armor and Daniel Micay's linux-hardened tree.

Signed-off-by: Rik van Riel <riel@redhat.com>
---
 kernel/fork.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/fork.c b/kernel/fork.c
index aa1076c5e4a9..b3591e9250a8 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -560,7 +560,7 @@ static struct task_struct *dup_task_struct(struct task_struct *orig, int node)
 	set_task_stack_end_magic(tsk);
 
 #ifdef CONFIG_CC_STACKPROTECTOR
-	tsk->stack_canary = get_random_long();
+	tsk->stack_canary = get_random_canary();
 #endif
 
 	/*
-- 
2.9.3

[PATCH 2/5] fork,random: use get_random_canary to set tsk->stack_canary

[riel]

From: Rik van Riel <riel@redhat.com>

Use the ascii-armor canary to prevent unterminated C string overflows
from being able to successfully overwrite the canary, even if they
somehow obtain the canary value.

Inspired by execshield ascii-armor and Daniel Micay's linux-hardened tree.

Signed-off-by: Rik van Riel <riel@redhat.com>
---
 kernel/fork.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/fork.c b/kernel/fork.c
index aa1076c5e4a9..b3591e9250a8 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -560,7 +560,7 @@ static struct task_struct *dup_task_struct(struct task_struct *orig, int node)
 	set_task_stack_end_magic(tsk);
 
 #ifdef CONFIG_CC_STACKPROTECTOR
-	tsk->stack_canary = get_random_long();
+	tsk->stack_canary = get_random_canary();
 #endif
 
 	/*
-- 
2.9.3

[PATCH 2/5] fork, random: use get_random_canary to set tsk->stack_canary

[riel at redhat.com]

From: Rik van Riel <riel@redhat.com>

Use the ascii-armor canary to prevent unterminated C string overflows
from being able to successfully overwrite the canary, even if they
somehow obtain the canary value.

Inspired by execshield ascii-armor and Daniel Micay's linux-hardened tree.

Signed-off-by: Rik van Riel <riel@redhat.com>
---
 kernel/fork.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/fork.c b/kernel/fork.c
index aa1076c5e4a9..b3591e9250a8 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -560,7 +560,7 @@ static struct task_struct *dup_task_struct(struct task_struct *orig, int node)
 	set_task_stack_end_magic(tsk);
 
 #ifdef CONFIG_CC_STACKPROTECTOR
-	tsk->stack_canary = get_random_long();
+	tsk->stack_canary = get_random_canary();
 #endif
 
 	/*
-- 
2.9.3

[kernel-hardening] [PATCH 2/5] fork,random: use get_random_canary to set tsk->stack_canary

[riel]

From: Rik van Riel <riel@redhat.com>

Use the ascii-armor canary to prevent unterminated C string overflows
from being able to successfully overwrite the canary, even if they
somehow obtain the canary value.

Inspired by execshield ascii-armor and Daniel Micay's linux-hardened tree.

Signed-off-by: Rik van Riel <riel@redhat.com>
---
 kernel/fork.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/fork.c b/kernel/fork.c
index aa1076c5e4a9..b3591e9250a8 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -560,7 +560,7 @@ static struct task_struct *dup_task_struct(struct task_struct *orig, int node)
 	set_task_stack_end_magic(tsk);
 
 #ifdef CONFIG_CC_STACKPROTECTOR
-	tsk->stack_canary = get_random_long();
+	tsk->stack_canary = get_random_canary();
 #endif
 
 	/*
-- 
2.9.3

[PATCH 3/5] x86: ascii armor the x86_64 boot init stack canary

[riel]

From: Rik van Riel <riel@redhat.com>

Use the ascii-armor canary to prevent unterminated C string overflows
from being able to successfully overwrite the canary, even if they
somehow obtain the canary value.

Inspired by execshield ascii-armor and Daniel Micay's linux-hardened tree.

Signed-off-by: Rik van Riel <riel@redhat.com>
---
 arch/x86/include/asm/stackprotector.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/x86/include/asm/stackprotector.h b/arch/x86/include/asm/stackprotector.h
index dcbd9bcce714..8abedf1d650e 100644
--- a/arch/x86/include/asm/stackprotector.h
+++ b/arch/x86/include/asm/stackprotector.h
@@ -74,6 +74,7 @@ static __always_inline void boot_init_stack_canary(void)
 	get_random_bytes(&canary, sizeof(canary));
 	tsc = rdtsc();
 	canary += tsc + (tsc << 32UL);
+	canary &= CANARY_MASK;
 
 	current->stack_canary = canary;
 #ifdef CONFIG_X86_64
-- 
2.9.3

[PATCH 3/5] x86: ascii armor the x86_64 boot init stack canary

[riel]

From: Rik van Riel <riel@redhat.com>

Use the ascii-armor canary to prevent unterminated C string overflows
from being able to successfully overwrite the canary, even if they
somehow obtain the canary value.

Inspired by execshield ascii-armor and Daniel Micay's linux-hardened tree.

Signed-off-by: Rik van Riel <riel@redhat.com>
---
 arch/x86/include/asm/stackprotector.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/x86/include/asm/stackprotector.h b/arch/x86/include/asm/stackprotector.h
index dcbd9bcce714..8abedf1d650e 100644
--- a/arch/x86/include/asm/stackprotector.h
+++ b/arch/x86/include/asm/stackprotector.h
@@ -74,6 +74,7 @@ static __always_inline void boot_init_stack_canary(void)
 	get_random_bytes(&canary, sizeof(canary));
 	tsc = rdtsc();
 	canary += tsc + (tsc << 32UL);
+	canary &= CANARY_MASK;
 
 	current->stack_canary = canary;
 #ifdef CONFIG_X86_64
-- 
2.9.3

[PATCH 3/5] x86: ascii armor the x86_64 boot init stack canary

[riel at redhat.com]

From: Rik van Riel <riel@redhat.com>

Use the ascii-armor canary to prevent unterminated C string overflows
from being able to successfully overwrite the canary, even if they
somehow obtain the canary value.

Inspired by execshield ascii-armor and Daniel Micay's linux-hardened tree.

Signed-off-by: Rik van Riel <riel@redhat.com>
---
 arch/x86/include/asm/stackprotector.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/x86/include/asm/stackprotector.h b/arch/x86/include/asm/stackprotector.h
index dcbd9bcce714..8abedf1d650e 100644
--- a/arch/x86/include/asm/stackprotector.h
+++ b/arch/x86/include/asm/stackprotector.h
@@ -74,6 +74,7 @@ static __always_inline void boot_init_stack_canary(void)
 	get_random_bytes(&canary, sizeof(canary));
 	tsc = rdtsc();
 	canary += tsc + (tsc << 32UL);
+	canary &= CANARY_MASK;
 
 	current->stack_canary = canary;
 #ifdef CONFIG_X86_64
-- 
2.9.3

[kernel-hardening] [PATCH 3/5] x86: ascii armor the x86_64 boot init stack canary

[riel]

From: Rik van Riel <riel@redhat.com>

Use the ascii-armor canary to prevent unterminated C string overflows
from being able to successfully overwrite the canary, even if they
somehow obtain the canary value.

Inspired by execshield ascii-armor and Daniel Micay's linux-hardened tree.

Signed-off-by: Rik van Riel <riel@redhat.com>
---
 arch/x86/include/asm/stackprotector.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/x86/include/asm/stackprotector.h b/arch/x86/include/asm/stackprotector.h
index dcbd9bcce714..8abedf1d650e 100644
--- a/arch/x86/include/asm/stackprotector.h
+++ b/arch/x86/include/asm/stackprotector.h
@@ -74,6 +74,7 @@ static __always_inline void boot_init_stack_canary(void)
 	get_random_bytes(&canary, sizeof(canary));
 	tsc = rdtsc();
 	canary += tsc + (tsc << 32UL);
+	canary &= CANARY_MASK;
 
 	current->stack_canary = canary;
 #ifdef CONFIG_X86_64
-- 
2.9.3

[PATCH 4/5] arm64: ascii armor the arm64 boot init stack canary

[riel]

From: Rik van Riel <riel@redhat.com>

Use the ascii-armor canary to prevent unterminated C string overflows
from being able to successfully overwrite the canary, even if they
somehow obtain the canary value.

Inspired by execshield ascii-armor and Daniel Micay's linux-hardened tree.

Signed-off-by: Rik van Riel <riel@redhat.com>
---
 arch/arm64/include/asm/stackprotector.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/include/asm/stackprotector.h b/arch/arm64/include/asm/stackprotector.h
index fe5e287dc56b..b86a0865ddf1 100644
--- a/arch/arm64/include/asm/stackprotector.h
+++ b/arch/arm64/include/asm/stackprotector.h
@@ -30,6 +30,7 @@ static __always_inline void boot_init_stack_canary(void)
 	/* Try to get a semi random initial value. */
 	get_random_bytes(&canary, sizeof(canary));
 	canary ^= LINUX_VERSION_CODE;
+	canary &= CANARY_MASK;
 
 	current->stack_canary = canary;
 	__stack_chk_guard = current->stack_canary;
-- 
2.9.3

[PATCH 4/5] arm64: ascii armor the arm64 boot init stack canary

[riel]

From: Rik van Riel <riel@redhat.com>

Use the ascii-armor canary to prevent unterminated C string overflows
from being able to successfully overwrite the canary, even if they
somehow obtain the canary value.

Inspired by execshield ascii-armor and Daniel Micay's linux-hardened tree.

Signed-off-by: Rik van Riel <riel@redhat.com>
---
 arch/arm64/include/asm/stackprotector.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/include/asm/stackprotector.h b/arch/arm64/include/asm/stackprotector.h
index fe5e287dc56b..b86a0865ddf1 100644
--- a/arch/arm64/include/asm/stackprotector.h
+++ b/arch/arm64/include/asm/stackprotector.h
@@ -30,6 +30,7 @@ static __always_inline void boot_init_stack_canary(void)
 	/* Try to get a semi random initial value. */
 	get_random_bytes(&canary, sizeof(canary));
 	canary ^= LINUX_VERSION_CODE;
+	canary &= CANARY_MASK;
 
 	current->stack_canary = canary;
 	__stack_chk_guard = current->stack_canary;
-- 
2.9.3

[PATCH 4/5] arm64: ascii armor the arm64 boot init stack canary

[riel at redhat.com]

From: Rik van Riel <riel@redhat.com>

Use the ascii-armor canary to prevent unterminated C string overflows
from being able to successfully overwrite the canary, even if they
somehow obtain the canary value.

Inspired by execshield ascii-armor and Daniel Micay's linux-hardened tree.

Signed-off-by: Rik van Riel <riel@redhat.com>
---
 arch/arm64/include/asm/stackprotector.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/include/asm/stackprotector.h b/arch/arm64/include/asm/stackprotector.h
index fe5e287dc56b..b86a0865ddf1 100644
--- a/arch/arm64/include/asm/stackprotector.h
+++ b/arch/arm64/include/asm/stackprotector.h
@@ -30,6 +30,7 @@ static __always_inline void boot_init_stack_canary(void)
 	/* Try to get a semi random initial value. */
 	get_random_bytes(&canary, sizeof(canary));
 	canary ^= LINUX_VERSION_CODE;
+	canary &= CANARY_MASK;
 
 	current->stack_canary = canary;
 	__stack_chk_guard = current->stack_canary;
-- 
2.9.3

[kernel-hardening] [PATCH 4/5] arm64: ascii armor the arm64 boot init stack canary

[riel]

From: Rik van Riel <riel@redhat.com>

Use the ascii-armor canary to prevent unterminated C string overflows
from being able to successfully overwrite the canary, even if they
somehow obtain the canary value.

Inspired by execshield ascii-armor and Daniel Micay's linux-hardened tree.

Signed-off-by: Rik van Riel <riel@redhat.com>
---
 arch/arm64/include/asm/stackprotector.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/include/asm/stackprotector.h b/arch/arm64/include/asm/stackprotector.h
index fe5e287dc56b..b86a0865ddf1 100644
--- a/arch/arm64/include/asm/stackprotector.h
+++ b/arch/arm64/include/asm/stackprotector.h
@@ -30,6 +30,7 @@ static __always_inline void boot_init_stack_canary(void)
 	/* Try to get a semi random initial value. */
 	get_random_bytes(&canary, sizeof(canary));
 	canary ^= LINUX_VERSION_CODE;
+	canary &= CANARY_MASK;
 
 	current->stack_canary = canary;
 	__stack_chk_guard = current->stack_canary;
-- 
2.9.3

[PATCH 5/5] sh64: ascii armor the sh64 boot init stack canary

[riel]

From: Rik van Riel <riel@redhat.com>

Use the ascii-armor canary to prevent unterminated C string overflows
from being able to successfully overwrite the canary, even if they
somehow obtain the canary value.

Inspired by execshield ascii-armor and Daniel Micay's linux-hardened tree.

Signed-off-by: Rik van Riel <riel@redhat.com>
---
 arch/sh/include/asm/stackprotector.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/sh/include/asm/stackprotector.h b/arch/sh/include/asm/stackprotector.h
index d9df3a76847c..141515a43b78 100644
--- a/arch/sh/include/asm/stackprotector.h
+++ b/arch/sh/include/asm/stackprotector.h
@@ -19,6 +19,7 @@ static __always_inline void boot_init_stack_canary(void)
 	/* Try to get a semi random initial value. */
 	get_random_bytes(&canary, sizeof(canary));
 	canary ^= LINUX_VERSION_CODE;
+	canary &= CANARY_MASK;
 
 	current->stack_canary = canary;
 	__stack_chk_guard = current->stack_canary;
-- 
2.9.3

[PATCH 5/5] sh64: ascii armor the sh64 boot init stack canary

[riel]

From: Rik van Riel <riel@redhat.com>

Use the ascii-armor canary to prevent unterminated C string overflows
from being able to successfully overwrite the canary, even if they
somehow obtain the canary value.

Inspired by execshield ascii-armor and Daniel Micay's linux-hardened tree.

Signed-off-by: Rik van Riel <riel@redhat.com>
---
 arch/sh/include/asm/stackprotector.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/sh/include/asm/stackprotector.h b/arch/sh/include/asm/stackprotector.h
index d9df3a76847c..141515a43b78 100644
--- a/arch/sh/include/asm/stackprotector.h
+++ b/arch/sh/include/asm/stackprotector.h
@@ -19,6 +19,7 @@ static __always_inline void boot_init_stack_canary(void)
 	/* Try to get a semi random initial value. */
 	get_random_bytes(&canary, sizeof(canary));
 	canary ^= LINUX_VERSION_CODE;
+	canary &= CANARY_MASK;
 
 	current->stack_canary = canary;
 	__stack_chk_guard = current->stack_canary;
-- 
2.9.3

[PATCH 5/5] sh64: ascii armor the sh64 boot init stack canary

[riel at redhat.com]

From: Rik van Riel <riel@redhat.com>

Use the ascii-armor canary to prevent unterminated C string overflows
from being able to successfully overwrite the canary, even if they
somehow obtain the canary value.

Inspired by execshield ascii-armor and Daniel Micay's linux-hardened tree.

Signed-off-by: Rik van Riel <riel@redhat.com>
---
 arch/sh/include/asm/stackprotector.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/sh/include/asm/stackprotector.h b/arch/sh/include/asm/stackprotector.h
index d9df3a76847c..141515a43b78 100644
--- a/arch/sh/include/asm/stackprotector.h
+++ b/arch/sh/include/asm/stackprotector.h
@@ -19,6 +19,7 @@ static __always_inline void boot_init_stack_canary(void)
 	/* Try to get a semi random initial value. */
 	get_random_bytes(&canary, sizeof(canary));
 	canary ^= LINUX_VERSION_CODE;
+	canary &= CANARY_MASK;
 
 	current->stack_canary = canary;
 	__stack_chk_guard = current->stack_canary;
-- 
2.9.3

[kernel-hardening] [PATCH 5/5] sh64: ascii armor the sh64 boot init stack canary

[riel]

From: Rik van Riel <riel@redhat.com>

Use the ascii-armor canary to prevent unterminated C string overflows
from being able to successfully overwrite the canary, even if they
somehow obtain the canary value.

Inspired by execshield ascii-armor and Daniel Micay's linux-hardened tree.

Signed-off-by: Rik van Riel <riel@redhat.com>
---
 arch/sh/include/asm/stackprotector.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/sh/include/asm/stackprotector.h b/arch/sh/include/asm/stackprotector.h
index d9df3a76847c..141515a43b78 100644
--- a/arch/sh/include/asm/stackprotector.h
+++ b/arch/sh/include/asm/stackprotector.h
@@ -19,6 +19,7 @@ static __always_inline void boot_init_stack_canary(void)
 	/* Try to get a semi random initial value. */
 	get_random_bytes(&canary, sizeof(canary));
 	canary ^= LINUX_VERSION_CODE;
+	canary &= CANARY_MASK;
 
 	current->stack_canary = canary;
 	__stack_chk_guard = current->stack_canary;
-- 
2.9.3

Re: [PATCH 1/5] random,stackprotect: introduce get_random_canary function

[Kees Cook]

On Wed, May 24, 2017 at 8:57 AM,  <riel@redhat.com> wrote:
> From: Rik van Riel <riel@redhat.com>
>
> Introduce the get_random_canary function, which provides a random
> unsigned long canary value with the first byte zeroed out on 64
> bit architectures, in order to mitigate non-terminated C string
> overflows.
>
> The null byte both prevents C string functions from reading the
> canary, and from writing it if the canary value were guessed or
> obtained through some other means.
>
> Reducing the entropy by 8 bits is acceptable on 64-bit systems,
> which will still have 56 bits of entropy left, but not on 32
> bit systems, so the "ascii armor" canary is only implemented on
> 64-bit systems.
>
> Inspired by the "ascii armor" code in the old execshield patches,
> and Daniel Micay's linux-hardened tree.
>
> Signed-off-by: Rik van Riel <riel@redhat.com>

Since the other recent canary fix went via -mm, perhaps these should
go that way too? If not, I can take them via my KSPP tree.

Acked-by: Kees Cook <keescook@chromium.org>

-Kees

> ---
>  include/linux/random.h | 21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)
>
> diff --git a/include/linux/random.h b/include/linux/random.h
> index ed5c3838780d..1fa0dc880bd7 100644
> --- a/include/linux/random.h
> +++ b/include/linux/random.h
> @@ -57,6 +57,27 @@ static inline unsigned long get_random_long(void)
>  #endif
>  }
>
> +/*
> + * On 64-bit architectures, protect against non-terminated C string overflows
> + * by zeroing out the first byte of the canary; this leaves 56 bits of entropy.
> + */
> +#ifdef CONFIG_64BIT
> +# ifdef __LITTLE_ENDIAN
> +#  define CANARY_MASK 0xffffffffffffff00UL
> +# else /* big endian, 64 bits: */
> +#  define CANARY_MASK 0x00ffffffffffffffUL
> +# endif
> +#else /* 32 bits: */
> +# define CANARY_MASK 0xffffffffUL
> +#endif
> +
> +static inline unsigned long get_random_canary(void)
> +{
> +       unsigned long val = get_random_long();
> +
> +       return val & CANARY_MASK;
> +}
> +
>  unsigned long randomize_page(unsigned long start, unsigned long range);
>
>  u32 prandom_u32(void);
> --
> 2.9.3
>



-- 
Kees Cook
Pixel Security

Re: [PATCH 1/5] random,stackprotect: introduce get_random_canary function

[Kees Cook]

On Wed, May 24, 2017 at 8:57 AM,  <riel@redhat.com> wrote:
> From: Rik van Riel <riel@redhat.com>
>
> Introduce the get_random_canary function, which provides a random
> unsigned long canary value with the first byte zeroed out on 64
> bit architectures, in order to mitigate non-terminated C string
> overflows.
>
> The null byte both prevents C string functions from reading the
> canary, and from writing it if the canary value were guessed or
> obtained through some other means.
>
> Reducing the entropy by 8 bits is acceptable on 64-bit systems,
> which will still have 56 bits of entropy left, but not on 32
> bit systems, so the "ascii armor" canary is only implemented on
> 64-bit systems.
>
> Inspired by the "ascii armor" code in the old execshield patches,
> and Daniel Micay's linux-hardened tree.
>
> Signed-off-by: Rik van Riel <riel@redhat.com>

Since the other recent canary fix went via -mm, perhaps these should
go that way too? If not, I can take them via my KSPP tree.

Acked-by: Kees Cook <keescook@chromium.org>

-Kees

> ---
>  include/linux/random.h | 21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)
>
> diff --git a/include/linux/random.h b/include/linux/random.h
> index ed5c3838780d..1fa0dc880bd7 100644
> --- a/include/linux/random.h
> +++ b/include/linux/random.h
> @@ -57,6 +57,27 @@ static inline unsigned long get_random_long(void)
>  #endif
>  }
>
> +/*
> + * On 64-bit architectures, protect against non-terminated C string overflows
> + * by zeroing out the first byte of the canary; this leaves 56 bits of entropy.
> + */
> +#ifdef CONFIG_64BIT
> +# ifdef __LITTLE_ENDIAN
> +#  define CANARY_MASK 0xffffffffffffff00UL
> +# else /* big endian, 64 bits: */
> +#  define CANARY_MASK 0x00ffffffffffffffUL
> +# endif
> +#else /* 32 bits: */
> +# define CANARY_MASK 0xffffffffUL
> +#endif
> +
> +static inline unsigned long get_random_canary(void)
> +{
> +       unsigned long val = get_random_long();
> +
> +       return val & CANARY_MASK;
> +}
> +
>  unsigned long randomize_page(unsigned long start, unsigned long range);
>
>  u32 prandom_u32(void);
> --
> 2.9.3
>



-- 
Kees Cook
Pixel Security

[PATCH 1/5] random, stackprotect: introduce get_random_canary function

[Kees Cook]

On Wed, May 24, 2017 at 8:57 AM,  <riel@redhat.com> wrote:
> From: Rik van Riel <riel@redhat.com>
>
> Introduce the get_random_canary function, which provides a random
> unsigned long canary value with the first byte zeroed out on 64
> bit architectures, in order to mitigate non-terminated C string
> overflows.
>
> The null byte both prevents C string functions from reading the
> canary, and from writing it if the canary value were guessed or
> obtained through some other means.
>
> Reducing the entropy by 8 bits is acceptable on 64-bit systems,
> which will still have 56 bits of entropy left, but not on 32
> bit systems, so the "ascii armor" canary is only implemented on
> 64-bit systems.
>
> Inspired by the "ascii armor" code in the old execshield patches,
> and Daniel Micay's linux-hardened tree.
>
> Signed-off-by: Rik van Riel <riel@redhat.com>

Since the other recent canary fix went via -mm, perhaps these should
go that way too? If not, I can take them via my KSPP tree.

Acked-by: Kees Cook <keescook@chromium.org>

-Kees

> ---
>  include/linux/random.h | 21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)
>
> diff --git a/include/linux/random.h b/include/linux/random.h
> index ed5c3838780d..1fa0dc880bd7 100644
> --- a/include/linux/random.h
> +++ b/include/linux/random.h
> @@ -57,6 +57,27 @@ static inline unsigned long get_random_long(void)
>  #endif
>  }
>
> +/*
> + * On 64-bit architectures, protect against non-terminated C string overflows
> + * by zeroing out the first byte of the canary; this leaves 56 bits of entropy.
> + */
> +#ifdef CONFIG_64BIT
> +# ifdef __LITTLE_ENDIAN
> +#  define CANARY_MASK 0xffffffffffffff00UL
> +# else /* big endian, 64 bits: */
> +#  define CANARY_MASK 0x00ffffffffffffffUL
> +# endif
> +#else /* 32 bits: */
> +# define CANARY_MASK 0xffffffffUL
> +#endif
> +
> +static inline unsigned long get_random_canary(void)
> +{
> +       unsigned long val = get_random_long();
> +
> +       return val & CANARY_MASK;
> +}
> +
>  unsigned long randomize_page(unsigned long start, unsigned long range);
>
>  u32 prandom_u32(void);
> --
> 2.9.3
>



-- 
Kees Cook
Pixel Security

[kernel-hardening] Re: [PATCH 1/5] random,stackprotect: introduce get_random_canary function

[Kees Cook]

On Wed, May 24, 2017 at 8:57 AM,  <riel@redhat.com> wrote:
> From: Rik van Riel <riel@redhat.com>
>
> Introduce the get_random_canary function, which provides a random
> unsigned long canary value with the first byte zeroed out on 64
> bit architectures, in order to mitigate non-terminated C string
> overflows.
>
> The null byte both prevents C string functions from reading the
> canary, and from writing it if the canary value were guessed or
> obtained through some other means.
>
> Reducing the entropy by 8 bits is acceptable on 64-bit systems,
> which will still have 56 bits of entropy left, but not on 32
> bit systems, so the "ascii armor" canary is only implemented on
> 64-bit systems.
>
> Inspired by the "ascii armor" code in the old execshield patches,
> and Daniel Micay's linux-hardened tree.
>
> Signed-off-by: Rik van Riel <riel@redhat.com>

Since the other recent canary fix went via -mm, perhaps these should
go that way too? If not, I can take them via my KSPP tree.

Acked-by: Kees Cook <keescook@chromium.org>

-Kees

> ---
>  include/linux/random.h | 21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)
>
> diff --git a/include/linux/random.h b/include/linux/random.h
> index ed5c3838780d..1fa0dc880bd7 100644
> --- a/include/linux/random.h
> +++ b/include/linux/random.h
> @@ -57,6 +57,27 @@ static inline unsigned long get_random_long(void)
>  #endif
>  }
>
> +/*
> + * On 64-bit architectures, protect against non-terminated C string overflows
> + * by zeroing out the first byte of the canary; this leaves 56 bits of entropy.
> + */
> +#ifdef CONFIG_64BIT
> +# ifdef __LITTLE_ENDIAN
> +#  define CANARY_MASK 0xffffffffffffff00UL
> +# else /* big endian, 64 bits: */
> +#  define CANARY_MASK 0x00ffffffffffffffUL
> +# endif
> +#else /* 32 bits: */
> +# define CANARY_MASK 0xffffffffUL
> +#endif
> +
> +static inline unsigned long get_random_canary(void)
> +{
> +       unsigned long val = get_random_long();
> +
> +       return val & CANARY_MASK;
> +}
> +
>  unsigned long randomize_page(unsigned long start, unsigned long range);
>
>  u32 prandom_u32(void);
> --
> 2.9.3
>



-- 
Kees Cook
Pixel Security

Re: [PATCH 2/5] fork,random: use get_random_canary to set tsk->stack_canary

[Kees Cook]

On Wed, May 24, 2017 at 8:57 AM,  <riel@redhat.com> wrote:
> From: Rik van Riel <riel@redhat.com>
>
> Use the ascii-armor canary to prevent unterminated C string overflows
> from being able to successfully overwrite the canary, even if they
> somehow obtain the canary value.
>
> Inspired by execshield ascii-armor and Daniel Micay's linux-hardened tree.
>
> Signed-off-by: Rik van Riel <riel@redhat.com>

Acked-by: Kees Cook <keescook@chromium.org>

-Kees

> ---
>  kernel/fork.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/kernel/fork.c b/kernel/fork.c
> index aa1076c5e4a9..b3591e9250a8 100644
> --- a/kernel/fork.c
> +++ b/kernel/fork.c
> @@ -560,7 +560,7 @@ static struct task_struct *dup_task_struct(struct task_struct *orig, int node)
>         set_task_stack_end_magic(tsk);
>
>  #ifdef CONFIG_CC_STACKPROTECTOR
> -       tsk->stack_canary = get_random_long();
> +       tsk->stack_canary = get_random_canary();
>  #endif
>
>         /*
> --
> 2.9.3
>



-- 
Kees Cook
Pixel Security

Re: [PATCH 2/5] fork,random: use get_random_canary to set tsk->stack_canary

[Kees Cook]

On Wed, May 24, 2017 at 8:57 AM,  <riel@redhat.com> wrote:
> From: Rik van Riel <riel@redhat.com>
>
> Use the ascii-armor canary to prevent unterminated C string overflows
> from being able to successfully overwrite the canary, even if they
> somehow obtain the canary value.
>
> Inspired by execshield ascii-armor and Daniel Micay's linux-hardened tree.
>
> Signed-off-by: Rik van Riel <riel@redhat.com>

Acked-by: Kees Cook <keescook@chromium.org>

-Kees

> ---
>  kernel/fork.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/kernel/fork.c b/kernel/fork.c
> index aa1076c5e4a9..b3591e9250a8 100644
> --- a/kernel/fork.c
> +++ b/kernel/fork.c
> @@ -560,7 +560,7 @@ static struct task_struct *dup_task_struct(struct task_struct *orig, int node)
>         set_task_stack_end_magic(tsk);
>
>  #ifdef CONFIG_CC_STACKPROTECTOR
> -       tsk->stack_canary = get_random_long();
> +       tsk->stack_canary = get_random_canary();
>  #endif
>
>         /*
> --
> 2.9.3
>



-- 
Kees Cook
Pixel Security

[PATCH 2/5] fork, random: use get_random_canary to set tsk->stack_canary

[Kees Cook]

On Wed, May 24, 2017 at 8:57 AM,  <riel@redhat.com> wrote:
> From: Rik van Riel <riel@redhat.com>
>
> Use the ascii-armor canary to prevent unterminated C string overflows
> from being able to successfully overwrite the canary, even if they
> somehow obtain the canary value.
>
> Inspired by execshield ascii-armor and Daniel Micay's linux-hardened tree.
>
> Signed-off-by: Rik van Riel <riel@redhat.com>

Acked-by: Kees Cook <keescook@chromium.org>

-Kees

> ---
>  kernel/fork.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/kernel/fork.c b/kernel/fork.c
> index aa1076c5e4a9..b3591e9250a8 100644
> --- a/kernel/fork.c
> +++ b/kernel/fork.c
> @@ -560,7 +560,7 @@ static struct task_struct *dup_task_struct(struct task_struct *orig, int node)
>         set_task_stack_end_magic(tsk);
>
>  #ifdef CONFIG_CC_STACKPROTECTOR
> -       tsk->stack_canary = get_random_long();
> +       tsk->stack_canary = get_random_canary();
>  #endif
>
>         /*
> --
> 2.9.3
>



-- 
Kees Cook
Pixel Security

[kernel-hardening] Re: [PATCH 2/5] fork,random: use get_random_canary to set tsk->stack_canary

[Kees Cook]

On Wed, May 24, 2017 at 8:57 AM,  <riel@redhat.com> wrote:
> From: Rik van Riel <riel@redhat.com>
>
> Use the ascii-armor canary to prevent unterminated C string overflows
> from being able to successfully overwrite the canary, even if they
> somehow obtain the canary value.
>
> Inspired by execshield ascii-armor and Daniel Micay's linux-hardened tree.
>
> Signed-off-by: Rik van Riel <riel@redhat.com>

Acked-by: Kees Cook <keescook@chromium.org>

-Kees

> ---
>  kernel/fork.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/kernel/fork.c b/kernel/fork.c
> index aa1076c5e4a9..b3591e9250a8 100644
> --- a/kernel/fork.c
> +++ b/kernel/fork.c
> @@ -560,7 +560,7 @@ static struct task_struct *dup_task_struct(struct task_struct *orig, int node)
>         set_task_stack_end_magic(tsk);
>
>  #ifdef CONFIG_CC_STACKPROTECTOR
> -       tsk->stack_canary = get_random_long();
> +       tsk->stack_canary = get_random_canary();
>  #endif
>
>         /*
> --
> 2.9.3
>



-- 
Kees Cook
Pixel Security

Re: [PATCH 3/5] x86: ascii armor the x86_64 boot init stack canary

[Kees Cook]

On Wed, May 24, 2017 at 8:57 AM,  <riel@redhat.com> wrote:
> From: Rik van Riel <riel@redhat.com>
>
> Use the ascii-armor canary to prevent unterminated C string overflows
> from being able to successfully overwrite the canary, even if they
> somehow obtain the canary value.
>
> Inspired by execshield ascii-armor and Daniel Micay's linux-hardened tree.
>
> Signed-off-by: Rik van Riel <riel@redhat.com>

Acked-by: Kees Cook <keescook@chromium.org>

-Kees

> ---
>  arch/x86/include/asm/stackprotector.h | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/arch/x86/include/asm/stackprotector.h b/arch/x86/include/asm/stackprotector.h
> index dcbd9bcce714..8abedf1d650e 100644
> --- a/arch/x86/include/asm/stackprotector.h
> +++ b/arch/x86/include/asm/stackprotector.h
> @@ -74,6 +74,7 @@ static __always_inline void boot_init_stack_canary(void)
>         get_random_bytes(&canary, sizeof(canary));
>         tsc = rdtsc();
>         canary += tsc + (tsc << 32UL);
> +       canary &= CANARY_MASK;
>
>         current->stack_canary = canary;
>  #ifdef CONFIG_X86_64
> --
> 2.9.3
>



-- 
Kees Cook
Pixel Security

Re: [PATCH 3/5] x86: ascii armor the x86_64 boot init stack canary

[Kees Cook]

On Wed, May 24, 2017 at 8:57 AM,  <riel@redhat.com> wrote:
> From: Rik van Riel <riel@redhat.com>
>
> Use the ascii-armor canary to prevent unterminated C string overflows
> from being able to successfully overwrite the canary, even if they
> somehow obtain the canary value.
>
> Inspired by execshield ascii-armor and Daniel Micay's linux-hardened tree.
>
> Signed-off-by: Rik van Riel <riel@redhat.com>

Acked-by: Kees Cook <keescook@chromium.org>

-Kees

> ---
>  arch/x86/include/asm/stackprotector.h | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/arch/x86/include/asm/stackprotector.h b/arch/x86/include/asm/stackprotector.h
> index dcbd9bcce714..8abedf1d650e 100644
> --- a/arch/x86/include/asm/stackprotector.h
> +++ b/arch/x86/include/asm/stackprotector.h
> @@ -74,6 +74,7 @@ static __always_inline void boot_init_stack_canary(void)
>         get_random_bytes(&canary, sizeof(canary));
>         tsc = rdtsc();
>         canary += tsc + (tsc << 32UL);
> +       canary &= CANARY_MASK;
>
>         current->stack_canary = canary;
>  #ifdef CONFIG_X86_64
> --
> 2.9.3
>



-- 
Kees Cook
Pixel Security

[PATCH 3/5] x86: ascii armor the x86_64 boot init stack canary

[Kees Cook]

On Wed, May 24, 2017 at 8:57 AM,  <riel@redhat.com> wrote:
> From: Rik van Riel <riel@redhat.com>
>
> Use the ascii-armor canary to prevent unterminated C string overflows
> from being able to successfully overwrite the canary, even if they
> somehow obtain the canary value.
>
> Inspired by execshield ascii-armor and Daniel Micay's linux-hardened tree.
>
> Signed-off-by: Rik van Riel <riel@redhat.com>

Acked-by: Kees Cook <keescook@chromium.org>

-Kees

> ---
>  arch/x86/include/asm/stackprotector.h | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/arch/x86/include/asm/stackprotector.h b/arch/x86/include/asm/stackprotector.h
> index dcbd9bcce714..8abedf1d650e 100644
> --- a/arch/x86/include/asm/stackprotector.h
> +++ b/arch/x86/include/asm/stackprotector.h
> @@ -74,6 +74,7 @@ static __always_inline void boot_init_stack_canary(void)
>         get_random_bytes(&canary, sizeof(canary));
>         tsc = rdtsc();
>         canary += tsc + (tsc << 32UL);
> +       canary &= CANARY_MASK;
>
>         current->stack_canary = canary;
>  #ifdef CONFIG_X86_64
> --
> 2.9.3
>



-- 
Kees Cook
Pixel Security

[kernel-hardening] Re: [PATCH 3/5] x86: ascii armor the x86_64 boot init stack canary

[Kees Cook]

On Wed, May 24, 2017 at 8:57 AM,  <riel@redhat.com> wrote:
> From: Rik van Riel <riel@redhat.com>
>
> Use the ascii-armor canary to prevent unterminated C string overflows
> from being able to successfully overwrite the canary, even if they
> somehow obtain the canary value.
>
> Inspired by execshield ascii-armor and Daniel Micay's linux-hardened tree.
>
> Signed-off-by: Rik van Riel <riel@redhat.com>

Acked-by: Kees Cook <keescook@chromium.org>

-Kees

> ---
>  arch/x86/include/asm/stackprotector.h | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/arch/x86/include/asm/stackprotector.h b/arch/x86/include/asm/stackprotector.h
> index dcbd9bcce714..8abedf1d650e 100644
> --- a/arch/x86/include/asm/stackprotector.h
> +++ b/arch/x86/include/asm/stackprotector.h
> @@ -74,6 +74,7 @@ static __always_inline void boot_init_stack_canary(void)
>         get_random_bytes(&canary, sizeof(canary));
>         tsc = rdtsc();
>         canary += tsc + (tsc << 32UL);
> +       canary &= CANARY_MASK;
>
>         current->stack_canary = canary;
>  #ifdef CONFIG_X86_64
> --
> 2.9.3
>



-- 
Kees Cook
Pixel Security

Re: [PATCH 4/5] arm64: ascii armor the arm64 boot init stack canary

[Kees Cook]

On Wed, May 24, 2017 at 8:57 AM,  <riel@redhat.com> wrote:
> From: Rik van Riel <riel@redhat.com>
>
> Use the ascii-armor canary to prevent unterminated C string overflows
> from being able to successfully overwrite the canary, even if they
> somehow obtain the canary value.
>
> Inspired by execshield ascii-armor and Daniel Micay's linux-hardened tree.
>
> Signed-off-by: Rik van Riel <riel@redhat.com>

Acked-by: Kees Cook <keescook@chromium.org>

-Kees

> ---
>  arch/arm64/include/asm/stackprotector.h | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/arch/arm64/include/asm/stackprotector.h b/arch/arm64/include/asm/stackprotector.h
> index fe5e287dc56b..b86a0865ddf1 100644
> --- a/arch/arm64/include/asm/stackprotector.h
> +++ b/arch/arm64/include/asm/stackprotector.h
> @@ -30,6 +30,7 @@ static __always_inline void boot_init_stack_canary(void)
>         /* Try to get a semi random initial value. */
>         get_random_bytes(&canary, sizeof(canary));
>         canary ^= LINUX_VERSION_CODE;
> +       canary &= CANARY_MASK;
>
>         current->stack_canary = canary;
>         __stack_chk_guard = current->stack_canary;
> --
> 2.9.3
>



-- 
Kees Cook
Pixel Security

Re: [PATCH 4/5] arm64: ascii armor the arm64 boot init stack canary

[Kees Cook]

On Wed, May 24, 2017 at 8:57 AM,  <riel@redhat.com> wrote:
> From: Rik van Riel <riel@redhat.com>
>
> Use the ascii-armor canary to prevent unterminated C string overflows
> from being able to successfully overwrite the canary, even if they
> somehow obtain the canary value.
>
> Inspired by execshield ascii-armor and Daniel Micay's linux-hardened tree.
>
> Signed-off-by: Rik van Riel <riel@redhat.com>

Acked-by: Kees Cook <keescook@chromium.org>

-Kees

> ---
>  arch/arm64/include/asm/stackprotector.h | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/arch/arm64/include/asm/stackprotector.h b/arch/arm64/include/asm/stackprotector.h
> index fe5e287dc56b..b86a0865ddf1 100644
> --- a/arch/arm64/include/asm/stackprotector.h
> +++ b/arch/arm64/include/asm/stackprotector.h
> @@ -30,6 +30,7 @@ static __always_inline void boot_init_stack_canary(void)
>         /* Try to get a semi random initial value. */
>         get_random_bytes(&canary, sizeof(canary));
>         canary ^= LINUX_VERSION_CODE;
> +       canary &= CANARY_MASK;
>
>         current->stack_canary = canary;
>         __stack_chk_guard = current->stack_canary;
> --
> 2.9.3
>



-- 
Kees Cook
Pixel Security

[PATCH 4/5] arm64: ascii armor the arm64 boot init stack canary

[Kees Cook]

On Wed, May 24, 2017 at 8:57 AM,  <riel@redhat.com> wrote:
> From: Rik van Riel <riel@redhat.com>
>
> Use the ascii-armor canary to prevent unterminated C string overflows
> from being able to successfully overwrite the canary, even if they
> somehow obtain the canary value.
>
> Inspired by execshield ascii-armor and Daniel Micay's linux-hardened tree.
>
> Signed-off-by: Rik van Riel <riel@redhat.com>

Acked-by: Kees Cook <keescook@chromium.org>

-Kees

> ---
>  arch/arm64/include/asm/stackprotector.h | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/arch/arm64/include/asm/stackprotector.h b/arch/arm64/include/asm/stackprotector.h
> index fe5e287dc56b..b86a0865ddf1 100644
> --- a/arch/arm64/include/asm/stackprotector.h
> +++ b/arch/arm64/include/asm/stackprotector.h
> @@ -30,6 +30,7 @@ static __always_inline void boot_init_stack_canary(void)
>         /* Try to get a semi random initial value. */
>         get_random_bytes(&canary, sizeof(canary));
>         canary ^= LINUX_VERSION_CODE;
> +       canary &= CANARY_MASK;
>
>         current->stack_canary = canary;
>         __stack_chk_guard = current->stack_canary;
> --
> 2.9.3
>



-- 
Kees Cook
Pixel Security

[kernel-hardening] Re: [PATCH 4/5] arm64: ascii armor the arm64 boot init stack canary

[Kees Cook]

On Wed, May 24, 2017 at 8:57 AM,  <riel@redhat.com> wrote:
> From: Rik van Riel <riel@redhat.com>
>
> Use the ascii-armor canary to prevent unterminated C string overflows
> from being able to successfully overwrite the canary, even if they
> somehow obtain the canary value.
>
> Inspired by execshield ascii-armor and Daniel Micay's linux-hardened tree.
>
> Signed-off-by: Rik van Riel <riel@redhat.com>

Acked-by: Kees Cook <keescook@chromium.org>

-Kees

> ---
>  arch/arm64/include/asm/stackprotector.h | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/arch/arm64/include/asm/stackprotector.h b/arch/arm64/include/asm/stackprotector.h
> index fe5e287dc56b..b86a0865ddf1 100644
> --- a/arch/arm64/include/asm/stackprotector.h
> +++ b/arch/arm64/include/asm/stackprotector.h
> @@ -30,6 +30,7 @@ static __always_inline void boot_init_stack_canary(void)
>         /* Try to get a semi random initial value. */
>         get_random_bytes(&canary, sizeof(canary));
>         canary ^= LINUX_VERSION_CODE;
> +       canary &= CANARY_MASK;
>
>         current->stack_canary = canary;
>         __stack_chk_guard = current->stack_canary;
> --
> 2.9.3
>



-- 
Kees Cook
Pixel Security

[PATCH 5/5] sh64: ascii armor the sh64 boot init stack canary

[Rik van Riel]

Use the ascii-armor canary to prevent unterminated C string overflows
from being able to successfully overwrite the canary, even if they
somehow obtain the canary value.

Inspired by execshield ascii-armor and Daniel Micay's linux-hardened tree.

Signed-off-by: Rik van Riel <riel@redhat.com>
---
 arch/sh/include/asm/stackprotector.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/sh/include/asm/stackprotector.h b/arch/sh/include/asm/stackprotector.h
index d9df3a76847c..141515a43b78 100644
--- a/arch/sh/include/asm/stackprotector.h
+++ b/arch/sh/include/asm/stackprotector.h
@@ -19,6 +19,7 @@ static __always_inline void boot_init_stack_canary(void)
 	/* Try to get a semi random initial value. */
 	get_random_bytes(&canary, sizeof(canary));
 	canary ^= LINUX_VERSION_CODE;
+	canary &= CANARY_MASK;
 
 	current->stack_canary = canary;
 	__stack_chk_guard = current->stack_canary;

[PATCH 5/5] sh64: ascii armor the sh64 boot init stack canary

[Rik van Riel]

Use the ascii-armor canary to prevent unterminated C string overflows
from being able to successfully overwrite the canary, even if they
somehow obtain the canary value.

Inspired by execshield ascii-armor and Daniel Micay's linux-hardened tree.

Signed-off-by: Rik van Riel <riel@redhat.com>
---
 arch/sh/include/asm/stackprotector.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/sh/include/asm/stackprotector.h b/arch/sh/include/asm/stackprotector.h
index d9df3a76847c..141515a43b78 100644
--- a/arch/sh/include/asm/stackprotector.h
+++ b/arch/sh/include/asm/stackprotector.h
@@ -19,6 +19,7 @@ static __always_inline void boot_init_stack_canary(void)
 	/* Try to get a semi random initial value. */
 	get_random_bytes(&canary, sizeof(canary));
 	canary ^= LINUX_VERSION_CODE;
+	canary &= CANARY_MASK;
 
 	current->stack_canary = canary;
 	__stack_chk_guard = current->stack_canary;

[PATCH 5/5] sh64: ascii armor the sh64 boot init stack canary

[Rik van Riel]

Use the ascii-armor canary to prevent unterminated C string overflows
from being able to successfully overwrite the canary, even if they
somehow obtain the canary value.

Inspired by execshield ascii-armor and Daniel Micay's linux-hardened tree.

Signed-off-by: Rik van Riel <riel@redhat.com>
---
 arch/sh/include/asm/stackprotector.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/sh/include/asm/stackprotector.h b/arch/sh/include/asm/stackprotector.h
index d9df3a76847c..141515a43b78 100644
--- a/arch/sh/include/asm/stackprotector.h
+++ b/arch/sh/include/asm/stackprotector.h
@@ -19,6 +19,7 @@ static __always_inline void boot_init_stack_canary(void)
 	/* Try to get a semi random initial value. */
 	get_random_bytes(&canary, sizeof(canary));
 	canary ^= LINUX_VERSION_CODE;
+	canary &= CANARY_MASK;
 
 	current->stack_canary = canary;
 	__stack_chk_guard = current->stack_canary;

[kernel-hardening] [PATCH 5/5] sh64: ascii armor the sh64 boot init stack canary

[Rik van Riel]

Use the ascii-armor canary to prevent unterminated C string overflows
from being able to successfully overwrite the canary, even if they
somehow obtain the canary value.

Inspired by execshield ascii-armor and Daniel Micay's linux-hardened tree.

Signed-off-by: Rik van Riel <riel@redhat.com>
---
 arch/sh/include/asm/stackprotector.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/sh/include/asm/stackprotector.h b/arch/sh/include/asm/stackprotector.h
index d9df3a76847c..141515a43b78 100644
--- a/arch/sh/include/asm/stackprotector.h
+++ b/arch/sh/include/asm/stackprotector.h
@@ -19,6 +19,7 @@ static __always_inline void boot_init_stack_canary(void)
 	/* Try to get a semi random initial value. */
 	get_random_bytes(&canary, sizeof(canary));
 	canary ^= LINUX_VERSION_CODE;
+	canary &= CANARY_MASK;
 
 	current->stack_canary = canary;
 	__stack_chk_guard = current->stack_canary;

Re: [PATCH 5/5] sh64: ascii armor the sh64 boot init stack canary

[Kees Cook]

On Wed, May 24, 2017 at 9:34 AM, Rik van Riel <riel@redhat.com> wrote:
> Use the ascii-armor canary to prevent unterminated C string overflows
> from being able to successfully overwrite the canary, even if they
> somehow obtain the canary value.
>
> Inspired by execshield ascii-armor and Daniel Micay's linux-hardened tree.
>
> Signed-off-by: Rik van Riel <riel@redhat.com>

Acked-by: Kees Cook <keescook@chromium.org>

-Kees

> ---
>  arch/sh/include/asm/stackprotector.h | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/arch/sh/include/asm/stackprotector.h b/arch/sh/include/asm/stackprotector.h
> index d9df3a76847c..141515a43b78 100644
> --- a/arch/sh/include/asm/stackprotector.h
> +++ b/arch/sh/include/asm/stackprotector.h
> @@ -19,6 +19,7 @@ static __always_inline void boot_init_stack_canary(void)
>         /* Try to get a semi random initial value. */
>         get_random_bytes(&canary, sizeof(canary));
>         canary ^= LINUX_VERSION_CODE;
> +       canary &= CANARY_MASK;
>
>         current->stack_canary = canary;
>         __stack_chk_guard = current->stack_canary;
>



-- 
Kees Cook
Pixel Security

Re: [PATCH 5/5] sh64: ascii armor the sh64 boot init stack canary

[Kees Cook]

On Wed, May 24, 2017 at 9:34 AM, Rik van Riel <riel@redhat.com> wrote:
> Use the ascii-armor canary to prevent unterminated C string overflows
> from being able to successfully overwrite the canary, even if they
> somehow obtain the canary value.
>
> Inspired by execshield ascii-armor and Daniel Micay's linux-hardened tree.
>
> Signed-off-by: Rik van Riel <riel@redhat.com>

Acked-by: Kees Cook <keescook@chromium.org>

-Kees

> ---
>  arch/sh/include/asm/stackprotector.h | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/arch/sh/include/asm/stackprotector.h b/arch/sh/include/asm/stackprotector.h
> index d9df3a76847c..141515a43b78 100644
> --- a/arch/sh/include/asm/stackprotector.h
> +++ b/arch/sh/include/asm/stackprotector.h
> @@ -19,6 +19,7 @@ static __always_inline void boot_init_stack_canary(void)
>         /* Try to get a semi random initial value. */
>         get_random_bytes(&canary, sizeof(canary));
>         canary ^= LINUX_VERSION_CODE;
> +       canary &= CANARY_MASK;
>
>         current->stack_canary = canary;
>         __stack_chk_guard = current->stack_canary;
>



-- 
Kees Cook
Pixel Security

[PATCH 5/5] sh64: ascii armor the sh64 boot init stack canary

[Kees Cook]

On Wed, May 24, 2017 at 9:34 AM, Rik van Riel <riel@redhat.com> wrote:
> Use the ascii-armor canary to prevent unterminated C string overflows
> from being able to successfully overwrite the canary, even if they
> somehow obtain the canary value.
>
> Inspired by execshield ascii-armor and Daniel Micay's linux-hardened tree.
>
> Signed-off-by: Rik van Riel <riel@redhat.com>

Acked-by: Kees Cook <keescook@chromium.org>

-Kees

> ---
>  arch/sh/include/asm/stackprotector.h | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/arch/sh/include/asm/stackprotector.h b/arch/sh/include/asm/stackprotector.h
> index d9df3a76847c..141515a43b78 100644
> --- a/arch/sh/include/asm/stackprotector.h
> +++ b/arch/sh/include/asm/stackprotector.h
> @@ -19,6 +19,7 @@ static __always_inline void boot_init_stack_canary(void)
>         /* Try to get a semi random initial value. */
>         get_random_bytes(&canary, sizeof(canary));
>         canary ^= LINUX_VERSION_CODE;
> +       canary &= CANARY_MASK;
>
>         current->stack_canary = canary;
>         __stack_chk_guard = current->stack_canary;
>



-- 
Kees Cook
Pixel Security

[kernel-hardening] Re: [PATCH 5/5] sh64: ascii armor the sh64 boot init stack canary

[Kees Cook]

On Wed, May 24, 2017 at 9:34 AM, Rik van Riel <riel@redhat.com> wrote:
> Use the ascii-armor canary to prevent unterminated C string overflows
> from being able to successfully overwrite the canary, even if they
> somehow obtain the canary value.
>
> Inspired by execshield ascii-armor and Daniel Micay's linux-hardened tree.
>
> Signed-off-by: Rik van Riel <riel@redhat.com>

Acked-by: Kees Cook <keescook@chromium.org>

-Kees

> ---
>  arch/sh/include/asm/stackprotector.h | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/arch/sh/include/asm/stackprotector.h b/arch/sh/include/asm/stackprotector.h
> index d9df3a76847c..141515a43b78 100644
> --- a/arch/sh/include/asm/stackprotector.h
> +++ b/arch/sh/include/asm/stackprotector.h
> @@ -19,6 +19,7 @@ static __always_inline void boot_init_stack_canary(void)
>         /* Try to get a semi random initial value. */
>         get_random_bytes(&canary, sizeof(canary));
>         canary ^= LINUX_VERSION_CODE;
> +       canary &= CANARY_MASK;
>
>         current->stack_canary = canary;
>         __stack_chk_guard = current->stack_canary;
>



-- 
Kees Cook
Pixel Security

Re: [kernel-hardening] [PATCH v2 0/5] stackprotector: ascii armor the stack canary

[Solar Designer]

On Wed, May 24, 2017 at 11:57:46AM -0400, riel@redhat.com wrote:
> Zero out the first byte of the stack canary value on 64 bit systems,
> in order to mitigate unterminated C string overflows.
> 
> The null byte both prevents C string functions from reading the
> canary, and from writing it if the canary value were guessed or
> obtained through some other means.
>     
> Reducing the entropy by 8 bits is acceptable on 64-bit systems,
> which will still have 56 bits of entropy left, but not on 32
> bit systems, so the "ascii armor" canary is only implemented on
> 64-bit systems.
> 
> Inspired by the "ascii armor" code in execshield and Daniel Micay's
> linux-hardened tree.
> 
> Also see https://github.com/thestinger/linux-hardened/

Brad trolls us all lightly with this trivia question:

https://twitter.com/grsecurity/status/905246423591084033

"For #trivia can you describe one scenario where this change actually
helps exploitation using similar C string funcs?"

I suppose the expected answer is:

The change helps exploitation when the overwriting string ends just
before the canary.  Its NUL overwriting the NUL byte in the canary would
go undetected.  Before this change, there would be a 255/256 chance of
detection.

I hope this was considered.  The change might still be a good tradeoff,
or it might not, depending on which scenarios are more likely (leak of
canary value or the string required in an exploit ending at that exact
byte location), and we probably lack such statistics.

I am not proposing to revert the change.  I had actually contemplated
speaking up when this was discussed, but did not for lack of a better
suggestion.  We could put/require a NUL in the middle of the canary, but
with the full canary being only 64-bit at most that would also make some
attacks easier.

So this is JFYI.  No action needed on it, I think.

Alexander

Re: [kernel-hardening] [PATCH v2 0/5] stackprotector: ascii armor the stack canary

[Solar Designer]

On Wed, May 24, 2017 at 11:57:46AM -0400, riel@redhat.com wrote:
> Zero out the first byte of the stack canary value on 64 bit systems,
> in order to mitigate unterminated C string overflows.
> 
> The null byte both prevents C string functions from reading the
> canary, and from writing it if the canary value were guessed or
> obtained through some other means.
>     
> Reducing the entropy by 8 bits is acceptable on 64-bit systems,
> which will still have 56 bits of entropy left, but not on 32
> bit systems, so the "ascii armor" canary is only implemented on
> 64-bit systems.
> 
> Inspired by the "ascii armor" code in execshield and Daniel Micay's
> linux-hardened tree.
> 
> Also see https://github.com/thestinger/linux-hardened/

Brad trolls us all lightly with this trivia question:

https://twitter.com/grsecurity/status/905246423591084033

"For #trivia can you describe one scenario where this change actually
helps exploitation using similar C string funcs?"

I suppose the expected answer is:

The change helps exploitation when the overwriting string ends just
before the canary.  Its NUL overwriting the NUL byte in the canary would
go undetected.  Before this change, there would be a 255/256 chance of
detection.

I hope this was considered.  The change might still be a good tradeoff,
or it might not, depending on which scenarios are more likely (leak of
canary value or the string required in an exploit ending at that exact
byte location), and we probably lack such statistics.

I am not proposing to revert the change.  I had actually contemplated
speaking up when this was discussed, but did not for lack of a better
suggestion.  We could put/require a NUL in the middle of the canary, but
with the full canary being only 64-bit at most that would also make some
attacks easier.

So this is JFYI.  No action needed on it, I think.

Alexander

[kernel-hardening] [PATCH v2 0/5] stackprotector: ascii armor the stack canary

[Solar Designer]

On Wed, May 24, 2017 at 11:57:46AM -0400, riel at redhat.com wrote:
> Zero out the first byte of the stack canary value on 64 bit systems,
> in order to mitigate unterminated C string overflows.
> 
> The null byte both prevents C string functions from reading the
> canary, and from writing it if the canary value were guessed or
> obtained through some other means.
>     
> Reducing the entropy by 8 bits is acceptable on 64-bit systems,
> which will still have 56 bits of entropy left, but not on 32
> bit systems, so the "ascii armor" canary is only implemented on
> 64-bit systems.
> 
> Inspired by the "ascii armor" code in execshield and Daniel Micay's
> linux-hardened tree.
> 
> Also see https://github.com/thestinger/linux-hardened/

Brad trolls us all lightly with this trivia question:

https://twitter.com/grsecurity/status/905246423591084033

"For #trivia can you describe one scenario where this change actually
helps exploitation using similar C string funcs?"

I suppose the expected answer is:

The change helps exploitation when the overwriting string ends just
before the canary.  Its NUL overwriting the NUL byte in the canary would
go undetected.  Before this change, there would be a 255/256 chance of
detection.

I hope this was considered.  The change might still be a good tradeoff,
or it might not, depending on which scenarios are more likely (leak of
canary value or the string required in an exploit ending at that exact
byte location), and we probably lack such statistics.

I am not proposing to revert the change.  I had actually contemplated
speaking up when this was discussed, but did not for lack of a better
suggestion.  We could put/require a NUL in the middle of the canary, but
with the full canary being only 64-bit at most that would also make some
attacks easier.

So this is JFYI.  No action needed on it, I think.

Alexander

Re: [kernel-hardening] [PATCH v2 0/5] stackprotector: ascii armor the stack canary

[Kees Cook]

On Tue, Sep 19, 2017 at 10:16 AM, Solar Designer <solar@openwall.com> wrote:
> On Wed, May 24, 2017 at 11:57:46AM -0400, riel@redhat.com wrote:
>> Zero out the first byte of the stack canary value on 64 bit systems,
>> in order to mitigate unterminated C string overflows.
>>
>> The null byte both prevents C string functions from reading the
>> canary, and from writing it if the canary value were guessed or
>> obtained through some other means.
>>
>> Reducing the entropy by 8 bits is acceptable on 64-bit systems,
>> which will still have 56 bits of entropy left, but not on 32
>> bit systems, so the "ascii armor" canary is only implemented on
>> 64-bit systems.
>>
>> Inspired by the "ascii armor" code in execshield and Daniel Micay's
>> linux-hardened tree.
>>
>> Also see https://github.com/thestinger/linux-hardened/
>
> Brad trolls us all lightly with this trivia question:
>
> https://twitter.com/grsecurity/status/905246423591084033
>
> "For #trivia can you describe one scenario where this change actually
> helps exploitation using similar C string funcs?"
>
> I suppose the expected answer is:
>
> The change helps exploitation when the overwriting string ends just
> before the canary.  Its NUL overwriting the NUL byte in the canary would
> go undetected.  Before this change, there would be a 255/256 chance of
> detection.
>
> I hope this was considered.  The change might still be a good tradeoff,
> or it might not, depending on which scenarios are more likely (leak of
> canary value or the string required in an exploit ending at that exact
> byte location), and we probably lack such statistics.
>
> I am not proposing to revert the change.  I had actually contemplated
> speaking up when this was discussed, but did not for lack of a better
> suggestion.  We could put/require a NUL in the middle of the canary, but
> with the full canary being only 64-bit at most that would also make some
> attacks easier.
>
> So this is JFYI.  No action needed on it, I think.

Agreed. And prior to this:
https://git.kernel.org/linus/5ea30e4e58040cfd6434c2f33dc3ea76e2c15b05
the same kernels had 4 NULL bytes in a row. ;) So it's all an improvement, IMO.

-Kees

-- 
Kees Cook
Pixel Security

Re: [kernel-hardening] [PATCH v2 0/5] stackprotector: ascii armor the stack canary

[Kees Cook]

On Tue, Sep 19, 2017 at 10:16 AM, Solar Designer <solar@openwall.com> wrote:
> On Wed, May 24, 2017 at 11:57:46AM -0400, riel@redhat.com wrote:
>> Zero out the first byte of the stack canary value on 64 bit systems,
>> in order to mitigate unterminated C string overflows.
>>
>> The null byte both prevents C string functions from reading the
>> canary, and from writing it if the canary value were guessed or
>> obtained through some other means.
>>
>> Reducing the entropy by 8 bits is acceptable on 64-bit systems,
>> which will still have 56 bits of entropy left, but not on 32
>> bit systems, so the "ascii armor" canary is only implemented on
>> 64-bit systems.
>>
>> Inspired by the "ascii armor" code in execshield and Daniel Micay's
>> linux-hardened tree.
>>
>> Also see https://github.com/thestinger/linux-hardened/
>
> Brad trolls us all lightly with this trivia question:
>
> https://twitter.com/grsecurity/status/905246423591084033
>
> "For #trivia can you describe one scenario where this change actually
> helps exploitation using similar C string funcs?"
>
> I suppose the expected answer is:
>
> The change helps exploitation when the overwriting string ends just
> before the canary.  Its NUL overwriting the NUL byte in the canary would
> go undetected.  Before this change, there would be a 255/256 chance of
> detection.
>
> I hope this was considered.  The change might still be a good tradeoff,
> or it might not, depending on which scenarios are more likely (leak of
> canary value or the string required in an exploit ending at that exact
> byte location), and we probably lack such statistics.
>
> I am not proposing to revert the change.  I had actually contemplated
> speaking up when this was discussed, but did not for lack of a better
> suggestion.  We could put/require a NUL in the middle of the canary, but
> with the full canary being only 64-bit at most that would also make some
> attacks easier.
>
> So this is JFYI.  No action needed on it, I think.

Agreed. And prior to this:
https://git.kernel.org/linus/5ea30e4e58040cfd6434c2f33dc3ea76e2c15b05
the same kernels had 4 NULL bytes in a row. ;) So it's all an improvement, IMO.

-Kees

-- 
Kees Cook
Pixel Security

[kernel-hardening] [PATCH v2 0/5] stackprotector: ascii armor the stack canary

[Kees Cook]

On Tue, Sep 19, 2017 at 10:16 AM, Solar Designer <solar@openwall.com> wrote:
> On Wed, May 24, 2017 at 11:57:46AM -0400, riel at redhat.com wrote:
>> Zero out the first byte of the stack canary value on 64 bit systems,
>> in order to mitigate unterminated C string overflows.
>>
>> The null byte both prevents C string functions from reading the
>> canary, and from writing it if the canary value were guessed or
>> obtained through some other means.
>>
>> Reducing the entropy by 8 bits is acceptable on 64-bit systems,
>> which will still have 56 bits of entropy left, but not on 32
>> bit systems, so the "ascii armor" canary is only implemented on
>> 64-bit systems.
>>
>> Inspired by the "ascii armor" code in execshield and Daniel Micay's
>> linux-hardened tree.
>>
>> Also see https://github.com/thestinger/linux-hardened/
>
> Brad trolls us all lightly with this trivia question:
>
> https://twitter.com/grsecurity/status/905246423591084033
>
> "For #trivia can you describe one scenario where this change actually
> helps exploitation using similar C string funcs?"
>
> I suppose the expected answer is:
>
> The change helps exploitation when the overwriting string ends just
> before the canary.  Its NUL overwriting the NUL byte in the canary would
> go undetected.  Before this change, there would be a 255/256 chance of
> detection.
>
> I hope this was considered.  The change might still be a good tradeoff,
> or it might not, depending on which scenarios are more likely (leak of
> canary value or the string required in an exploit ending at that exact
> byte location), and we probably lack such statistics.
>
> I am not proposing to revert the change.  I had actually contemplated
> speaking up when this was discussed, but did not for lack of a better
> suggestion.  We could put/require a NUL in the middle of the canary, but
> with the full canary being only 64-bit at most that would also make some
> attacks easier.
>
> So this is JFYI.  No action needed on it, I think.

Agreed. And prior to this:
https://git.kernel.org/linus/5ea30e4e58040cfd6434c2f33dc3ea76e2c15b05
the same kernels had 4 NULL bytes in a row. ;) So it's all an improvement, IMO.

-Kees

-- 
Kees Cook
Pixel Security

Re: [kernel-hardening] [PATCH v2 0/5] stackprotector: ascii armor the stack canary

[Kees Cook]

On Tue, Sep 19, 2017 at 10:16 AM, Solar Designer <solar@openwall.com> wrote:
> On Wed, May 24, 2017 at 11:57:46AM -0400, riel@redhat.com wrote:
>> Zero out the first byte of the stack canary value on 64 bit systems,
>> in order to mitigate unterminated C string overflows.
>>
>> The null byte both prevents C string functions from reading the
>> canary, and from writing it if the canary value were guessed or
>> obtained through some other means.
>>
>> Reducing the entropy by 8 bits is acceptable on 64-bit systems,
>> which will still have 56 bits of entropy left, but not on 32
>> bit systems, so the "ascii armor" canary is only implemented on
>> 64-bit systems.
>>
>> Inspired by the "ascii armor" code in execshield and Daniel Micay's
>> linux-hardened tree.
>>
>> Also see https://github.com/thestinger/linux-hardened/
>
> Brad trolls us all lightly with this trivia question:
>
> https://twitter.com/grsecurity/status/905246423591084033
>
> "For #trivia can you describe one scenario where this change actually
> helps exploitation using similar C string funcs?"
>
> I suppose the expected answer is:
>
> The change helps exploitation when the overwriting string ends just
> before the canary.  Its NUL overwriting the NUL byte in the canary would
> go undetected.  Before this change, there would be a 255/256 chance of
> detection.
>
> I hope this was considered.  The change might still be a good tradeoff,
> or it might not, depending on which scenarios are more likely (leak of
> canary value or the string required in an exploit ending at that exact
> byte location), and we probably lack such statistics.
>
> I am not proposing to revert the change.  I had actually contemplated
> speaking up when this was discussed, but did not for lack of a better
> suggestion.  We could put/require a NUL in the middle of the canary, but
> with the full canary being only 64-bit at most that would also make some
> attacks easier.
>
> So this is JFYI.  No action needed on it, I think.

Agreed. And prior to this:
https://git.kernel.org/linus/5ea30e4e58040cfd6434c2f33dc3ea76e2c15b05
the same kernels had 4 NULL bytes in a row. ;) So it's all an improvement, IMO.

-Kees

-- 
Kees Cook
Pixel Security

Re: [kernel-hardening] [PATCH v2 0/5] stackprotector: ascii armor the stack canary

[Daniel Micay]

> Brad trolls us all lightly with this trivia question:
> 
> https://twitter.com/grsecurity/status/905246423591084033

I'll respond to your proposed scenario rather than guessing at what is
being suggested there and if it's actually the same thing as what you've
brought up.

They've stated many times that stack canaries are near useless and
enabling PaX UDEREF on i386 actually disabled stack canaries and
presumably still does after the last public patches:

+	select HAVE_CC_STACKPROTECTOR		if X86_64 || !PAX_MEMORY_UDEREF

Making the kernel more vulnerable to remote stack overflow exploits like
this month's overhyped Bluetooth bug to improve security against local
exploits seems like much more of a compromise. Not that i386 has any
real relevance anymore, but I think that's some interesting context...

It's not like upstream has a monopoly on needing to make hard choices
for security features, I don't see this as one of those hard choices
like whether sacrificing availability for SIZE_OVERFLOW makes sense.

> "For #trivia can you describe one scenario where this change actually
> helps exploitation using similar C string funcs?"
> 
> I suppose the expected answer is:
> 
> The change helps exploitation when the overwriting string ends just
> before the canary.  Its NUL overwriting the NUL byte in the canary
> would
> go undetected.  Before this change, there would be a 255/256 chance of
> detection.
>
> I hope this was considered.  The change might still be a good
> tradeoff,
> or it might not, depending on which scenarios are more likely (leak of
> canary value or the string required in an exploit ending at that exact
> byte location), and we probably lack such statistics.

It was considered that guaranteeing some forms of string overflows are
contained within the frame can have a negative impact but I don't think
it's significant.

There would need to be something worth overwriting between the source of
the overflow and the canary. Since SSP reorders local variables within
the stack frame based on risk of overflow, the targets would be locals
that Clang/GCC considers to have a higher risk of overflow. An array can
only have other arrays between it and the canary and there's ordering by
size (small vs. large at least) within the array section. If there was a
zero byte in any of that other data between the source of the overflow
and the canary, this wouldn't make a difference anyway.

Since the canary is only checked on return, it's also only relevant if
control flow still makes it to the function epilogue and it still
matters after the attacker has accomplished their goal.

The chance of there being a zero as a leading byte is already 1/256, but
there could also be a NUL elsewhere in the canary compromising a subset
of the entropy for this kind of scenario too so it's a tiny bit higher
than a 1/256 chance already. Not much resistance to brute force if it
really does ever end up mattering, unless canaries with NUL in leading
positions were actually *rejected*...

There's a more substantial compromise between zero and non-zero poison
on free but for production use. This was part of a set of changes where
CopperheadOS switched from non-zero fill -> zero fill on free for the
kernel and userspace and added a leading zero byte on 64-bit for heap
and stack canaries in the production builds while keeping around the old
way of doing things for some forms of debug builds. Non-zero fill on
free ended up causing too much harm by turning benign bugs into bugs
that might have been exploitable even beyond DoS in some cases tied to
non-NUL-terminated strings where non-zero poisoning got rid of zeroes
that were otherwise all over the heap containing them or when it broke
code wrongly depending on uninitialized data being zero in practice. I
think there's more of an argument to be had about poison-on-free as a
mitigation. This leading zero byte? Not so much, beyond perhaps wanting
the option to turn it off for bug finding... but there are better tools
for that now.

> I am not proposing to revert the change.  I had actually contemplated
> speaking up when this was discussed, but did not for lack of a better
> suggestion.  We could put/require a NUL in the middle of the canary,
> but
> with the full canary being only 64-bit at most that would also make
> some
> attacks easier.
> 
> So this is JFYI.  No action needed on it, I think.

It might make sense to put it in the middle, but a scenario where it
matters seems unlikely and as you mention it would make it weaker it
some other cases. I think it's already the best choice right now.

I'd choose XOR canaries over a leading zero byte if it could only be one
or the other, but I'm not really convinced that there's any significant
loss compared to the normal canaries.

Re: [kernel-hardening] [PATCH v2 0/5] stackprotector: ascii armor the stack canary

[Daniel Micay]

> Brad trolls us all lightly with this trivia question:
> 
> https://twitter.com/grsecurity/status/905246423591084033

I'll respond to your proposed scenario rather than guessing at what is
being suggested there and if it's actually the same thing as what you've
brought up.

They've stated many times that stack canaries are near useless and
enabling PaX UDEREF on i386 actually disabled stack canaries and
presumably still does after the last public patches:

+	select HAVE_CC_STACKPROTECTOR		if X86_64 || !PAX_MEMORY_UDEREF

Making the kernel more vulnerable to remote stack overflow exploits like
this month's overhyped Bluetooth bug to improve security against local
exploits seems like much more of a compromise. Not that i386 has any
real relevance anymore, but I think that's some interesting context...

It's not like upstream has a monopoly on needing to make hard choices
for security features, I don't see this as one of those hard choices
like whether sacrificing availability for SIZE_OVERFLOW makes sense.

> "For #trivia can you describe one scenario where this change actually
> helps exploitation using similar C string funcs?"
> 
> I suppose the expected answer is:
> 
> The change helps exploitation when the overwriting string ends just
> before the canary.  Its NUL overwriting the NUL byte in the canary
> would
> go undetected.  Before this change, there would be a 255/256 chance of
> detection.
>
> I hope this was considered.  The change might still be a good
> tradeoff,
> or it might not, depending on which scenarios are more likely (leak of
> canary value or the string required in an exploit ending at that exact
> byte location), and we probably lack such statistics.

It was considered that guaranteeing some forms of string overflows are
contained within the frame can have a negative impact but I don't think
it's significant.

There would need to be something worth overwriting between the source of
the overflow and the canary. Since SSP reorders local variables within
the stack frame based on risk of overflow, the targets would be locals
that Clang/GCC considers to have a higher risk of overflow. An array can
only have other arrays between it and the canary and there's ordering by
size (small vs. large at least) within the array section. If there was a
zero byte in any of that other data between the source of the overflow
and the canary, this wouldn't make a difference anyway.

Since the canary is only checked on return, it's also only relevant if
control flow still makes it to the function epilogue and it still
matters after the attacker has accomplished their goal.

The chance of there being a zero as a leading byte is already 1/256, but
there could also be a NUL elsewhere in the canary compromising a subset
of the entropy for this kind of scenario too so it's a tiny bit higher
than a 1/256 chance already. Not much resistance to brute force if it
really does ever end up mattering, unless canaries with NUL in leading
positions were actually *rejected*...

There's a more substantial compromise between zero and non-zero poison
on free but for production use. This was part of a set of changes where
CopperheadOS switched from non-zero fill -> zero fill on free for the
kernel and userspace and added a leading zero byte on 64-bit for heap
and stack canaries in the production builds while keeping around the old
way of doing things for some forms of debug builds. Non-zero fill on
free ended up causing too much harm by turning benign bugs into bugs
that might have been exploitable even beyond DoS in some cases tied to
non-NUL-terminated strings where non-zero poisoning got rid of zeroes
that were otherwise all over the heap containing them or when it broke
code wrongly depending on uninitialized data being zero in practice. I
think there's more of an argument to be had about poison-on-free as a
mitigation. This leading zero byte? Not so much, beyond perhaps wanting
the option to turn it off for bug finding... but there are better tools
for that now.

> I am not proposing to revert the change.  I had actually contemplated
> speaking up when this was discussed, but did not for lack of a better
> suggestion.  We could put/require a NUL in the middle of the canary,
> but
> with the full canary being only 64-bit at most that would also make
> some
> attacks easier.
> 
> So this is JFYI.  No action needed on it, I think.

It might make sense to put it in the middle, but a scenario where it
matters seems unlikely and as you mention it would make it weaker it
some other cases. I think it's already the best choice right now.

I'd choose XOR canaries over a leading zero byte if it could only be one
or the other, but I'm not really convinced that there's any significant
loss compared to the normal canaries.

[kernel-hardening] [PATCH v2 0/5] stackprotector: ascii armor the stack canary

[Daniel Micay]

> Brad trolls us all lightly with this trivia question:
> 
> https://twitter.com/grsecurity/status/905246423591084033

I'll respond to your proposed scenario rather than guessing at what is
being suggested there and if it's actually the same thing as what you've
brought up.

They've stated many times that stack canaries are near useless and
enabling PaX UDEREF on i386 actually disabled stack canaries and
presumably still does after the last public patches:

+	select HAVE_CC_STACKPROTECTOR		if X86_64 || !PAX_MEMORY_UDEREF

Making the kernel more vulnerable to remote stack overflow exploits like
this month's overhyped Bluetooth bug to improve security against local
exploits seems like much more of a compromise. Not that i386 has any
real relevance anymore, but I think that's some interesting context...

It's not like upstream has a monopoly on needing to make hard choices
for security features, I don't see this as one of those hard choices
like whether sacrificing availability for SIZE_OVERFLOW makes sense.

> "For #trivia can you describe one scenario where this change actually
> helps exploitation using similar C string funcs?"
> 
> I suppose the expected answer is:
> 
> The change helps exploitation when the overwriting string ends just
> before the canary.  Its NUL overwriting the NUL byte in the canary
> would
> go undetected.  Before this change, there would be a 255/256 chance of
> detection.
>
> I hope this was considered.  The change might still be a good
> tradeoff,
> or it might not, depending on which scenarios are more likely (leak of
> canary value or the string required in an exploit ending at that exact
> byte location), and we probably lack such statistics.

It was considered that guaranteeing some forms of string overflows are
contained within the frame can have a negative impact but I don't think
it's significant.

There would need to be something worth overwriting between the source of
the overflow and the canary. Since SSP reorders local variables within
the stack frame based on risk of overflow, the targets would be locals
that Clang/GCC considers to have a higher risk of overflow. An array can
only have other arrays between it and the canary and there's ordering by
size (small vs. large at least) within the array section. If there was a
zero byte in any of that other data between the source of the overflow
and the canary, this wouldn't make a difference anyway.

Since the canary is only checked on return, it's also only relevant if
control flow still makes it to the function epilogue and it still
matters after the attacker has accomplished their goal.

The chance of there being a zero as a leading byte is already 1/256, but
there could also be a NUL elsewhere in the canary compromising a subset
of the entropy for this kind of scenario too so it's a tiny bit higher
than a 1/256 chance already. Not much resistance to brute force if it
really does ever end up mattering, unless canaries with NUL in leading
positions were actually *rejected*...

There's a more substantial compromise between zero and non-zero poison
on free but for production use. This was part of a set of changes where
CopperheadOS switched from non-zero fill -> zero fill on free for the
kernel and userspace and added a leading zero byte on 64-bit for heap
and stack canaries in the production builds while keeping around the old
way of doing things for some forms of debug builds. Non-zero fill on
free ended up causing too much harm by turning benign bugs into bugs
that might have been exploitable even beyond DoS in some cases tied to
non-NUL-terminated strings where non-zero poisoning got rid of zeroes
that were otherwise all over the heap containing them or when it broke
code wrongly depending on uninitialized data being zero in practice. I
think there's more of an argument to be had about poison-on-free as a
mitigation. This leading zero byte? Not so much, beyond perhaps wanting
the option to turn it off for bug finding... but there are better tools
for that now.

> I am not proposing to revert the change.  I had actually contemplated
> speaking up when this was discussed, but did not for lack of a better
> suggestion.  We could put/require a NUL in the middle of the canary,
> but
> with the full canary being only 64-bit at most that would also make
> some
> attacks easier.
> 
> So this is JFYI.  No action needed on it, I think.

It might make sense to put it in the middle, but a scenario where it
matters seems unlikely and as you mention it would make it weaker it
some other cases. I think it's already the best choice right now.

I'd choose XOR canaries over a leading zero byte if it could only be one
or the other, but I'm not really convinced that there's any significant
loss compared to the normal canaries.

Re: [kernel-hardening] [PATCH v2 0/5] stackprotector: ascii armor the stack canary

[Yann Droneaud]

Hi,

Le mardi 19 septembre 2017 à 19:16 +0200, Solar Designer a écrit :
>
> We could put/require a NUL in the middle of the canary,
> but with the full canary being only 64-bit at most that would also
> make some attacks easier.
> 

Are you suggesting to randomly select which byte to set to 0 in each
canary ?

Regards.

-- 
Yann Droneaud
OPTEYA

Re: [kernel-hardening] [PATCH v2 0/5] stackprotector: ascii armor the stack canary

[Yann Droneaud]

Hi,

Le mardi 19 septembre 2017 à 19:16 +0200, Solar Designer a écrit :
>
> We could put/require a NUL in the middle of the canary,
> but with the full canary being only 64-bit at most that would also
> make some attacks easier.
> 

Are you suggesting to randomly select which byte to set to 0 in each
canary ?

Regards.

-- 
Yann Droneaud
OPTEYA

[kernel-hardening] [PATCH v2 0/5] stackprotector: ascii armor the stack canary

[Yann Droneaud]

Hi,

Le mardi 19 septembre 2017 ? 19:16 +0200, Solar Designer a ?crit :
>
> We could put/require a NUL in the middle of the canary,
> but with the full canary being only 64-bit at most that would also
> make some attacks easier.
> 

Are you suggesting to randomly select which byte to set to 0 in each
canary ?

Regards.

-- 
Yann Droneaud
OPTEYA

Re: [kernel-hardening] [PATCH v2 0/5] stackprotector: ascii armor the stack canary

[Solar Designer]

On Wed, Sep 20, 2017 at 01:18:04PM +0200, Yann Droneaud wrote:
> Le mardi 19 septembre 2017 ?? 19:16 +0200, Solar Designer a ??crit :
> >
> > We could put/require a NUL in the middle of the canary,
> > but with the full canary being only 64-bit at most that would also
> > make some attacks easier.
> 
> Are you suggesting to randomly select which byte to set to 0 in each
> canary ?

Definitely not.  That's only 8 different possibilities per canary, and
the weakest one will affect exploitability in each scenario.  So that
would be a fairly clear change to the worse.

I suggest that we make no further changes at this time, unless someone
comes up with an idea that would clearly hurt exploitation more than it
helps exploitation, overall across different scenarios.

Alexander

Re: [kernel-hardening] [PATCH v2 0/5] stackprotector: ascii armor the stack canary

[Solar Designer]

On Wed, Sep 20, 2017 at 01:18:04PM +0200, Yann Droneaud wrote:
> Le mardi 19 septembre 2017 ?? 19:16 +0200, Solar Designer a ??crit :
> >
> > We could put/require a NUL in the middle of the canary,
> > but with the full canary being only 64-bit at most that would also
> > make some attacks easier.
> 
> Are you suggesting to randomly select which byte to set to 0 in each
> canary ?

Definitely not.  That's only 8 different possibilities per canary, and
the weakest one will affect exploitability in each scenario.  So that
would be a fairly clear change to the worse.

I suggest that we make no further changes at this time, unless someone
comes up with an idea that would clearly hurt exploitation more than it
helps exploitation, overall across different scenarios.

Alexander

[kernel-hardening] [PATCH v2 0/5] stackprotector: ascii armor the stack canary

[Solar Designer]

On Wed, Sep 20, 2017 at 01:18:04PM +0200, Yann Droneaud wrote:
> Le mardi 19 septembre 2017 ?? 19:16 +0200, Solar Designer a ??crit :
> >
> > We could put/require a NUL in the middle of the canary,
> > but with the full canary being only 64-bit at most that would also
> > make some attacks easier.
> 
> Are you suggesting to randomly select which byte to set to 0 in each
> canary ?

Definitely not.  That's only 8 different possibilities per canary, and
the weakest one will affect exploitability in each scenario.  So that
would be a fairly clear change to the worse.

I suggest that we make no further changes at this time, unless someone
comes up with an idea that would clearly hurt exploitation more than it
helps exploitation, overall across different scenarios.

Alexander