[PATCH v2] sched: move stack_canary to the start of the randomizable region

Ruidong Tian posted 1 patch 3 weeks ago
include/linux/sched.h | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
[PATCH v2] sched: move stack_canary to the start of the randomizable region
Posted by Ruidong Tian 3 weeks ago
task_struct keeps growing over time.  On architectures that compute the
per-task stack canary offset from asm-offsets.h and pass it to the
compiler via -mstack-protector-guard-offset=, this growth eventually
pushes stack_canary beyond what the target ISA can encode.

On RISC-V, canary loads are emitted as

    ld	t0, TSK_STACK_CANARY(tp)

where TSK_STACK_CANARY must fit into a 12-bit signed immediate, i.e.
[-2048, 2047].  Once stack_canary sits past byte 2047 of task_struct,
the build fails with

    cc1: error: '<N>' is not a valid offset in
        '-mstack-protector-guard-offset='

  * On RISC-V, CONFIG_STACKPROTECTOR_PER_TASK depends on !RANDSTRUCT,
    so randomized_struct_fields_start/end always expand to nothing and
    stack_canary lands at a small, stable offset well within the
    12-bit signed immediate range.  The build error goes away.

  * On architectures that enable RANDSTRUCT for hardening, stack_canary
    stays inside the randomized region and is still shuffled together
    with the other fields by the layout randomization, so its hardening
    coverage is preserved.  asm-offsets-based architectures read the
    shuffled offset at build time, so the generated canary accesses
    remain correct.

pahole on a typical 64-bit config shows that the area around the
wakee_* fields already contains a usable hole:

    struct __call_single_node  wake_entry;           /*    56    16 */
    /* --- cacheline 1 boundary (64 bytes) ---       */
    unsigned int               wakee_flips;          /*    72     4 */
    /* XXX 4 bytes hole, try to pack */
    unsigned long int          wakee_flip_decay_ts;  /*    80     8 */
    struct task_struct *       last_wakee;           /*    88     8 */

Move wakee_flips to sit after last_wakee.  That opens up a clean
8-byte slot at offset 72 into which stack_canary fits exactly:

    struct __call_single_node  wake_entry;           /*    56    16 */
    /* --- cacheline 1 boundary (64 bytes) ---       */
    unsigned long              stack_canary;         /*    72     8 */
    unsigned long int          wakee_flip_decay_ts;  /*    80     8 */
    struct task_struct *       last_wakee;           /*    88     8 */
    unsigned int               wakee_flips;          /*    96     4 */

Suggested-by: K Prateek Nayak <kprateek.nayak@amd.com>
Signed-off-by: Ruidong Tian <tianruidong@linux.alibaba.com>
---
 include/linux/sched.h | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/include/linux/sched.h b/include/linux/sched.h
index 368c7b4d7cb5..5064c4afd19c 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -848,9 +848,15 @@ struct task_struct {
 
 	int				on_cpu;
 	struct __call_single_node	wake_entry;
-	unsigned int			wakee_flips;
+
+#ifdef CONFIG_STACKPROTECTOR
+	/* Canary value for the -fstack-protector GCC feature: */
+	unsigned long			stack_canary;
+#endif
+
 	unsigned long			wakee_flip_decay_ts;
 	struct task_struct		*last_wakee;
+	unsigned int			wakee_flips;
 
 	/*
 	 * recent_used_cpu is initially set as the last CPU used by a task
@@ -1060,10 +1066,6 @@ struct task_struct {
 	pid_t				pid;
 	pid_t				tgid;
 
-#ifdef CONFIG_STACKPROTECTOR
-	/* Canary value for the -fstack-protector GCC feature: */
-	unsigned long			stack_canary;
-#endif
 	/*
 	 * Pointers to the (original) parent process, youngest child, younger sibling,
 	 * older sibling, respectively.  (p->father can be replaced with
-- 
2.51.2.612.gdc70283dfc
Re: [PATCH v2] sched: move stack_canary to the start of the randomizable region
Posted by Valentin Schneider 2 weeks, 3 days ago
On 09/05/26 11:50, Ruidong Tian wrote:
> task_struct keeps growing over time.  On architectures that compute the
> per-task stack canary offset from asm-offsets.h and pass it to the
> compiler via -mstack-protector-guard-offset=, this growth eventually
> pushes stack_canary beyond what the target ISA can encode.
>
> On RISC-V, canary loads are emitted as
>
>     ld	t0, TSK_STACK_CANARY(tp)
>
> where TSK_STACK_CANARY must fit into a 12-bit signed immediate, i.e.
> [-2048, 2047].  Once stack_canary sits past byte 2047 of task_struct,
> the build fails with
>
>     cc1: error: '<N>' is not a valid offset in
>         '-mstack-protector-guard-offset='
>
>   * On RISC-V, CONFIG_STACKPROTECTOR_PER_TASK depends on !RANDSTRUCT,
>     so randomized_struct_fields_start/end always expand to nothing and
>     stack_canary lands at a small, stable offset well within the
>     12-bit signed immediate range.  The build error goes away.
>
>   * On architectures that enable RANDSTRUCT for hardening, stack_canary
>     stays inside the randomized region and is still shuffled together
>     with the other fields by the layout randomization, so its hardening
>     coverage is preserved.  asm-offsets-based architectures read the
>     shuffled offset at build time, so the generated canary accesses
>     remain correct.
>
> pahole on a typical 64-bit config shows that the area around the
> wakee_* fields already contains a usable hole:
>
>     struct __call_single_node  wake_entry;           /*    56    16 */
>     /* --- cacheline 1 boundary (64 bytes) ---       */
>     unsigned int               wakee_flips;          /*    72     4 */
>     /* XXX 4 bytes hole, try to pack */
>     unsigned long int          wakee_flip_decay_ts;  /*    80     8 */
>     struct task_struct *       last_wakee;           /*    88     8 */
>
> Move wakee_flips to sit after last_wakee.  That opens up a clean
> 8-byte slot at offset 72 into which stack_canary fits exactly:
>
>     struct __call_single_node  wake_entry;           /*    56    16 */
>     /* --- cacheline 1 boundary (64 bytes) ---       */
>     unsigned long              stack_canary;         /*    72     8 */
>     unsigned long int          wakee_flip_decay_ts;  /*    80     8 */
>     struct task_struct *       last_wakee;           /*    88     8 */
>     unsigned int               wakee_flips;          /*    96     4 */
>
> Suggested-by: K Prateek Nayak <kprateek.nayak@amd.com>
> Signed-off-by: Ruidong Tian <tianruidong@linux.alibaba.com>

Reviewed-by: Valentin Schneider <vschneid@redhat.com>