After commit a0f7085f6a63 ("LoongArch: Add RANDOMIZE_KSTACK_OFFSET
support"), there is a new instruction "sub.d $sp, $sp, $t0" for the
secondary stack in do_syscall(), then there exists a objtool warning
"do_syscall+0x11c: return with modified stack frame" and there is no
handle_syscall() which is the previous frame of do_syscall() in the
call trace when executing the command "echo l > /proc/sysrq-trigger".
objdump shows something like this:
0000000000000000 <do_syscall>:
0: 02ff8063 addi.d $sp, $sp, -32
4: 29c04076 st.d $fp, $sp, 16
8: 29c02077 st.d $s0, $sp, 8
c: 29c06061 st.d $ra, $sp, 24
10: 02c08076 addi.d $fp, $sp, 32
...
74: 0011b063 sub.d $sp, $sp, $t0
...
a8: 4c000181 jirl $ra, $t0, 0
...
dc: 02ff82c3 addi.d $sp, $fp, -32
e0: 28c06061 ld.d $ra, $sp, 24
e4: 28c04076 ld.d $fp, $sp, 16
e8: 28c02077 ld.d $s0, $sp, 8
ec: 02c08063 addi.d $sp, $sp, 32
f0: 4c000020 jirl $zero, $ra, 0
The instruction "sub.d $sp, $sp, $t0" changes the stack bottom
and the new stack size is a random value, in order to find the
return address of do_syscall() which is stored in the original
stack frame after executing "jirl $ra, $t0, 0", it should use
fp which points to the original stack top.
This is a rare case, add a member "secondary_stack" in the struct
symbol as a label to avoid affecting the current normal case, then
set it as true only if there exists the secondary stack instruction
"sub.d $sp, $sp, $t0", at last check this label for the other special
secondary stack instructions about fp to change the cfa base and cfa
offset during the period of secondary stack in update_cfi_state().
Tested with the following two configs:
(1) CONFIG_RANDOMIZE_KSTACK_OFFSET=y &&
CONFIG_RANDOMIZE_KSTACK_OFFSET_DEFAULT=n
(2) CONFIG_RANDOMIZE_KSTACK_OFFSET=y &&
CONFIG_RANDOMIZE_KSTACK_OFFSET_DEFAULT=y
Cc: stable@vger.kernel.org # 6.9+
Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
---
tools/objtool/arch/loongarch/decode.c | 8 +++++++-
tools/objtool/check.c | 22 ++++++++++++++++++++++
tools/objtool/include/objtool/elf.h | 1 +
3 files changed, 30 insertions(+), 1 deletion(-)
diff --git a/tools/objtool/arch/loongarch/decode.c b/tools/objtool/arch/loongarch/decode.c
index db4dd05cdb49..4085714ffd18 100644
--- a/tools/objtool/arch/loongarch/decode.c
+++ b/tools/objtool/arch/loongarch/decode.c
@@ -122,7 +122,7 @@ static bool decode_insn_reg2i12_fomat(union loongarch_instruction inst,
switch (inst.reg2i12_format.opcode) {
case addid_op:
if ((inst.reg2i12_format.rd == CFI_SP) || (inst.reg2i12_format.rj == CFI_SP)) {
- /* addi.d sp,sp,si12 or addi.d fp,sp,si12 */
+ /* addi.d sp,sp,si12 or addi.d fp,sp,si12 or addi.d sp,fp,si12 */
insn->immediate = sign_extend64(inst.reg2i12_format.immediate, 11);
ADD_OP(op) {
op->src.type = OP_SRC_ADD;
@@ -275,6 +275,8 @@ static bool decode_insn_reg2i16_fomat(union loongarch_instruction inst,
static bool decode_insn_reg3_fomat(union loongarch_instruction inst,
struct instruction *insn)
{
+ struct symbol *func;
+
switch (inst.reg3_format.opcode) {
case subd_op:
if ((inst.reg3_format.rd == CFI_SP) && (inst.reg3_format.rj == CFI_SP)) {
@@ -282,6 +284,10 @@ static bool decode_insn_reg3_fomat(union loongarch_instruction inst,
* sub.d sp,sp,t0
* this is a rare case for the secondary stack.
*/
+ func = find_func_containing(insn->sec, insn->offset);
+ if (!func)
+ return false;
+ func->secondary_stack = true;
}
break;
default:
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index 01237d167223..c7b9942fee29 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -2993,6 +2993,28 @@ static int update_cfi_state(struct instruction *insn,
break;
}
+ if (op->dest.reg == CFI_BP && op->src.reg == CFI_SP) {
+ /* addi.d fp,sp,imm for the secondary stack on LoongArch */
+ if (cfa->base == CFI_SP && cfa->offset == op->src.offset) {
+ if (insn->sym->secondary_stack) {
+ cfa->base = CFI_BP;
+ cfa->offset = 0;
+ }
+ }
+ break;
+ }
+
+ if (op->dest.reg == CFI_SP && op->src.reg == CFI_BP) {
+ /* addi.d sp,fp,imm for the secondary stack on LoongArch */
+ if (cfa->base == CFI_FP && cfa->offset == 0) {
+ if (insn->sym->secondary_stack) {
+ cfa->base = CFI_SP;
+ cfa->offset = -op->src.offset;
+ }
+ }
+ break;
+ }
+
if (op->dest.reg == CFI_SP && op->src.reg == CFI_BP) {
/* lea disp(%rbp), %rsp */
diff --git a/tools/objtool/include/objtool/elf.h b/tools/objtool/include/objtool/elf.h
index 2b8a69de4db8..586916e0d441 100644
--- a/tools/objtool/include/objtool/elf.h
+++ b/tools/objtool/include/objtool/elf.h
@@ -68,6 +68,7 @@ struct symbol {
u8 warned : 1;
u8 embedded_insn : 1;
u8 local_label : 1;
+ u8 secondary_stack : 1;
struct list_head pv_target;
struct reloc *relocs;
};
--
2.42.0On 2024-08-05 11:26, Tiezhu Yang wrote:
> After commit a0f7085f6a63 ("LoongArch: Add RANDOMIZE_KSTACK_OFFSET
> support"), there is a new instruction "sub.d $sp, $sp, $t0" for the
> secondary stack in do_syscall(), then there exists a objtool warning
> "do_syscall+0x11c: return with modified stack frame" and there is no
> handle_syscall() which is the previous frame of do_syscall() in the
> call trace when executing the command "echo l > /proc/sysrq-trigger".
>
> objdump shows something like this:
>
> 0000000000000000 <do_syscall>:
> 0: 02ff8063 addi.d $sp, $sp, -32
> 4: 29c04076 st.d $fp, $sp, 16
> 8: 29c02077 st.d $s0, $sp, 8
> c: 29c06061 st.d $ra, $sp, 24
> 10: 02c08076 addi.d $fp, $sp, 32
> ...
> 74: 0011b063 sub.d $sp, $sp, $t0
> ...
> a8: 4c000181 jirl $ra, $t0, 0
> ...
> dc: 02ff82c3 addi.d $sp, $fp, -32
> e0: 28c06061 ld.d $ra, $sp, 24
> e4: 28c04076 ld.d $fp, $sp, 16
> e8: 28c02077 ld.d $s0, $sp, 8
> ec: 02c08063 addi.d $sp, $sp, 32
> f0: 4c000020 jirl $zero, $ra, 0
>
> The instruction "sub.d $sp, $sp, $t0" changes the stack bottom
> and the new stack size is a random value, in order to find the
> return address of do_syscall() which is stored in the original
> stack frame after executing "jirl $ra, $t0, 0", it should use
> fp which points to the original stack top.
>
> This is a rare case, add a member "secondary_stack" in the struct
> symbol as a label to avoid affecting the current normal case, then
> set it as true only if there exists the secondary stack instruction
> "sub.d $sp, $sp, $t0", at last check this label for the other special
> secondary stack instructions about fp to change the cfa base and cfa
> offset during the period of secondary stack in update_cfi_state().
>
> Tested with the following two configs:
> (1) CONFIG_RANDOMIZE_KSTACK_OFFSET=y &&
> CONFIG_RANDOMIZE_KSTACK_OFFSET_DEFAULT=n
> (2) CONFIG_RANDOMIZE_KSTACK_OFFSET=y &&
> CONFIG_RANDOMIZE_KSTACK_OFFSET_DEFAULT=y
>
> Cc: stable@vger.kernel.org # 6.9+
> Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
> ---
> tools/objtool/arch/loongarch/decode.c | 8 +++++++-
> tools/objtool/check.c | 22 ++++++++++++++++++++++
> tools/objtool/include/objtool/elf.h | 1 +
> 3 files changed, 30 insertions(+), 1 deletion(-)
>
> diff --git a/tools/objtool/arch/loongarch/decode.c b/tools/objtool/arch/loongarch/decode.c
> index db4dd05cdb49..4085714ffd18 100644
> --- a/tools/objtool/arch/loongarch/decode.c
> +++ b/tools/objtool/arch/loongarch/decode.c
> @@ -122,7 +122,7 @@ static bool decode_insn_reg2i12_fomat(union loongarch_instruction inst,
> switch (inst.reg2i12_format.opcode) {
> case addid_op:
> if ((inst.reg2i12_format.rd == CFI_SP) || (inst.reg2i12_format.rj == CFI_SP)) {
> - /* addi.d sp,sp,si12 or addi.d fp,sp,si12 */
> + /* addi.d sp,sp,si12 or addi.d fp,sp,si12 or addi.d sp,fp,si12 */
> insn->immediate = sign_extend64(inst.reg2i12_format.immediate, 11);
> ADD_OP(op) {
> op->src.type = OP_SRC_ADD;
> @@ -275,6 +275,8 @@ static bool decode_insn_reg2i16_fomat(union loongarch_instruction inst,
> static bool decode_insn_reg3_fomat(union loongarch_instruction inst,
> struct instruction *insn)
> {
> + struct symbol *func;
> +
> switch (inst.reg3_format.opcode) {
> case subd_op:
> if ((inst.reg3_format.rd == CFI_SP) && (inst.reg3_format.rj == CFI_SP)) {
> @@ -282,6 +284,10 @@ static bool decode_insn_reg3_fomat(union loongarch_instruction inst,
> * sub.d sp,sp,t0
> * this is a rare case for the secondary stack.
> */
> + func = find_func_containing(insn->sec, insn->offset);
> + if (!func)
> + return false;
> + func->secondary_stack = true;
> }
> break;
> default:
> diff --git a/tools/objtool/check.c b/tools/objtool/check.c
> index 01237d167223..c7b9942fee29 100644
> --- a/tools/objtool/check.c
> +++ b/tools/objtool/check.c
> @@ -2993,6 +2993,28 @@ static int update_cfi_state(struct instruction *insn,
> break;
> }
>
> + if (op->dest.reg == CFI_BP && op->src.reg == CFI_SP) {
> + /* addi.d fp,sp,imm for the secondary stack on LoongArch */
> + if (cfa->base == CFI_SP && cfa->offset == op->src.offset) {
> + if (insn->sym->secondary_stack) {
> + cfa->base = CFI_BP;
> + cfa->offset = 0;
> + }
> + }
> + break;
> + }
> +
> + if (op->dest.reg == CFI_SP && op->src.reg == CFI_BP) {
> + /* addi.d sp,fp,imm for the secondary stack on LoongArch */
> + if (cfa->base == CFI_FP && cfa->offset == 0) {
> + if (insn->sym->secondary_stack) {
> + cfa->base = CFI_SP;
> + cfa->offset = -op->src.offset;
> + }
> + }
> + break;
> + }
> +
> if (op->dest.reg == CFI_SP && op->src.reg == CFI_BP) {
>
> /* lea disp(%rbp), %rsp */
> diff --git a/tools/objtool/include/objtool/elf.h b/tools/objtool/include/objtool/elf.h
> index 2b8a69de4db8..586916e0d441 100644
> --- a/tools/objtool/include/objtool/elf.h
> +++ b/tools/objtool/include/objtool/elf.h
> @@ -68,6 +68,7 @@ struct symbol {
> u8 warned : 1;
> u8 embedded_insn : 1;
> u8 local_label : 1;
> + u8 secondary_stack : 1;
> struct list_head pv_target;
> struct reloc *relocs;
> };
LGTM although I think we should make update_cfi_state() arch-specific.
It also fix many warning when use Clang, thanks.
Jinyang
Hi Tiezhu,
kernel test robot noticed the following build errors:
[auto build test ERROR on linus/master]
[also build test ERROR on v6.11-rc2 next-20240802]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Tiezhu-Yang/objtool-LoongArch-Decode-secondary-stack-instruction/20240805-113316
base: linus/master
patch link: https://lore.kernel.org/r/20240805032700.16038-3-yangtiezhu%40loongson.cn
patch subject: [PATCH v2 2/3] objtool: Handle secondary stack related instructions
config: x86_64-allmodconfig (https://download.01.org/0day-ci/archive/20240805/202408051602.NqvBpTsv-lkp@intel.com/config)
compiler: clang version 18.1.5 (https://github.com/llvm/llvm-project 617a15a9eac96088ae5e9134248d8236e34b91b1)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240805/202408051602.NqvBpTsv-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202408051602.NqvBpTsv-lkp@intel.com/
All errors (new ones prefixed by >>):
scripts/genksyms/parse.y: warning: 9 shift/reduce conflicts [-Wconflicts-sr]
scripts/genksyms/parse.y: warning: 5 reduce/reduce conflicts [-Wconflicts-rr]
scripts/genksyms/parse.y: note: rerun with option '-Wcounterexamples' to generate conflict counterexamples
>> check.c:3009:22: error: use of undeclared identifier 'CFI_FP'
3009 | if (cfa->base == CFI_FP && cfa->offset == 0) {
| ^
1 error generated.
make[5]: *** [tools/build/Makefile.build:105: tools/objtool/check.o] Error 1
make[5]: *** Waiting for unfinished jobs....
make[4]: *** [Makefile:70: tools/objtool/objtool-in.o] Error 2
make[3]: *** [Makefile:72: objtool] Error 2
make[2]: *** [Makefile:1360: tools/objtool] Error 2
make[2]: Target 'prepare' not remade because of errors.
make[1]: *** [Makefile:224: __sub-make] Error 2
make[1]: Target 'prepare' not remade because of errors.
make: *** [Makefile:224: __sub-make] Error 2
make: Target 'prepare' not remade because of errors.
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
On 08/05/2024 11:26 AM, Tiezhu Yang wrote:
> After commit a0f7085f6a63 ("LoongArch: Add RANDOMIZE_KSTACK_OFFSET
> support"), there is a new instruction "sub.d $sp, $sp, $t0" for the
> secondary stack in do_syscall(), then there exists a objtool warning
> "do_syscall+0x11c: return with modified stack frame" and there is no
> handle_syscall() which is the previous frame of do_syscall() in the
> call trace when executing the command "echo l > /proc/sysrq-trigger".
...
> diff --git a/tools/objtool/check.c b/tools/objtool/check.c
> index 01237d167223..c7b9942fee29 100644
> --- a/tools/objtool/check.c
> +++ b/tools/objtool/check.c
> @@ -2993,6 +2993,28 @@ static int update_cfi_state(struct instruction *insn,
> break;
> }
>
> + if (op->dest.reg == CFI_BP && op->src.reg == CFI_SP) {
> + /* addi.d fp,sp,imm for the secondary stack on LoongArch */
> + if (cfa->base == CFI_SP && cfa->offset == op->src.offset) {
> + if (insn->sym->secondary_stack) {
> + cfa->base = CFI_BP;
> + cfa->offset = 0;
> + }
> + }
> + break;
> + }
> +
> + if (op->dest.reg == CFI_SP && op->src.reg == CFI_BP) {
> + /* addi.d sp,fp,imm for the secondary stack on LoongArch */
> + if (cfa->base == CFI_FP && cfa->offset == 0) {
Here should be CFI_BP instead of CFI_FP which is only defined
for LoongArch.
> + if (insn->sym->secondary_stack) {
> + cfa->base = CFI_SP;
> + cfa->offset = -op->src.offset;
> + }
> + }
> + break;
> + }
> +
> if (op->dest.reg == CFI_SP && op->src.reg == CFI_BP) {
>
> /* lea disp(%rbp), %rsp */
Oh, sorry, I forgot to test this change on x86.
Here is the test info on x86: the cfa->base is CFI_BP
or CFI_BP_INDIRECT and the cfa->offset is not 0
if (op->dest.reg == CFI_SP && op->src.reg == CFI_BP),
thus it can check the following condition
if(cfa->base == CFI_BP && cfa->offset == 0)
to distinguish x86 and LoongArch.
So the correct change should be something like this
to make sure it works well for both x86 and LoongArch:
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index 01237d167223..0832d20c95d2 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -2993,10 +2993,28 @@ static int update_cfi_state(struct instruction
*insn,
break;
}
- if (op->dest.reg == CFI_SP && op->src.reg ==
CFI_BP) {
+ if (op->dest.reg == CFI_BP && op->src.reg ==
CFI_SP) {
+ /* addi.d fp,sp,imm for the secondary
stack on LoongArch */
+ if (cfa->base == CFI_SP && cfa->offset
== op->src.offset) {
+ if (insn->sym->secondary_stack) {
+ cfa->base = CFI_BP;
+ cfa->offset = 0;
+ }
+ }
+ break;
+ }
- /* lea disp(%rbp), %rsp */
- cfi->stack_size = -(op->src.offset +
regs[CFI_BP].offset);
+ if (op->dest.reg == CFI_SP && op->src.reg ==
CFI_BP) {
+ /* addi.d sp,fp,imm for the secondary
stack on LoongArch */
+ if (cfa->base == CFI_BP && cfa->offset
== 0) {
+ if (insn->sym->secondary_stack) {
+ cfa->base = CFI_SP;
+ cfa->offset =
-op->src.offset;
+ }
+ } else {
+ /* lea disp(%rbp), %rsp */
+ cfi->stack_size =
-(op->src.offset + regs[CFI_BP].offset);
+ }
break;
}
I will wait for some days to get more review comments
and then send v3 later.
Thanks,
Tiezhu
© 2016 - 2025 Red Hat, Inc.