Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

kpatch-build: use original vmlinux #192

Merged
merged 1 commit into from
May 17, 2014
Merged

Conversation

jpoimboe
Copy link
Member

There's at least one case in the kernel (ddebug_proc_show) where the
compiled instructions are affected by the source file path given to gcc.
Which means that compiling the kernel with O= will result in many of the
function addresses changing. This causes a mismatch between the locally
compiled vmlinux and the original vmlinux, which is very dangerous,
since we need the addresses to be correct.

The easy fix is just to use the original vmlinux for all the function
addresses.

Other potential ways to fix it which we might want to consider in the
future:

  • use a combination of the old System.map and the new vmlinux to find
    the addresses. The function ordering should be the same. For
    non-duplicate symbols, use System.map. For duplicate symbols, use
    vmlinux to find what order the symbol comes in. e.g. the 2nd
    occurrence of foo() in System.map. This would be my pick. It adds a
    little complexity to the lookup code, but seems safe and wouldn't
    require the kernel debuginfo package.
  • do something similar at runtime, i.e. use kallsyms_lookup_name for
    non-dups and kallsyms_on_each_symbol for dups, and look for the nth
    occurrence of the symbol (value of n is decided at build time). This
    has the complexity of the previous option but it's done at runtime
    rather than build time, so... why? Doing it at build time is better.
  • compile the kernel in place. This basically means no more caching
    because recompiling with --function-sections causes everything to be
    recompiled again. This is bad for kpatch developers' SSDs...

There's at least one case in the kernel (ddebug_proc_show) where the
compiled instructions are affected by the source file path given to gcc.
Which means that compiling the kernel with O= will result in many of the
function addresses changing.  This causes a mismatch between the locally
compiled vmlinux and the original vmlinux, which is very dangerous,
since we need the addresses to be correct.

The easy fix is just to use the original vmlinux for all the function
addresses.

Other potential ways to fix it which we might want to consider in the
future:

- use a combination of the old System.map and the new vmlinux to find
  the addresses.  The function ordering should be the same.  For
  non-duplicate symbols, use System.map.  For duplicate symbols, use
  vmlinux to find what order the symbol comes in.  e.g. the 2nd
  occurrence of foo() in System.map.  It adds a little complexity to the
  lookup code, but seems safe and wouldn't require the kernel debuginfo
  package.  However, this may not help us for patching modules.

- do something similar at runtime, i.e. use kallsyms_lookup_name for
  non-dups and kallsyms_on_each_symbol for dups, and look for the nth
  occurrence of the symbol (value of n is decided at build time).  This
  has the complexity of the previous option but it's done at runtime
  rather than build time, so... why?  Doing it at build time is better.

- compile the kernel in place.  This basically means no more caching
  because recompiling with --function-sections causes everything to be
  recompiled again.  This is bad for kpatch developers' SSDs...
@jpoimboe
Copy link
Member Author

@Spartacus06 I'm going to go ahead and merge this since I suspect it's causing issue #194. I also verified that it fixes the failing integration test.

jpoimboe added a commit that referenced this pull request May 17, 2014
kpatch-build: use original vmlinux
@jpoimboe jpoimboe merged commit 0f3f8ae into dynup:master May 17, 2014
@jpoimboe jpoimboe deleted the vmlinux-mismatch branch May 29, 2014 18:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant