link-vmlinux-syms doesn't handle symbol collisions properly about kpatch HOT 10 CLOSED

This issue is actually hard to resolve. create-diff-object changes unchanged LOCAL symbols to GLOBAL symbols with section index UNDEF so that they can be linked to the version in vmlinux. However, this introduces the chance that the local symbol name collides with an existing global symbol in the kernel and we can't determine, from link-vmlinux-syms point of view, which symbol was being referenced since it has no FILE context as LOCAL symbols do.

In the meantime, commit 2be6178 checks for this ambiguity and errors out of the situation above is ever encountered.

from kpatch.

I attempted to recreate this issue, but strangely got another problem. I used the following patch:

Index: src/mm/ksm.c
===================================================================
--- src.orig/mm/ksm.c
+++ src/mm/ksm.c
@@ -2216,6 +2216,10 @@ static ssize_t pages_to_scan_store(struc
 {
    int err;
    unsigned long nr_pages;
+   char buf2[64];
+
+   pages_to_scan_show(kobj, attr, buf2);
+   printk("pages_to_scan_show says %s\n", buf2);

    err = kstrtoul(buf, 10, &nr_pages);
    if (err || nr_pages > UINT_MAX)

I expected an error, but instead it succeeded, and strangely pages_to_scan_show doesn't show up in the symbol table.

from kpatch.

Looking at the rela table for pages_to_scan_store with the patch applied:

Relocation section [30] '.rela.text.pages_to_scan_store' for section [29] '.text.pages_to_scan_store' at offset 0x8868 contains 9 entries:
  Offset              Type            Value               Addend Name
  0x0000000000000001  X86_64_NONE     000000000000000000      -4 __fentry__
  0x0000000000000009  X86_64_32S      000000000000000000      +0 .rodata.str1.1
  0x0000000000000023  X86_64_PC32     000000000000000000      -4 .data.ksm_thread_pages_to_scan
  0x0000000000000037  X86_64_PC32     000000000000000000      -4 sprintf
  0x0000000000000042  X86_64_32S      000000000000000000     +14 .rodata.str1.1
  0x0000000000000049  X86_64_PC32     000000000000000000      -4 printk
  0x000000000000005a  X86_64_PC32     000000000000000000      -4 kstrtoull
  0x0000000000000072  X86_64_PC32     000000000000000000      -4 .data.ksm_thread_pages_to_scan
  0x00000000000000a2  X86_64_PC32     000000000000000000      -4 __stack_chk_fail

Looks like the compiler is inlining pages_to_scan_show.

from kpatch.

And the patch does work :)

# echo 100 > pages_to_scan 
# dmesg
[17771.153314] pages_to_scan_show says 100

from kpatch.

Blasted compiler.

from kpatch.

/me wonders how to get gcc to not inline an otherwise not inlined function.

from kpatch.

I'm playing around with using -fno-inline, in addition to -f[function|data]-sections in the KCFLAGS for building the diff objects.

from kpatch.

This issue is as fixed as it can be right now, see commit 2be6178

from kpatch.

This may be lower priority, but let's leave it open since it helps us keep track of a real issue that will need to be fixed at some point. I'm not sure that changing unchanged LOCAL symbols to GLOBAL undef symbols in create-diff-object is the best approach.

from kpatch.

With the addition of dynrela support, link-vmlinux-syms doesn't exist anymore and the issue has disappeared because we no longer do the change-symbol-binding trick for pinning addresses into the symbol table.

from kpatch.