lkml.org 
[lkml]   [2013]   [Oct]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    Subject[PATCH] static_key: WARN on usage before jump_label_init was called
    On Sun, Oct 06, 2013 at 02:12:47AM +0200, Hannes Frederic Sowa wrote:
    > On Sat, Oct 05, 2013 at 08:05:58PM -0400, Steven Rostedt wrote:
    > > > if (type == JUMP_LABEL_ENABLE) {
    > > > - /*
    > > > - * We are enabling this jump label. If it is not a nop
    > > > - * then something must have gone wrong.
    > > > - */
    > > > - if (unlikely(memcmp((void *)entry->code, ideal_nop, 5) != 0))
    > > > - bug_at((void *)entry->code, __LINE__);
    > > > + if (init) {
    > > > + /*
    > > > + * Jump label is enabled for the first time.
    > > > + * So we expect a default_nop...
    > > > + */
    > > > + if (unlikely(memcmp((void *)entry->code, default_nop, 5)
    > > > + != 0))
    > > > + bug_at((void *)entry->code, __LINE__);
    > > > + } else {
    > > > + /*
    > > > + * ...otherwise expect an ideal_nop. Otherwise
    > > > + * something went horribly wrong.
    > > > + */
    > > > + if (unlikely(memcmp((void *)entry->code, ideal_nop, 5)
    > > > + != 0))
    > > > + bug_at((void *)entry->code, __LINE__);
    > > > + }
    > >
    > > I don't know if I like this change. This is similar to a bug we had
    > > with the Xen folks, where they didn't realize that jump labels are not
    > > suppose to be used (or set) before jump_label_init() is called.
    > >
    > > I'll have to take a deeper look at this on Monday.
    >
    > Yes, I understand and saw the commit to call jump_label_init
    > earlier. Maybe the default could be to insert illegal instructions by
    > default if we try to replace them with nops or branches afterwards anyway.

    This would not help, but maybe someting like this patch. Andi Kleen
    also recently posted something similar, I cleaned it up a bit.

    [PATCH] static_key: WARN on usage before jump_label_init was called

    Based on a patch from Andi Kleen.

    Cc: Steven Rostedt <rostedt@goodmis.org>
    Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
    Cc: Andi Kleen <andi@firstfloor.org>
    Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
    ---
    include/linux/jump_label.h | 11 +++++++++++
    include/linux/jump_label_ratelimit.h | 2 ++
    kernel/jump_label.c | 5 +++++
    lib/Makefile | 2 +-
    lib/jump_label_initialized.c | 6 ++++++
    5 files changed, 25 insertions(+), 1 deletion(-)
    create mode 100644 lib/jump_label_initialized.c

    diff --git a/include/linux/jump_label.h b/include/linux/jump_label.h
    index a507907..ed3a4bd 100644
    --- a/include/linux/jump_label.h
    +++ b/include/linux/jump_label.h
    @@ -48,6 +48,14 @@

    #include <linux/types.h>
    #include <linux/compiler.h>
    +#include <linux/bug.h>
    +
    +extern bool static_key_initialized;
    +
    +#define STATIC_KEY_CHECK_USE() do { \
    + WARN(!static_key_initialized, "%s used before call to jump_label_init", \
    + __func__); \
    +} while (0)

    #if defined(CC_HAVE_ASM_GOTO) && defined(CONFIG_JUMP_LABEL)

    @@ -128,6 +136,7 @@ struct static_key {

    static __always_inline void jump_label_init(void)
    {
    + static_key_initialized = true;
    }

    static __always_inline bool static_key_false(struct static_key *key)
    @@ -146,11 +155,13 @@ static __always_inline bool static_key_true(struct static_key *key)

    static inline void static_key_slow_inc(struct static_key *key)
    {
    + STATIC_KEY_CHECK_USE();
    atomic_inc(&key->enabled);
    }

    static inline void static_key_slow_dec(struct static_key *key)
    {
    + STATIC_KEY_CHECK_USE();
    atomic_dec(&key->enabled);
    }

    diff --git a/include/linux/jump_label_ratelimit.h b/include/linux/jump_label_ratelimit.h
    index 1137883..089f70f 100644
    --- a/include/linux/jump_label_ratelimit.h
    +++ b/include/linux/jump_label_ratelimit.h
    @@ -23,12 +23,14 @@ struct static_key_deferred {
    };
    static inline void static_key_slow_dec_deferred(struct static_key_deferred *key)
    {
    + STATIC_KEY_CHECK_USE();
    static_key_slow_dec(&key->key);
    }
    static inline void
    jump_label_rate_limit(struct static_key_deferred *key,
    unsigned long rl)
    {
    + STATIC_KEY_CHECK_USE();
    }
    #endif /* HAVE_JUMP_LABEL */
    #endif /* _LINUX_JUMP_LABEL_RATELIMIT_H */
    diff --git a/kernel/jump_label.c b/kernel/jump_label.c
    index 297a924..9019f15 100644
    --- a/kernel/jump_label.c
    +++ b/kernel/jump_label.c
    @@ -58,6 +58,7 @@ static void jump_label_update(struct static_key *key, int enable);

    void static_key_slow_inc(struct static_key *key)
    {
    + STATIC_KEY_CHECK_USE();
    if (atomic_inc_not_zero(&key->enabled))
    return;

    @@ -103,12 +104,14 @@ static void jump_label_update_timeout(struct work_struct *work)

    void static_key_slow_dec(struct static_key *key)
    {
    + STATIC_KEY_CHECK_USE();
    __static_key_slow_dec(key, 0, NULL);
    }
    EXPORT_SYMBOL_GPL(static_key_slow_dec);

    void static_key_slow_dec_deferred(struct static_key_deferred *key)
    {
    + STATIC_KEY_CHECK_USE();
    __static_key_slow_dec(&key->key, key->timeout, &key->work);
    }
    EXPORT_SYMBOL_GPL(static_key_slow_dec_deferred);
    @@ -116,6 +119,7 @@ EXPORT_SYMBOL_GPL(static_key_slow_dec_deferred);
    void jump_label_rate_limit(struct static_key_deferred *key,
    unsigned long rl)
    {
    + STATIC_KEY_CHECK_USE();
    key->timeout = rl;
    INIT_DELAYED_WORK(&key->work, jump_label_update_timeout);
    }
    @@ -212,6 +216,7 @@ void __init jump_label_init(void)
    key->next = NULL;
    #endif
    }
    + static_key_initialized = true;
    jump_label_unlock();
    }

    diff --git a/lib/Makefile b/lib/Makefile
    index f3bb2cb..7f48ddc 100644
    --- a/lib/Makefile
    +++ b/lib/Makefile
    @@ -26,7 +26,7 @@ obj-y += bcd.o div64.o sort.o parser.o halfmd4.o debug_locks.o random32.o \
    bust_spinlocks.o hexdump.o kasprintf.o bitmap.o scatterlist.o \
    gcd.o lcm.o list_sort.o uuid.o flex_array.o iovec.o clz_ctz.o \
    bsearch.o find_last_bit.o find_next_bit.o llist.o memweight.o kfifo.o \
    - percpu_ida.o
    + percpu_ida.o jump_label_initialized.o
    obj-y += string_helpers.o
    obj-$(CONFIG_TEST_STRING_HELPERS) += test-string_helpers.o
    obj-y += kstrtox.o
    diff --git a/lib/jump_label_initialized.c b/lib/jump_label_initialized.c
    new file mode 100644
    index 0000000..a668a40
    --- /dev/null
    +++ b/lib/jump_label_initialized.c
    @@ -0,0 +1,6 @@
    +#include <linux/types.h>
    +#include <linux/cache.h>
    +
    +bool static_key_initialized __read_mostly = false;
    +EXPORT_SYMBOL_GPL(static_key_initialized);
    +
    --
    1.8.3.1


    \
     
     \ /
      Last update: 2013-10-06 20:41    [W:2.282 / U:0.088 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site