26 Aug, 2016

1 commit

  • There's no reliable way to determine which module tainted the kernel
    with TAINT_LIVEPATCH. For example, /sys/module//taint
    doesn't report it. Neither does the "mod -t" command in the crash tool.

    Make it crystal clear who the guilty party is by associating
    TAINT_LIVEPATCH with any module which sets the "livepatch" modinfo
    attribute. The flag will still get set in the kernel like before, but
    now it also sets the same flag in mod->taint.

    Note that now the taint flag gets set when the module is loaded rather
    than when it's enabled.

    I also renamed find_livepatch_modinfo() to check_modinfo_livepatch() to
    better reflect its purpose: it's basically a livepatch-specific
    sub-function of check_modinfo().

    Reported-by: Chunyu Hu
    Reviewed-by: Petr Mladek
    Acked-by: Miroslav Benes
    Acked-by: Jessica Yu
    Acked-by: Rusty Russell
    Signed-off-by: Josh Poimboeuf
    Signed-off-by: Jiri Kosina

    Josh Poimboeuf
     

19 Aug, 2016

1 commit


04 Aug, 2016

1 commit

  • Add ro_after_init support for modules by adding a new page-aligned section
    in the module layout (after rodata) for ro_after_init data and enabling RO
    protection for that section after module init runs.

    Signed-off-by: Jessica Yu
    Acked-by: Kees Cook
    Signed-off-by: Rusty Russell

    Jessica Yu
     

17 May, 2016

1 commit


30 Apr, 2016

1 commit

  • Current object-walking helper checks the presence of obj->funcs to
    determine the end of objs array in klp_object structure. This is
    somewhat fragile because one can easily forget about funcs definition
    during livepatch creation. In such a case the livepatch module is
    successfully loaded and all objects after the incorrect one are omitted.
    This is very confusing. Let's make the helper more robust and check also
    for the other external member, name. Thus the helper correctly stops on
    an empty item of the array. We need to have a check for obj->funcs in
    klp_init_object() to make it work.

    The same applies to a func-walking helper.

    As a benefit we'll check for new_func member definition during the
    livepatch initialization. There is no such check anywhere in the code
    now.

    [jkosina@suse.cz: fix shortlog]
    Signed-off-by: Miroslav Benes
    Acked-by: Josh Poimboeuf
    Acked-by: Jessica Yu
    Signed-off-by: Jiri Kosina

    Miroslav Benes
     

15 Apr, 2016

1 commit


14 Apr, 2016

1 commit

  • When livepatch tries to patch a function it takes the function address
    and asks ftrace to install the livepatch handler at that location.
    ftrace will look for an mcount call site at that exact address.

    On powerpc the mcount location is not the first instruction of the
    function, and in fact it's not at a constant offset from the start of
    the function. To accommodate this add a hook which arch code can
    override to customise the behaviour.

    Signed-off-by: Torsten Duwe
    Signed-off-by: Balbir Singh
    Signed-off-by: Petr Mladek
    Signed-off-by: Michael Ellerman

    Michael Ellerman
     

08 Apr, 2016

1 commit

  • Commit 425595a7fc20 ("livepatch: reuse module loader code to write
    relocations") adds a possibility of dereferncing pointers supplied by the
    consumer of the livepatch API before sanity (NULL) checking them (patch
    and patch->mod).

    Spotted by smatch tool.

    Reported-by: Dan Carpenter
    Acked-by: Josh Poimboeuf
    Acked-by: Jessica Yu
    Signed-off-by: Jiri Kosina

    Jiri Kosina
     

01 Apr, 2016

1 commit

  • Reuse module loader code to write relocations, thereby eliminating the need
    for architecture specific relocation code in livepatch. Specifically, reuse
    the apply_relocate_add() function in the module loader to write relocations
    instead of duplicating functionality in livepatch's arch-dependent
    klp_write_module_reloc() function.

    In order to accomplish this, livepatch modules manage their own relocation
    sections (marked with the SHF_RELA_LIVEPATCH section flag) and
    livepatch-specific symbols (marked with SHN_LIVEPATCH symbol section
    index). To apply livepatch relocation sections, livepatch symbols
    referenced by relocs are resolved and then apply_relocate_add() is called
    to apply those relocations.

    In addition, remove x86 livepatch relocation code and the s390
    klp_write_module_reloc() function stub. They are no longer needed since
    relocation work has been offloaded to module loader.

    Lastly, mark the module as a livepatch module so that the module loader
    canappropriately identify and initialize it.

    Signed-off-by: Jessica Yu
    Reviewed-by: Miroslav Benes
    Acked-by: Josh Poimboeuf
    Acked-by: Heiko Carstens # for s390 changes
    Signed-off-by: Jiri Kosina

    Jessica Yu
     

17 Mar, 2016

1 commit

  • Remove the livepatch module notifier in favor of directly enabling and
    disabling patches to modules in the module loader. Hard-coding the
    function calls ensures that ftrace_module_enable() is run before
    klp_module_coming() during module load, and that klp_module_going() is
    run before ftrace_release_mod() during module unload. This way, ftrace
    and livepatch code is run in the correct order during the module
    load/unload sequence without dependence on the module notifier call chain.

    Signed-off-by: Jessica Yu
    Reviewed-by: Petr Mladek
    Acked-by: Josh Poimboeuf
    Acked-by: Rusty Russell
    Signed-off-by: Jiri Kosina

    Jessica Yu
     

10 Mar, 2016

1 commit

  • klp_find_callback() stops the search when sympos is not defined and
    a second symbol of the same name is found. It means that the current
    error message about the unresolvable ambiguity always prints "(2 matches)".

    Let's remove this information. The total number of occurrences is
    not much helpful. The author of the patch still must put a non-trivial
    effort into searching the right position in the object file.

    [jkosina@suse.cz: fixed grammar as suggested by Josh]
    Signed-off-by: Petr Mladek
    Acked-by: Josh Poimboeuf
    Acked-by: Chris J Arges
    Signed-off-by: Jiri Kosina

    Petr Mladek
     

05 Dec, 2015

1 commit

  • Calling set_memory_rw() and set_memory_ro() for every iteration of the
    loop in klp_write_object_relocations() is messy, inefficient, and
    error-prone.

    Change all the read-only pages to read-write before the loop and convert
    them back to read-only again afterwards.

    Suggested-by: Miroslav Benes
    Signed-off-by: Josh Poimboeuf
    Reviewed-by: Petr Mladek
    Signed-off-by: Jiri Kosina

    Josh Poimboeuf
     

04 Dec, 2015

3 commits

  • The following directory structure will allow for cases when the same
    function name exists in a single object.
    /sys/kernel/livepatch///

    The sympos number corresponds to the nth occurrence of the symbol name in
    kallsyms for the patched object.

    An example of patching multiple symbols can be found here:
    https://github.com/dynup/kpatch/issues/493

    Signed-off-by: Chris J Arges
    Reviewed-by: Petr Mladek
    Acked-by: Josh Poimboeuf
    Signed-off-by: Jiri Kosina

    Chris J Arges
     
  • In cases of duplicate symbols, sympos will be used to disambiguate instead
    of val. By default sympos will be 0, and patching will only succeed if
    the symbol is unique. Specifying a positive value will ensure that
    occurrence of the symbol in kallsyms for the patched object will be used
    for patching if it is valid. For external relocations sympos is not
    supported.

    Remove klp_verify_callback, klp_verify_args and klp_verify_vmlinux_symbol
    as they are no longer used.

    From the klp_reloc structure remove val, as it can be refactored as a
    local variable in klp_write_object_relocations.

    Signed-off-by: Chris J Arges
    Reviewed-by: Petr Mladek
    Acked-by: Josh Poimboeuf
    Signed-off-by: Jiri Kosina

    Chris J Arges
     
  • Currently, patching objects with duplicate symbol names fail because the
    creation of the sysfs function directory collides with the previous
    attempt. Appending old_addr to the function name is problematic as it
    reveals the address of the function being patch to a normal user. Using
    the symbol's occurrence in kallsyms to postfix the function name in the
    sysfs directory solves the issue of having consistent unique names and
    ensuring that the address is not exposed to a normal user.

    In addition, using the symbol position as the user's method to disambiguate
    symbols instead of addr allows for disambiguating symbols in modules as
    well for both function addresses and for relocs. This also simplifies much
    of the code. Special handling for kASLR is no longer needed and can be
    removed. The klp_find_verify_func_addr function can be replaced by
    klp_find_object_symbol, and klp_verify_vmlinux_symbol and its callback can
    be removed completely.

    In cases of duplicate symbols, old_sympos will be used to disambiguate
    instead of old_addr. By default old_sympos will be 0, and patching will
    only succeed if the symbol is unique. Specifying a positive value will
    ensure that occurrence of the symbol in kallsyms for the patched object
    will be used for patching if it is valid.

    In addition, make old_addr an internal structure field not to be specified
    by the user. Finally, remove klp_find_verify_func_addr as it can be
    replaced by klp_find_object_symbol directly.

    Support for symbol position disambiguation for relocations is added in the
    next patch in this series.

    Signed-off-by: Chris J Arges
    Reviewed-by: Petr Mladek
    Acked-by: Josh Poimboeuf
    Signed-off-by: Jiri Kosina

    Chris J Arges
     

12 Nov, 2015

1 commit

  • With kASLR enabled, old_addr provided by patch module is being shifted
    accrodingly so that the symbol lookups work. To have module relocations
    handled properly as well, the same transformation needs to be perfomed
    on relocation address information.

    [jkosina@suse.cz: extended / reworded changelog a bit]
    Reported-by: Cyril B.
    Signed-off-by: Zhou Chengming
    Acked-by: Josh Poimboeuf
    Signed-off-by: Jiri Kosina

    Zhou Chengming
     

15 Jul, 2015

1 commit

  • In case of func->state or func->old_addr not having expected values,
    we'd rather bail out immediately from klp_disable_func().

    This can't really happen with the current codebase, but fix this
    anyway in the sake of robustness.

    [jkosina@suse.com: reworded the changelog a bit]
    Signed-off-by: Minfei Huang
    Acked-by: Josh Poimboeuf
    Signed-off-by: Jiri Kosina

    Minfei Huang
     

22 Jun, 2015

1 commit


03 Jun, 2015

1 commit

  • The list of loaded modules is walked through in
    module_kallsyms_on_each_symbol (called by kallsyms_on_each_symbol). The
    module_mutex lock should be acquired to prevent potential corruptions
    in the list.

    This was uncovered with new lockdep asserts in module code introduced by
    the commit 0be964be0d45 ("module: Sanitize RCU usage and locking") in
    recent next- trees.

    Signed-off-by: Miroslav Benes
    Acked-by: Josh Poimboeuf
    Cc: stable@vger.kernel.org
    Signed-off-by: Jiri Kosina

    Miroslav Benes
     

25 May, 2015

1 commit


20 May, 2015

2 commits

  • klp_for_each_object and klp_for_each_func are now used all over the
    code. One need not think what is the proper condition to check in the
    for loop now.

    Signed-off-by: Jiri Slaby
    Acked-by: Josh Poimboeuf
    Signed-off-by: Jiri Kosina

    Jiri Slaby
     
  • Make kobj variable (of type struct kobject) statically allocated in
    klp_object structure. It will allow us to move in the func-object-patch
    hierarchy through kobject links.

    The only reason to have it dynamic was to not have empty release
    callback in the code. However we have empty callbacks for function and
    patch in the code now, so it is no longer valid and the advantage of
    static allocation is clear.

    Signed-off-by: Miroslav Benes
    Signed-off-by: Jiri Slaby
    Acked-by: Josh Poimboeuf
    Signed-off-by: Jiri Kosina

    Miroslav Benes
     

18 May, 2015

1 commit

  • The previous patches can be applied, once the corresponding module is
    loaded. In general, the patch will do relocation (if necessary) and
    obtain/verify function address before we start to enable patch.

    There are three different situations in which the coming module notifier
    can fail:

    1) relocations are not applied for some reason. In this case kallsyms
    for module symbol is not called at all. The patch is not applied to the
    module. If the user disable and enable patch again, there is possible
    bug in klp_enable_func. If the user specified func->old_addr for some
    function in the module (and he shouldn't do that, but nevertheless) our
    warning would not catch it, ftrace will reject to register the handler
    because of wrong address or will register the handler for wrong address.

    2) relocations are applied successfully, but kallsyms lookup fails. In
    this case func->old_addr can be correct for all previous lookups, 0 for
    current failed one, and "unspecified" for the rest. If we undergo the
    same scenario as in 1, the behaviour differs for three cases, but the
    patch is not enabled anyway.

    3) the object is initialized, but klp_enable_object fails in the
    notifier due to possible ftrace error. Since it is improbable that
    ftrace would heal itself in the future, we would get those errors
    everytime the patch is enabled.

    In order to fix above situations, we can make obj->mod to NULL, if the
    coming modified notifier fails.

    Signed-off-by: Minfei Huang
    Acked-by: Josh Poimboeuf
    Reviewed-by: Miroslav Benes
    Signed-off-by: Jiri Kosina

    Minfei Huang
     

12 May, 2015

1 commit


29 Apr, 2015

1 commit

  • We give up old_addr hint from the coming patch module in cases when kernel load
    base has been randomized (as in such case, the coming module has no idea about
    the exact randomization offset).

    We are currently too pessimistic, and give up immediately as soon as
    CONFIG_RANDOMIZE_BASE is set; this doesn't however directly imply that the
    load base has actually been randomized. There are config options that
    disable kASLR (such as hibernation), user could have disabled kaslr on
    kernel command-line, etc.

    The loader propagates the information whether kernel has been randomized
    through bootparams. This allows us to have the condition more accurate.

    On top of that, it seems unnecessary to give up old_addr hints even if
    randomization is active. The relocation offset can be computed using
    kaslr_ofsset(), and therefore old_addr can be adjusted accordingly.

    Acked-by: Josh Poimboeuf
    Signed-off-by: Jiri Kosina

    Jiri Kosina
     

14 Apr, 2015

1 commit


17 Mar, 2015

1 commit

  • There is a notifier that handles live patches for coming and going modules.
    It takes klp_mutex lock to avoid races with coming and going patches but
    it does not keep the lock all the time. Therefore the following races are
    possible:

    1. The notifier is called sometime in STATE_MODULE_COMING. The module
    is visible by find_module() in this state all the time. It means that
    new patch can be registered and enabled even before the notifier is
    called. It might create wrong order of stacked patches, see below
    for an example.

    2. New patch could still see the module in the GOING state even after
    the notifier has been called. It will try to initialize the related
    object structures but the module could disappear at any time. There
    will stay mess in the structures. It might even cause an invalid
    memory access.

    This patch solves the problem by adding a boolean variable into struct module.
    The value is true after the coming and before the going handler is called.
    New patches need to be applied when the value is true and they need to ignore
    the module when the value is false.

    Note that we need to know state of all modules on the system. The races are
    related to new patches. Therefore we do not know what modules will get
    patched.

    Also note that we could not simply ignore going modules. The code from the
    module could be called even in the GOING state until mod->exit() finishes.
    If we start supporting patches with semantic changes between function
    calls, we need to apply new patches to any still usable code.
    See below for an example.

    Finally note that the patch solves only the situation when a new patch is
    registered. There are no such problems when the patch is being removed.
    It does not matter who disable the patch first, whether the normal
    disable_patch() or the module notifier. There is nothing to do
    once the patch is disabled.

    Alternative solutions:
    ======================

    + reject new patches when a patched module is coming or going; this is ugly

    + wait with adding new patch until the module leaves the COMING and GOING
    states; this might be dangerous and complicated; we would need to release
    kgr_lock in the middle of the patch registration to avoid a deadlock
    with the coming and going handlers; also we might need a waitqueue for
    each module which seems to be even bigger overhead than the boolean

    + stop modules from entering COMING and GOING states; wait until modules
    leave these states when they are already there; looks complicated; we would
    need to ignore the module that asked to stop the others to avoid a deadlock;
    also it is unclear what to do when two modules asked to stop others and
    both are in COMING state (situation when two new patches are applied)

    + always register/enable new patches and fix up the potential mess (registered
    patches order) in klp_module_init(); this is nasty and prone to regressions
    in the future development

    + add another MODULE_STATE where the kallsyms are visible but the module is not
    used yet; this looks too complex; the module states are checked on "many"
    locations

    Example of patch stacking breakage:
    ===================================

    The notifier could _not_ _simply_ ignore already initialized module objects.
    For example, let's have three patches (P1, P2, P3) for functions a() and b()
    where a() is from vmcore and b() is from a module M. Something like:

    a() b()
    P1 a1() b1()
    P2 a2() b2()
    P3 a3() b3(3)

    If you load the module M after all patches are registered and enabled.
    The ftrace ops for function a() and b() has listed the functions in this
    order:

    ops_a->func_stack -> list(a3,a2,a1)
    ops_b->func_stack -> list(b3,b2,b1)

    , so the pointer to b3() is the first and will be used.

    Then you might have the following scenario. Let's start with state when patches
    P1 and P2 are registered and enabled but the module M is not loaded. Then ftrace
    ops for b() does not exist. Then we get into the following race:

    CPU0 CPU1

    load_module(M)

    complete_formation()

    mod->state = MODULE_STATE_COMING;
    mutex_unlock(&module_mutex);

    klp_register_patch(P3);
    klp_enable_patch(P3);

    # STATE 1

    klp_module_notify(M)
    klp_module_notify_coming(P1);
    klp_module_notify_coming(P2);
    klp_module_notify_coming(P3);

    # STATE 2

    The ftrace ops for a() and b() then looks:

    STATE1:

    ops_a->func_stack -> list(a3,a2,a1);
    ops_b->func_stack -> list(b3);

    STATE2:
    ops_a->func_stack -> list(a3,a2,a1);
    ops_b->func_stack -> list(b2,b1,b3);

    therefore, b2() is used for the module but a3() is used for vmcore
    because they were the last added.

    Example of the race with going modules:
    =======================================

    CPU0 CPU1

    delete_module() #SYSCALL

    try_stop_module()
    mod->state = MODULE_STATE_GOING;

    mutex_unlock(&module_mutex);

    klp_register_patch()
    klp_enable_patch()

    #save place to switch universe

    b() # from module that is going
    a() # from core (patched)

    mod->exit();

    Note that the function b() can be called until we call mod->exit().

    If we do not apply patch against b() because it is in MODULE_STATE_GOING,
    it will call patched a() with modified semantic and things might get wrong.

    [jpoimboe@redhat.com: use one boolean instead of two]
    Signed-off-by: Petr Mladek
    Acked-by: Josh Poimboeuf
    Acked-by: Rusty Russell
    Signed-off-by: Jiri Kosina

    Petr Mladek
     

05 Mar, 2015

1 commit


03 Mar, 2015

1 commit

  • While one must hold RCU-sched (aka. preempt_disable) for find_symbol()
    one must equally hold it over the use of the object returned.

    The moment you release the RCU-sched read lock, the object can be dead
    and gone.

    [jkosina@suse.cz: change subject line to be aligned with other patches]
    Cc: Seth Jennings
    Cc: Josh Poimboeuf
    Cc: Masami Hiramatsu
    Cc: Miroslav Benes
    Cc: Petr Mladek
    Cc: Jiri Kosina
    Cc: "Paul E. McKenney"
    Cc: Rusty Russell
    Signed-off-by: Peter Zijlstra (Intel)
    Reviewed-by: Masami Hiramatsu
    Acked-by: Paul E. McKenney
    Acked-by: Josh Poimboeuf
    Signed-off-by: Jiri Kosina

    Peter Zijlstra
     

23 Feb, 2015

1 commit


19 Feb, 2015

1 commit

  • If registering the function with ftrace has previously succeeded,
    unregistering will almost never fail. Even if it does, it's not a fatal
    error. We can still carry on and disable the klp_func from being used
    by removing it from the klp_ops func stack.

    Signed-off-by: Josh Poimboeuf
    Reviewed-by: Miroslav Benes
    Reviewed-by: Petr Mladek
    Signed-off-by: Jiri Kosina

    Josh Poimboeuf
     

16 Feb, 2015

1 commit


07 Feb, 2015

1 commit


04 Feb, 2015

1 commit


21 Jan, 2015

3 commits

  • Fix a potentially uninitialized return value in klp_enable_func().

    Signed-off-by: Josh Poimboeuf
    Reviewed-by: Miroslav Benes
    Signed-off-by: Jiri Kosina

    Josh Poimboeuf
     
  • Add support for patching a function multiple times. If multiple patches
    affect a function, the function in the most recently enabled patch
    "wins". This enables a cumulative patch upgrade path, where each patch
    is a superset of previous patches.

    This requires restructuring the data a little bit. With the current
    design, where each klp_func struct has its own ftrace_ops, we'd have to
    unregister the old ops and then register the new ops, because
    FTRACE_OPS_FL_IPMODIFY prevents us from having two ops registered for
    the same function at the same time. That would leave a regression
    window where the function isn't patched at all (not good for a patch
    upgrade path).

    This patch replaces the per-klp_func ftrace_ops with a global klp_ops
    list, with one ftrace_ops per original function. A single ftrace_ops is
    shared between all klp_funcs which have the same old_addr. This allows
    the switch between function versions to happen instantaneously by
    updating the klp_ops struct's func_stack list. The winner is the
    klp_func at the top of the func_stack (front of the list).

    [ jkosina@suse.cz: turn WARN_ON() into WARN_ON_ONCE() in ftrace handler to
    avoid storm in pathological cases ]

    Signed-off-by: Josh Poimboeuf
    Reviewed-by: Jiri Slaby
    Signed-off-by: Jiri Kosina

    Josh Poimboeuf
     
  • Only allow the topmost patch on the stack to be enabled or disabled, so
    that patches can't be removed or added in an arbitrary order.

    Suggested-by: Jiri Kosina
    Signed-off-by: Josh Poimboeuf
    Reviewed-by: Jiri Slaby
    Signed-off-by: Jiri Kosina

    Josh Poimboeuf
     

20 Jan, 2015

1 commit


10 Jan, 2015

1 commit

  • When applying multiple patches to a module, if the module is loaded
    after the patches are loaded, the patches are applied in reverse order:

    $ insmod patch1.ko
    [ 43.172992] livepatch: enabling patch 'patch1'

    $ insmod patch2.ko
    [ 46.571563] livepatch: enabling patch 'patch2'

    $ modprobe nfsd
    [ 52.888922] livepatch: applying patch 'patch2' to loading module 'nfsd'
    [ 52.899847] livepatch: applying patch 'patch1' to loading module 'nfsd'

    Fix the loading order by storing the klp_patches list in queue order.

    Signed-off-by: Josh Poimboeuf
    Signed-off-by: Jiri Kosina

    Josh Poimboeuf
     

09 Jan, 2015

1 commit

  • We are aborting a build in case when gcc doesn't support fentry on x86_64
    (regs->ip modification can't really reliably work with mcount).

    This however breaks allmodconfig for people with older gccs that don't
    support -mfentry.

    Turn the build-time failure into runtime failure, resulting in the whole
    infrastructure not being initialized if CC_USING_FENTRY is unset.

    Reported-by: Andrew Morton
    Signed-off-by: Jiri Kosina
    Signed-off-by: Andrew Morton
    Acked-by: Josh Poimboeuf

    Jiri Kosina