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-next>] [day] [month] [year] [list]
Date:   Fri, 17 Jan 2020 16:03:00 +0100
From:   Petr Mladek <pmladek@...e.com>
To:     Jiri Kosina <jikos@...nel.org>,
        Josh Poimboeuf <jpoimboe@...hat.com>,
        Miroslav Benes <mbenes@...e.cz>
Cc:     Joe Lawrence <joe.lawrence@...hat.com>,
        Kamalesh Babulal <kamalesh@...ux.vnet.ibm.com>,
        Nicolai Stange <nstange@...e.de>,
        live-patching@...r.kernel.org, linux-kernel@...r.kernel.org,
        Petr Mladek <pmladek@...e.com>
Subject: [POC 00/23] livepatch: Split livepatch module per-object

Hi,

first, do not get scared by the size of the patchset. There are only
few patches that are really complicated and need attention at this
stage. I just wanted to split it as much as possible to review
and discuss each change separately.

Now to the problem. There are long term complains about maintainability
of the arch-specific code that is needed to livepatch modules that
are loaded after the livepatch itself.

There was always an idea about splitting the livepatch module
per-livepatched object. One interesting approach was drafted
on the last Livepatch microconference at Linux Plubmers 2019.

I played with the idea and came up with this POC. Of course,
there are pros and cons.

On the positive note:

    + The approach seems to work.

    + The same scenarios are supported. It is even newly possible to use
      the livepatch-specific relocations and reload the livepatched module.

    + The livepatch-specific relocations are still needed but
      they are handled together with other relocations. As
      a result, the other code modifications work out of box,
      e.g. alternatives, parainstructions.

    + Some problematic code could get removed (last 4 patches):

      + module_disable_ro()
      + arch_klp_init_object_loaded()
      + copy_module_elf()

    + The amount if livepatch-specific hooks in the module loader
      is about the same. They are _not_ longer arch-specific. But
      they are a bit tricky, see negatives below.


On the negative side:

    + It adds dependency on userspace tool "modprobe" called
      via usermodhelper. It brings several new problems:

       + How to distinguish modprobe called by user or by kernel
         when resolving races and errors.

       + How to pass the real error code to the usermodhelper caller.

       + Automatic dependencies are generated and handled in
         the userspace. Might create unwanted cyclic load.
	 It requires crazy workarounds from the kernel side,
	 see the patch 19.

     + There is a new bunch of races that sometimes need a tricky
       solution. For example, see the patches 8, 9, 15.

     + It might be slightly more complicated to prepare and use
       the livepatches. There are more modules that need to built
       and are visibly to the administrators. Also it complicates
       sharing some common helper functionality.


>From my point of view. The new code is much less arch-dependent
and more self-contained. Therefore it should be easier to maintain
in the long term.

On the other hand, it is more tricky regarding possible races and
infinite loops. They are not always easy to solve because of
"modprobe" called via userspace and because of more switches
between klp_mutex and module_mutex guarded code. Anyway, once
this is solved, it should just work for a long time as is.

All in all, I think that this approach is worth exploring.
I am curious about your opinion.

Best Regards,
Petr

PS: The patchset applies against Linus' master (v5.5-rc6).

Petr Mladek (23):
  module: Allow to delete module also from inside kernel
  livepatch: Split livepatch modules per livepatched object
  livepatch: Better checks of struct klp_object definition
  livepatch: Prevent loading livepatch sub-module unintentionally.
  livepatch: Initialize and free livepatch submodule
  livepatch: Enable the livepatch submodule
  livepatch: Remove obsolete functionality from klp_module_coming()
  livepatch: Automatically load livepatch module when the patch module
    is loaded
  livepatch: Handle race when livepatches are reloaded during a module
    load
  livepatch: Handle modprobe exit code
  livepatch: Safely detect forced transition when removing split
    livepatch modules
  livepatch: Automatically remove livepatch module when the object is
    freed
  livepatch: Remove livepatch module when the livepatched module is
    unloaded
  livepatch: Never block livepatch modules when the related module is
    being removed
  livepatch: Prevent infinite loop when loading livepatch module
  livepatch: Add patch into the global list early
  livepatch: Load livepatches for modules when loading  the main
    livepatch
  module: Refactor add_unformed_module()
  module/livepatch: Allow to use exported symbols from livepatch module
    for "vmlinux"
  module/livepatch: Relocate local variables in the module loaded when
    the livepatch is being loaded
  livepatch: Remove obsolete arch_klp_init_object_loaded()
  livepatch/module: Remove obsolete copy_module_elf()
  module: Remove obsolete module_disable_ro()

 Documentation/livepatch/module-elf-format.rst      |  15 +-
 arch/x86/kernel/Makefile                           |   1 -
 arch/x86/kernel/livepatch.c                        |  53 --
 include/linux/livepatch.h                          |  36 +-
 include/linux/module.h                             |  10 +-
 kernel/livepatch/core.c                            | 743 ++++++++++++++-------
 kernel/livepatch/core.h                            |   5 -
 kernel/livepatch/transition.c                      |  22 +-
 kernel/module.c                                    | 279 ++++----
 lib/livepatch/Makefile                             |   2 +
 lib/livepatch/test_klp_atomic_replace.c            |  18 +-
 lib/livepatch/test_klp_callbacks_demo.c            |  90 ++-
 lib/livepatch/test_klp_callbacks_demo.h            |  11 +
 lib/livepatch/test_klp_callbacks_demo2.c           |  62 +-
 lib/livepatch/test_klp_callbacks_demo2.h           |  11 +
 ...t_klp_callbacks_demo__test_klp_callbacks_busy.c |  50 ++
 ...st_klp_callbacks_demo__test_klp_callbacks_mod.c |  42 ++
 lib/livepatch/test_klp_livepatch.c                 |  18 +-
 lib/livepatch/test_klp_state.c                     |  53 +-
 lib/livepatch/test_klp_state2.c                    |  53 +-
 samples/livepatch/Makefile                         |   4 +
 samples/livepatch/livepatch-callbacks-demo.c       |  90 ++-
 samples/livepatch/livepatch-callbacks-demo.h       |  11 +
 ...h-callbacks-demo__livepatch-callbacks-busymod.c |  54 ++
 ...patch-callbacks-demo__livepatch-callbacks-mod.c |  46 ++
 samples/livepatch/livepatch-sample.c               |  18 +-
 samples/livepatch/livepatch-shadow-fix1.c          | 120 +---
 .../livepatch-shadow-fix1__livepatch-shadow-mod.c  | 155 +++++
 samples/livepatch/livepatch-shadow-fix2.c          |  92 +--
 .../livepatch-shadow-fix2__livepatch-shadow-mod.c  | 127 ++++
 .../testing/selftests/livepatch/test-callbacks.sh  |  19 +-
 31 files changed, 1424 insertions(+), 886 deletions(-)
 delete mode 100644 arch/x86/kernel/livepatch.c
 create mode 100644 lib/livepatch/test_klp_callbacks_demo.h
 create mode 100644 lib/livepatch/test_klp_callbacks_demo2.h
 create mode 100644 lib/livepatch/test_klp_callbacks_demo__test_klp_callbacks_busy.c
 create mode 100644 lib/livepatch/test_klp_callbacks_demo__test_klp_callbacks_mod.c
 create mode 100644 samples/livepatch/livepatch-callbacks-demo.h
 create mode 100644 samples/livepatch/livepatch-callbacks-demo__livepatch-callbacks-busymod.c
 create mode 100644 samples/livepatch/livepatch-callbacks-demo__livepatch-callbacks-mod.c
 create mode 100644 samples/livepatch/livepatch-shadow-fix1__livepatch-shadow-mod.c
 create mode 100644 samples/livepatch/livepatch-shadow-fix2__livepatch-shadow-mod.c

-- 
2.16.4

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ