Hi Josh,

This is an upstream reproducer of a downstream bug report from Ben.  He
encountered a sympos bug on rhel-10 with the same pwq_cache variable.
Here I distilled it down to minimal (upstream) reproducer config and
patch.

From what I can tell, function symbols already have a "HACK" disclaimer
around their workaround -- the attached patch tries to extend the same
idea to data symbols.  It may not have 100% coverage, and it's certainly
brittle as it assumes ordering as per the linker script.  But the idea
is to band-aid the problem until a more robust fix is ready.

The comment in find_sympos() suggests leveraging the checksum mechanism
as a better fix, but perhaps we could also pass the vmlinux file through
and read the ordering from the post-linked object?  I can work on that
for a v2, but in the meantime do you think this bandaid would be
sufficient for at least downstream use?


Config
======

Basic default x86 configuration and livepatch dependencies:

  $ make defconfig
  $ ./scripts/config --file .config \
             --set-val CONFIG_FTRACE y \
             --set-val CONFIG_KALLSYMS_ALL y \
             --set-val CONFIG_FUNCTION_TRACER y \
             --set-val CONFIG_DYNAMIC_FTRACE y \
             --set-val CONFIG_DYNAMIC_DEBUG y \
             --set-val CONFIG_LIVEPATCH y
  $ make olddefconfig


Base kernel and livepatch build
===============================

Build the reference kernel:

  $ make -j$(nproc)

and a minimal patch to fs/eventpoll.c :: ep_unregister_pollwait() to
provoke klp-relocation reference to pwq_cache:

  $ cat test.patch
  diff --git a/fs/eventpoll.c b/fs/eventpoll.c
  index 1234567..abcdefg 100644
  --- a/fs/eventpoll.c
  +++ b/fs/eventpoll.c
  @@ -684,6 +684,7 @@ static void ep_unregister_pollwait(struct eventpoll *ep, struct epitem *epi)
          while ((pwq = *p) != NULL) {
                  *p = pwq->next;
                  ep_remove_wait_queue(pwq);
  +               WARN_ON_ONCE(!pwq_cache);
                  kmem_cache_free(pwq_cache, pwq);
          }
   }

  $ ./scripts/livepatch/klp-build -T test.patch 
  Validating patch(es)
  Building original kernel
  Copying original object files
  Fixing patch(es)
  Building patched kernel
  Copying patched object files
  Generating original checksums
  Generating patched checksums
  Diffing objects
  vmlinux.o: new function: ep_unregister_pollwait.isra.0
  vmlinux.o: changed function: ep_remove
  vmlinux.o: changed function: ep_clear_and_put
  vmlinux.o: changed function: eventpoll_release_file
  Building patch module: livepatch-test.ko
  SUCCESS

sympos inspection
=================

klp-build assigns sympos=2 to the pwq_cache klp-relocation:

  $ readelf --wide --relocs livepatch-test.ko | grep -w '.klp.sym.vmlinux.pwq_cache'
  0000000000000008  0000006600000001 R_X86_64_64            0000000000000000 .klp.sym.vmlinux.pwq_cache,2 - 4
  0000000000000069  0000006600000002 R_X86_64_PC32          0000000000000000 .klp.sym.vmlinux.pwq_cache,2 - 4

but check out the ordering difference between vmlinux.o and the
post-linked vmlinux [sympos denoted]:

  vmlinux.o (symbol table order, what objtool uses):

    $ objdump -t vmlinux.o | awk '/\.c$/{file=$NF}; /pwq_cache/{print file": "$0}' | column -t
    workqueue.c:  0000000000012ae0  l  O  .bss                  0000000000000008  pwq_cache      << sympos=1
    eventpoll.c:  000000000000fb40  l  O  .data..ro_after_init  0000000000000008  pwq_cache      << sympos=2

  vmlinux (address order, what kallsyms uses *):

    $ objdump -t vmlinux | awk '/\.c$/{file=$NF}; /pwq_cache/{print file": "$0}' | column -t | sort -k2
    eventpoll.c:  ffffffff82b77b40  l  O  .rodata  0000000000000008  pwq_cache                   << sympos=1
    workqueue.c:  ffffffff83a85ae0  l  O  .bss     0000000000000008  pwq_cache                   << sympos=2
    * Note: .data..ro_after_init becomes .rodata after linking

With that, I think find_sympos() needs the same considerations for data
symbols as it currently has for function symbols.
 
Joe Lawrence (1):
  objtool/klp: fix symbol ordering for data symbols

 tools/objtool/klp-diff.c | 40 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 40 insertions(+)

-- 
2.54.0