linux-kernel - Re: [PATCH v3 04/29] x86: assembly, use ENDPROC for functions

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <4c3b765e-483a-4d9b-c451-6eccc02403f3@suse.cz>
Date:   Fri, 12 May 2017 09:53:48 +0200
From:   Jiri Slaby <jslaby@...e.cz>
To:     Josh Poimboeuf <jpoimboe@...hat.com>
Cc:     mingo@...hat.com, tglx@...utronix.de, hpa@...or.com,
        x86@...nel.org, linux-kernel@...r.kernel.org,
        Boris Ostrovsky <boris.ostrovsky@...cle.com>,
        Juergen Gross <jgross@...e.com>, xen-devel@...ts.xenproject.org
Subject: Re: [PATCH v3 04/29] x86: assembly, use ENDPROC for functions

On 04/26/2017, 03:42 AM, Josh Poimboeuf wrote:
>> @@ -323,7 +323,7 @@ ENTRY(resume_userspace)
>>  	movl	%esp, %eax
>>  	call	prepare_exit_to_usermode
>>  	jmp	restore_all
>> -END(ret_from_exception)
>> +ENDPROC(ret_from_exception)
> 
> What exactly is the motivation of this patch?  It would be good to
> describe that in the commit message.
> 
> Is the point to allow objtool to generate CFI for it?  If so, I don't
> really see how that would work.  Today, objtool considers ENDPROC to
> annotate a *callable* function which conforms to the C calling ABI and
> can be called by another function.  The stack is in a known state at
> function entry, and so the CFI (or frame pointer info) can be reliably
> determined.

Ugh, I haven't checked this in 100 % of cases, but this looks pretty
fragile to me. From reading the code, the use of END or ENDPROC is
rather random -- depending on mood and who wrote the code.

> But entry code is different.  In most cases, the global symbols aren't
> actually called, and they don't follow any conventions.  The code is
> spaghetti-esque, with HW handlers and jumps everywhere.  The state of
> the stack at symbol entry varies per "function".  That's why objtool
> ignores these files.

Unfortunately, this is true.

> For special cases (like entry code), I was thinking we'd need manual CFI
> annotations, like we had before.  Or maybe there's another way, like
> some new macros which tell objtool about the HW entry points and the
> state of the registers there.
> 
> But I'm having trouble seeing how marking these code snippets with
> ENTRY/ENDPROC would help objtool make any sense of the code and where
> things are on the stack.

Ok, my intention was to have every line of assembly code in between of
FUNC_START/FUNC_END. That way, every rsp related push/pop/sub/add can be
annotated very easily. For the C-like functions this is all what needs
to be done.

Then there is the spaghetti code. And I was thinking about manual
annotations like:

  # skip the frame pointer checking between START+END here
  OBJTOOL(SKIP_CHECKING)

  # this fn has unusual frame (like interrupts have),
    and you can find return RIP stored at fp + 0x20
  OBJTOOL(RIP_IS_AT, 0x20)

  # put this raw CFI for this location into eh_frame
  OBJTOOL(RAW_CFI, 0x00, 0x00, 0x00)


Similarly, I have OBJTOOL(START_FUNC) and OBJTOOL(END_FUNC) emitted with
each FUNC_START/FUNC_END. So far, when manually expanded for simplicity,
it looks like this:

#define OBJTOOL_START_FUNC                              \
        .pushsection .discard.asmfunctions ASM_NL       \
        .long 0xfd111111 ASM_NL                         \
        .long 1f - . ASM_NL                             \
        .popsection ASM_NL                              \
        1:

#define OBJTOOL_END_FUNC                                \
        .pushsection .discard.asmfunctions ASM_NL       \
        .long 0xfe111111 ASM_NL                         \
        .long 1f - . ASM_NL                             \
        .popsection ASM_NL                              \
        1:

0xfd111111, 0xfe111111 are "opcodes" for objtool meaning
START_FUNC/END_FUNC. Similar would be SKIP_CHECKING, RIP_IS_AT, and
RAW_CFI from the above.

So on the objtool side, it looks like:
        switch (data->magic) {
        case 0xfd111111:
                pc_begin = rela->addend;
                break;
        case 0xfe111111:
                ret = dwarf_annotate_func(dwarf, rela->sym->sec,
                                pc_begin, rela->addend - pc_begin);
                if (ret < 0)
                        return -1;

                break;

So this was my idea -- having all code marked as function and manually
annotate those which are different.

thanks,
-- 
js
suse labs