lkml.org 
[lkml]   [2019]   [Oct]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: [PATCH v4 03/16] x86/alternatives,jump_label: Provide better text_poke() batching interface

    * Peter Zijlstra <peterz@infradead.org> wrote:

    > --- a/arch/x86/kernel/jump_label.c
    > +++ b/arch/x86/kernel/jump_label.c
    > @@ -35,18 +35,19 @@ static void bug_at(unsigned char *ip, in
    > BUG();
    > }
    >
    > -static void __jump_label_set_jump_code(struct jump_entry *entry,
    > - enum jump_label_type type,
    > - union jump_code_union *code,
    > - int init)
    > +static const void *
    > +__jump_label_set_jump_code(struct jump_entry *entry, enum jump_label_type type, int init)
    > {
    > + static union jump_code_union code; /* relies on text_mutex */
    > const unsigned char default_nop[] = { STATIC_KEY_INIT_NOP };
    > const unsigned char *ideal_nop = ideal_nops[NOP_ATOMIC5];
    > const void *expect;
    > int line;
    >
    > - code->jump = 0xe9;
    > - code->offset = jump_entry_target(entry) -
    > + lockdep_assert_held(&text_mutex);
    > +
    > + code.jump = JMP32_INSN_OPCODE;
    > + code.offset = jump_entry_target(entry) -
    > (jump_entry_code(entry) + JUMP_LABEL_NOP_SIZE);
    >
    > if (init) {
    > @@ -54,23 +55,23 @@ static void __jump_label_set_jump_code(s
    > } else if (type == JUMP_LABEL_JMP) {
    > expect = ideal_nop; line = __LINE__;
    > } else {
    > - expect = code->code; line = __LINE__;
    > + expect = code.code; line = __LINE__;

    Side note: the whole 'line' logic looked weird to me and it obsfuscates
    the logic a bit, and I had to look it up to see what it's about:
    improving the debug output of text-patching crashes.

    How about something like the below on top of your queue? We have %phD
    that can nicely print instructions in hex.

    Totally untested though.

    Thanks,

    Ingo

    ---
    arch/x86/kernel/jump_label.c | 21 ++++++++++-----------
    1 file changed, 10 insertions(+), 11 deletions(-)

    Index: tip/arch/x86/kernel/jump_label.c
    ===================================================================
    --- tip.orig/arch/x86/kernel/jump_label.c
    +++ tip/arch/x86/kernel/jump_label.c
    @@ -16,14 +16,15 @@
    #include <asm/alternative.h>
    #include <asm/text-patching.h>

    -static void bug_at(const void *ip, int line)
    +static void bug_at(const void *ip, const void *op_expected, const void *op_unexpected)
    {
    /*
    * The location is not an op that we were expecting.
    * Something went wrong. Crash the box, as something could be
    * corrupting the kernel.
    */
    - pr_crit("jump_label: Fatal kernel bug, unexpected op at %pS [%p] (%5ph) %d\n", ip, ip, ip, line);
    + pr_crit("jump_label: Fatal kernel bug, expected op (%*phD), unexpected op (%*phD) at %pS [%p] (%5ph\n",
    + JUMP_LABEL_NOP_SIZE, op_expected, JUMP_LABEL_NOP_SIZE, op_unexpected, ip, ip, ip);
    BUG();
    }

    @@ -34,23 +35,21 @@ __jump_label_set_jump_code(struct jump_e
    const unsigned char *ideal_nop = ideal_nops[NOP_ATOMIC5];
    const void *expect, *code;
    const void *addr, *dest;
    - int line;

    addr = (void *)jump_entry_code(entry);
    dest = (void *)jump_entry_target(entry);

    code = text_gen_insn(JMP32_INSN_OPCODE, addr, dest);

    - if (init) {
    - expect = default_nop; line = __LINE__;
    - } else if (type == JUMP_LABEL_JMP) {
    - expect = ideal_nop; line = __LINE__;
    - } else {
    - expect = code; line = __LINE__;
    - }
    + if (init)
    + expect = default_nop;
    + else if (type == JUMP_LABEL_JMP)
    + expect = ideal_nop;
    + else
    + expect = code;

    if (memcmp(addr, expect, JUMP_LABEL_NOP_SIZE))
    - bug_at(addr, line);
    + bug_at(addr, expect, addr);

    if (type == JUMP_LABEL_NOP)
    code = ideal_nop;
    \
     
     \ /
      Last update: 2019-10-21 10:48    [W:3.513 / U:0.072 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site