Re: [PATCH v3 2/2] riscv: Enable per-task stack canaries

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 2020/7/15 上午5:37, Palmer Dabbelt wrote:
On Fri, 10 Jul 2020 09:19:58 PDT (-0700), guoren@xxxxxxxxxx wrote:
From: Guo Ren <guoren@xxxxxxxxxxxxxxxxx>

This enables the use of per-task stack canary values if GCC has
support for emitting the stack canary reference relative to the
value of tp, which holds the task struct pointer in the riscv
kernel.

After compare arm64 and x86 implementations, seems arm64's is more
flexible and readable. The key point is how gcc get the offset of
stack_canary from gs/el0_sp.

x86: Use a fix offset from gs, not flexible.

struct fixed_percpu_data {
    /*
     * GCC hardcodes the stack canary as %gs:40.  Since the
     * irq_stack is the object at %gs:0, we reserve the bottom
     * 48 bytes of the irq stack for the canary.
     */
    char            gs_base[40]; // :(
    unsigned long   stack_canary;
};

arm64: Use -mstack-protector-guard-offset & guard-reg
    gcc options:
    -mstack-protector-guard=sysreg
    -mstack-protector-guard-reg=sp_el0
    -mstack-protector-guard-offset=xxx

riscv: Use -mstack-protector-guard-offset & guard-reg
    gcc options:
    -mstack-protector-guard=tls
    -mstack-protector-guard-reg=tp
    -mstack-protector-guard-offset=xxx

Here is riscv gcc's work [1].

[1] https://gcc.gnu.org/pipermail/gcc-patches/2020-July/549583.html

In the end, these codes are inserted by gcc before return:

*  0xffffffe00020b396 <+120>:   ld      a5,1008(tp) # 0x3f0
*  0xffffffe00020b39a <+124>:   xor     a5,a5,a4
*  0xffffffe00020b39c <+126>:   mv      a0,s5
*  0xffffffe00020b39e <+128>:   bnez a5,0xffffffe00020b61c <_do_fork+766>
   0xffffffe00020b3a2 <+132>:   ld      ra,136(sp)
   0xffffffe00020b3a4 <+134>:   ld      s0,128(sp)
   0xffffffe00020b3a6 <+136>:   ld      s1,120(sp)
   0xffffffe00020b3a8 <+138>:   ld      s2,112(sp)
   0xffffffe00020b3aa <+140>:   ld      s3,104(sp)
   0xffffffe00020b3ac <+142>:   ld      s4,96(sp)
   0xffffffe00020b3ae <+144>:   ld      s5,88(sp)
   0xffffffe00020b3b0 <+146>:   ld      s6,80(sp)
   0xffffffe00020b3b2 <+148>:   ld      s7,72(sp)
   0xffffffe00020b3b4 <+150>:   addi    sp,sp,144
   0xffffffe00020b3b6 <+152>:   ret
   ...
*  0xffffffe00020b61c <+766>:   auipc   ra,0x7f8
*  0xffffffe00020b620 <+770>:   jalr    -1764(ra) # 0xffffffe000a02f38 <__stack_chk_fail>

Signed-off-by: Guo Ren <guoren@xxxxxxxxxxxxxxxxx>
Signed-off-by: cooper <cooper.qu@xxxxxxxxxxxxxxxxx>

IIRC we're required to use full names here.  I'm assuming that's meant to be "Signed-off-by: Cooper Qu ...", and I know it's a bit procedural but I can't
make that change.

Otherwise these two look good, the first one is on for-next.  I can boot with a
defconfig ammended with CONFIG_STACKPROTECTOR=y,
Thanks!

Hi Palmer,

That's ok to change it to full names as follows.

Signed-off-by: Cooper Qu <cooper.qu@xxxxxxxxxxxxxxxxx>


Best Regards,

Cooper




[Index of Archives]     [Linux Samsung SoC]     [Linux Rockchip SoC]     [Linux Actions SoC]     [Linux for Synopsys ARC Processors]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]


  Powered by Linux