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

changes to .kprobes.text not detected #150

Closed
jpoimboe opened this issue Apr 29, 2014 · 2 comments · Fixed by #151
Closed

changes to .kprobes.text not detected #150

jpoimboe opened this issue Apr 29, 2014 · 2 comments · Fixed by #151

Comments

@jpoimboe
Copy link
Member

With the following patch, kpatch-build doesn't detect any changes:

~ $ cat ~/nmi.patch 
Index: src/arch/x86/kernel/nmi.c
===================================================================
--- src.orig/arch/x86/kernel/nmi.c
+++ src/arch/x86/kernel/nmi.c
@@ -518,6 +518,8 @@ do_nmi(struct pt_regs *regs, long error_

    inc_irq_stat(__nmi_count);

+   printk("nmi\n");
+
    if (!ignore_nmis)
        default_do_nmi(regs);

~ $ kpatch-build -d nmi.patch 
DEBUG mode enabled
Using cache at /home/jpoimboe/.kpatch/3.13.10-200.fc20.x86_64/src
Testing patch file
patching file arch/x86/kernel/nmi.c
Building original kernel
Building patched kernel
Detecting changed objects
Rebuilding changed objects
Extracting new and modified ELF sections
nmi.o: no changed functions were found
ERROR: kpatch build failed. Check /tmp/kpatch-build-1398776924.log for more details.

I think it's because do_nmi() is annotated with __kprobes which places it in the .kprobes.text section. But I'd expect the inclusion logic to catch the fact that .kprobes.text changed and report an error.

@jpoimboe
Copy link
Member Author

Hm, maybe the "no changed functions were found" check happens before the "some unexpected section changed" check?

@jpoimboe
Copy link
Member Author

Yeah, with a bigger patch that changes another non-kprobes function, I get the error I expected:

~ $ cat nmi.patch 
Index: src/arch/x86/kernel/nmi.c
===================================================================
--- src.orig/arch/x86/kernel/nmi.c
+++ src/arch/x86/kernel/nmi.c
@@ -146,6 +146,8 @@ int __register_nmi_handler(unsigned int
    if (!action->handler)
        return -EINVAL;

+   printk("a\n");
+
    spin_lock_irqsave(&desc->lock, flags);

    /*
@@ -518,6 +520,8 @@ do_nmi(struct pt_regs *regs, long error_

    inc_irq_stat(__nmi_count);

+   printk("nmi\n");
+
    if (!ignore_nmis)
        default_do_nmi(regs);

~ $ kpatch-build nmi.patch 
Using cache at /home/jpoimboe/.kpatch/3.13.10-200.fc20.x86_64/src
Testing patch file
patching file arch/x86/kernel/nmi.c
Building original kernel
Building patched kernel
Detecting changed objects
Rebuilding changed objects
Extracting new and modified ELF sections
nmi.o: changed function: __register_nmi_handler
nmi.o: changed function: unregister_nmi_handler
nmi.o: changed section .kprobes.text not selected for inclusion
/usr/local/libexec/kpatch/create-diff-object: unreconcilable difference
ERROR: kpatch build failed. Check /tmp/kpatch-build-1398785999.log for more details.

jpoimboe added a commit to jpoimboe/kpatch that referenced this issue Apr 29, 2014
If a patch changes a single function which is in a special section that
we don't support, create-diff-object reports "no changed functions were
found".  Give a clearer error message in that case, by checking
reachability errors before unchanged errors and by printing all
reachability errors errors instead of the first one it encounters.

Fixes dynup#150.
jpoimboe added a commit to jpoimboe/kpatch that referenced this issue Apr 29, 2014
If a patch changes a single function which is in a special section that
we don't support, create-diff-object reports "no changed functions were
found".  Give a clearer error message in that case, by checking
reachability errors before unchanged errors and by printing all
reachability errors errors instead of the first one it encounters.

Fixes dynup#150.
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 a pull request may close this issue.

1 participant