lkml.org 
[lkml]   [2021]   [Oct]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 2/2] tools/nolibc: x86-64: Fix startup code bug
Hi Ammar,

sorry for the delay, I needed to check a few things first.

On Fri, Oct 15, 2021 at 03:25:07PM +0700, Ammar Faizi wrote:
> Before this patch, the _start function looks like this:
>
> 0000000000001170 <_start>:
> 1170: pop %rdi
> 1171: mov %rsp,%rsi
> 1174: lea 0x8(%rsi,%rdi,8),%rdx
> 1179: and $0xfffffffffffffff0,%rsp
> 117d: sub $0x8,%rsp
> 1181: call 1000 <main>
> 1186: movzbq %al,%rdi
> 118a: mov $0x3c,%rax
> 1191: syscall
> 1193: hlt
> 1194: data16 cs nopw 0x0(%rax,%rax,1)
> 119f: nop
>
> Note the "and" to %rsp, it makes the %rsp be 16-byte aligned, but then
> there is a "sub" with $0x8 which makes the %rsp no longer 16-byte
> aligned, then it calls main. That's the bug!
>
> Right before "call", the %rsp must be 16-byte aligned. So the "sub"
> here breaks the alignment. Remove it.

That's very interesting because my understanding till now was that
the stack had to be 16-aligned in the callee, not the caller. But I've
checked the psABI doc, and it indeed says in section 3.2.2 that it's
rsp+8 which is 16-aligned. Of course, when pushing a frame pointer
onto the stack, it becomes the same. Thanks for spotting this one!

> Also the content of %rbp may be unspecified at process initialization
> time. For example, if the _start gets called by an interpreter, the
> interpreter may not zero the %rbp, so we should zero the %rbp on _start.

OK.

> Extra fixes:
> - Use NR_exit_group instead of NR_exit.

Please, this is independent on the fix above so it must be subject
of a different patch with its own justification. Also it should cover
all supported architectures, otherwise programs will start to behave
differently on different targets.

> - Use `mov %eax,%edi` instead of `movzbq %al,%rdi`. This makes the
> exit code more observable from strace. While the exit code is
> only 8-bit, the kernel has taken care of that, so no need to
> worry about it.

I'm fine with this one as well, but similarly, it should be in its own
patch and applied to all architectures.

> /* startup code */
> -asm(".section .text\n"
> - ".global _start\n"
> - "_start:\n"
> - "pop %rdi\n" // argc (first arg, %rdi)
> - "mov %rsp, %rsi\n" // argv[] (second arg, %rsi)
> - "lea 8(%rsi,%rdi,8),%rdx\n" // then a NULL then envp (third arg, %rdx)
> - "and $-16, %rsp\n" // x86 ABI : esp must be 16-byte aligned when
> - "sub $8, %rsp\n" // entering the callee
> - "call main\n" // main() returns the status code, we'll exit with it.
> - "movzb %al, %rdi\n" // retrieve exit code from 8 lower bits
> - "mov $60, %rax\n" // NR_exit == 60
> - "syscall\n" // really exit
> - "hlt\n" // ensure it does not return
> - "");
> +asm(
> + ".section .text\n"
> + ".global _start\n"
> +
> + "_start:\n\t"
> + "popq %rdi\n\t" // argc (first arg, %rdi)
> + "movq %rsp, %rsi\n\t" // argv[] (second arg, %rsi)
> + "leaq 8(%rsi,%rdi,8), %rdx\n\t" // then a NULL, then envp (third arg, %rdx)
> +
> + /*
> + * The System V ABI mandates the deepest stack frame should be zero.
> + * Thus we zero the %rbp here.
> + */
> + "xorl %ebp, %ebp\n\t"
> +
> + /*
> + * The System V ABI mandates the %rsp needs to be aligned at 16-byte
> + * before performing a function call.
> + */
> + "andq $-16, %rsp\n\t"
> +
> + /*
> + * main() returns the status code, we will exit with it.
> + */
> + "callq main\n\t"
> +
> + /*
> + * Move the return value to the first argument of exit_group.
> + */
> + "movl %eax, %edi\n\t"
> +
> + /*
> + * NR_exit_group == 231
> + */
> + "movl $231, %eax\n\t"
> +
> + /*
> + * Really exit.
> + */
> + "syscall\n\t"
> +
> + /*
> + * Ensure it does not return.
> + */
> + "hlt\n\t"
> +);

I find the whole thing much less readable here, as asm code tends to
be read as visual groups of patterns. I'm suggesting that you place a
multi-line comment before the asm statement indicating the general
rules (e.g. lowest stack frame must be zero, rsp+8 must be multiple of
16 etc), then only comment each instruction on the same line as it was
before.

Thank you!
Willy

\
 
 \ /
  Last update: 2021-10-18 07:00    [W:0.135 / U:0.220 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site