On Wed, Jan 25, 2023 at 1:58 AM Christophe Leroy
[off-list ref] wrote:

Le 21/01/2023 à 01:49, Song Liu a écrit :

quoted

Josh reported a bug:

   When the object to be patched is a module, and that module is
   rmmod'ed and reloaded, it fails to load with:

   module: x86/modules: Skipping invalid relocation target, existing value is nonzero for type 2, loc 00000000ba0302e9, val ffffffffa03e293c
   livepatch: failed to initialize patch 'livepatch_nfsd' for module 'nfsd' (-8)
   livepatch: patch 'livepatch_nfsd' failed for module 'nfsd', refusing to load module 'nfsd'

   The livepatch module has a relocation which references a symbol
   in the _previous_ loading of nfsd. When apply_relocate_add()
   tries to replace the old relocation with a new one, it sees that
   the previous one is nonzero and it errors out.

He also proposed three different solutions. We could remove the error
check in apply_relocate_add() introduced by commit eda9cec4c9a1
("x86/module: Detect and skip invalid relocations"). However the check
is useful for detecting corrupted modules.

We could also deny the patched modules to be removed. If it proved to be
a major drawback for users, we could still implement a different
approach. The solution would also complicate the existing code a lot.

We thus decided to reverse the relocation patching (clear all relocation
targets on x86_64). The solution is not
universal and is too much arch-specific, but it may prove to be simpler
in the end.

Would it be possible to not add empty clear_relocate_add() on all
architecture while only x86 implements it ? Can we make it an empty
static inline in generic livepatch.h for the architectures not
implementing it, like we do for most mm functions ?

Another solution would be to define an empty generic weak version of
clear_relocate_add()

Sure, I will replace these with a week function.

Thanks,
Song