Re: [PATCH v10 02/15] livepatch: avoid position-based search if `-z unique-symbol` is available

[Josh Poimboeuf <jpoimboe@redhat.com>]

On Wed, Feb 16, 2022 at 03:32:41PM -0500, Joe Lawrence wrote:
> > Right, so we'd have to abandon position-based search in favor of
> > file+func based search.
> > 
> > It's not perfect because there are still a few file+func duplicates.
> > But it might be good enough.  We would presumably just refuse to patch a
> > duplicate.  Or we could remove them (and enforce their continued removal
> > with tooling-based warnings).
> > 
> 
> You're talking about duplicate file+func combinations as stored in the
> symbol table?

Right.

> ...
>       6 OBJECT core.c::__func__.3
>       6 OBJECT core.c::__func__.5
>       7 OBJECT core.c::__func__.1
>       8 OBJECT core.c::__func__.0
>       8 OBJECT core.c::__func__.2
> 
> We could probably minimize the FUNC duplicates with unique names, but
> I'm not as optimistic about the OBJECTs as most are created via macros
> like __already_done.X.  Unless clever macro magic?

Good point about objects, as we rely on disambiguating them for klp
relocations.  Luckily, the fact that most of them are created by macros
is largely a good thing.  We consider most of those to be "special"
static locals, which don't actually need to be correlated or referenced
with a klp reloc.

For example:

- '__func__' is just the function name.  The patched function shouldn't
  need to reference the original function's function name string.

- '__already_done' is used for printk_once(); no harm in making a new
  variable initialized to false and printing it again; or converting
  printk_once() to just printk() to avoid an extra print.

- '__key' is used by lockdep to track lock usage and validate locking
  order.  It probably makes sense to use a new key in the patched
  function, since the new function might have different locking
  behavior.

> Next question: what are the odds that these entries, at least the ones
> we can't easily rename, need disambiguity for livepatching?  or
> kpatch-build for related purposes?

I would guess the odds are rather low, given the fact that there are so
few functions, and we don't care about most of the objects on the list.

If duplicates were to become problematic then we could consider adding
tooling which warns on a duplicate file:sym pair with the goal of
eliminating duplicates (exculding the "special" objects).

-- 
Josh