linux-kernel - Re: [PATCH v10 00/10] livepatch: Atomic replace feature

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: <45877626-2d41-4af8-a8ff-5897585122ad@redhat.com>
Date:   Wed, 7 Mar 2018 16:55:53 -0500
From:   Joe Lawrence <joe.lawrence@...hat.com>
To:     Petr Mladek <pmladek@...e.com>, Jiri Kosina <jikos@...nel.org>,
        Josh Poimboeuf <jpoimboe@...hat.com>,
        Miroslav Benes <mbenes@...e.cz>
Cc:     Jason Baron <jbaron@...mai.com>, Jessica Yu <jeyu@...nel.org>,
        Evgenii Shatokhin <eshatokhin@...tuozzo.com>,
        live-patching@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v10 00/10] livepatch: Atomic replace feature

On 03/07/2018 03:20 AM, Petr Mladek wrote:
> The atomic replace allows to create cumulative patches. They
> are useful when you maintain many livepatches and want to remove
> one that is lower on the stack. In addition it is very useful when
> more patches touch the same function and there are dependencies
> between them.
> 
> 
> Changes against v9:
> 
>   + Fixed check of valid NOPs for already loaded objects,
>     regression introduced in v9 [Joe, Mirek]
>   + Allow to replace even disabled patches [Evgenii]
> 
> Changes against v8:
> 
>   + Fixed handling of statically defined struct klp_object
>     with empty array of functions [Joe, Mirek]
>   + Removed redundant func->new_func assignment for NOPs [Mirek]
>   + Improved some wording [Mirek]
>
> [ ... snip ... ]

Hi Petr,

I tried updating the test cases I was adding in "[PATCH v0 0/3]
additional cumulative livepatch doc/samples" and although one of the
cases is better than before, I'm running into a new issue:  an expected
pre-unpatch callback is not executed (its obj->patched is false).

Here's the updated test case:

Test 11
-------

- load livepatch
- load second livepatch (atomic replace)     <- callbacks ok
- disable second livepatch                   <- pre-unpatch skipped
- unload livepatch
- unload second livepatch

  % insmod samples/livepatch/livepatch-callbacks-demo.ko
  [ 2306.806046] livepatch: enabling patch 'livepatch_callbacks_demo'
  [ 2306.806048] livepatch: 'livepatch_callbacks_demo': initializing patching transition
  [ 2306.806083] livepatch_callbacks_demo: pre_patch_callback: vmlinux
  [ 2306.806083] livepatch: 'livepatch_callbacks_demo': starting patching transition
  [ 2307.743170] livepatch: 'livepatch_callbacks_demo': completing patching transition
  [ 2307.743317] livepatch_callbacks_demo: post_patch_callback: vmlinux
  [ 2307.743319] livepatch: 'livepatch_callbacks_demo': patching complet

  % insmod samples/livepatch/livepatch-callbacks-demo2.ko replace=1
  [ 2316.161804] livepatch: enabling patch 'livepatch_callbacks_demo2'
  [ 2316.161807] livepatch: 'livepatch_callbacks_demo2': initializing patching transition
  [ 2316.161842] livepatch_callbacks_demo2: pre_patch_callback: vmlinux
  [ 2316.161843] livepatch: 'livepatch_callbacks_demo2': starting patching transition
  [ 2317.727141] livepatch: 'livepatch_callbacks_demo2': completing patching transition
  [ 2317.727254] livepatch_callbacks_demo2: post_patch_callback: vmlinux
  [ 2317.727255] livepatch: 'livepatch_callbacks_demo2': patching complete

  % echo 0 > /sys/kernel/livepatch/livepatch_callbacks_demo2/enabled
  [ 2328.995854] livepatch: 'livepatch_callbacks_demo2': initializing unpatching transition
  [ 2328.995898] livepatch: 'livepatch_callbacks_demo2': starting unpatching transition
  [ 2330.719234] livepatch: 'livepatch_callbacks_demo2': completing unpatching transition
  [ 2330.719597] livepatch_callbacks_demo2: post_unpatch_callback: vmlinux
  [ 2330.719599] livepatch: 'livepatch_callbacks_demo2': unpatching complete

  % rmmod samples/livepatch/livepatch-callbacks-demo2.ko
  % rmmod samples/livepatch/livepatch-callbacks-demo.ko

Running against v10, callbacks seem to be good up until I disable an
atomic replace patch.  My understanding is that the original patch's
unpatch callbacks should be skipped (as they were).  I was surprised to
see that atomic replacement patch only ran it's post-unpatch callback.

Unfortunately I'm running out of time to further debug today, but
thought I would share these results.  I can dig in more tomorrow.

Regards,

-- Joe