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

create-diff-object: check NOP-padded instruction when symbol missing #1374

Closed
wants to merge 1 commit into from

Conversation

anatasluo
Copy link
Contributor

commit 3e54c63 has added support for x86 NOP-padded functions.

However, it works only when pfx symbol exists.

In fedora 39 with kernel 6.7.4-200.fc39.x86_64.
I met the following error:
kpatch_bundle_symbols: 251: symbol uptime_proc_show at offset 16 within section .text.uptime_proc_show, expected 0.

Problem here is, the symbol table of uptime.o has no __pfx symbol.

To solve this problem, we can add a check if the padding area is filled with nop instructions.

commit 3e54c63 has added support for x86
NOP-padded functions.

However, it works only when pfx symbol exists.

In fedora 39 with kernel 6.7.4-200.fc39.x86_64.
I met the following error:
kpatch_bundle_symbols: 251: symbol uptime_proc_show at
offset 16 within section .text.uptime_proc_show, expected 0.

Problem here is, the symbol table of uptime.o has no __pfx symbol.

To solve this problem, we can add a check if the padding area
is filled with nop instructions.

Signed-off-by: Longjun Luo <[email protected]>
@anatasluo
Copy link
Contributor Author

anatasluo commented Feb 20, 2024

Hi guys, I met a problem when I used the kpatch.

About environment information:

➜  kpatch git:(master) ✗ cat /etc/os-release 
NAME="Fedora Linux"
VERSION="39 (Workstation Edition)"
ID=fedora
VERSION_ID=39
VERSION_CODENAME=""
PLATFORM_ID="platform:f39"
PRETTY_NAME="Fedora Linux 39 (Workstation Edition)"
ANSI_COLOR="0;38;2;60;110;180"
LOGO=fedora-logo-icon
CPE_NAME="cpe:/o:fedoraproject:fedora:39"
DEFAULT_HOSTNAME="fedora"
HOME_URL="https://fedoraproject.org/"
DOCUMENTATION_URL="https://docs.fedoraproject.org/en-US/fedora/f39/system-administrators-guide/"
SUPPORT_URL="https://ask.fedoraproject.org/"
BUG_REPORT_URL="https://bugzilla.redhat.com/"
REDHAT_BUGZILLA_PRODUCT="Fedora"
REDHAT_BUGZILLA_PRODUCT_VERSION=39
REDHAT_SUPPORT_PRODUCT="Fedora"
REDHAT_SUPPORT_PRODUCT_VERSION=39
SUPPORT_END=2024-11-12
VARIANT="Workstation Edition"
VARIANT_ID=workstation
➜  kpatch git:(master) ✗ uname -r
6.7.4-200.fc39.x86_64

The build log:

ld: warning: orphan section `.bss.kobj_ns_type_lock' from `vmlinux.o' being placed in section `.bss.kobj_ns_type_lock'
ld: warning: orphan section `.bss.uevent_seqnum' from `vmlinux.o' being placed in section `.bss.uevent_seqnum'
ld: warning: orphan section `.bss.maple_node_cache' from `vmlinux.o' being placed in section `.bss.maple_node_cache'
ld: warning: orphan section `.bss.backtrace_idle' from `vmlinux.o' being placed in section `.bss.backtrace_idle'
ld: warning: orphan section `.bss.backtrace_flag' from `vmlinux.o' being placed in section `.bss.backtrace_flag'
ld: warning: orphan section `.bss.radix_tree_node_cachep' from `vmlinux.o' being placed in section `.bss.radix_tree_node_cachep'
ld: warning: orphan section `.bss.copy_mc_fragile_key' from `vmlinux.o' being placed in section `.bss.copy_mc_fragile_key'
ld: warning: orphan section `.bss.pc_conf_lock' from `vmlinux.o' being placed in section `.bss.pc_conf_loc
k'
  NM      System.map
  SORTTAB vmlinux
  RELOCS  arch/x86/boot/compressed/vmlinux.relocs
  RSTRIP  vmlinux
create-diff-object: ERROR: uptime.o: kpatch_bundle_symbols: 251: symbol uptime_proc_show at offset 16 with
in section .text.uptime_proc_show, expected 0

My patch file:

From f2473a482cb18862acbe6a46de2b85149b6f4188 Mon Sep 17 00:00:00 2001
From: anatasluo <[email protected]>
Date: Thu, 15 Feb 2024 16:20:03 +0800
Subject: [PATCH] patch test

---
 fs/proc/uptime.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/fs/proc/uptime.c b/fs/proc/uptime.c
index b5343d209..596fd9569 100644
--- a/fs/proc/uptime.c
+++ b/fs/proc/uptime.c
@@ -30,6 +30,7 @@ static int uptime_proc_show(struct seq_file *m, void *v)
 
 	idle.tv_sec = div_u64_rem(idle_nsec, NSEC_PER_SEC, &rem);
 	idle.tv_nsec = rem;
+	seq_printf(m, "livepatch works \n");
 	seq_printf(m, "%lu.%02lu %lu.%02lu\n",
 			(unsigned long) uptime.tv_sec,
 			(uptime.tv_nsec / (NSEC_PER_SEC / 100)),
-- 
2.43.0

My kpatch command:

./kpatch-build/kpatch-build -d ~/0001-patch-test.patch

@anatasluo
Copy link
Contributor Author

Here is the symbol table of uptime.o:

[anatasluo@localhost .kpatch]$ readelf -sW ./tmp/orig/fs/proc/uptime.o 

Symbol table '.symtab' contains 33 entries:
   Num:    Value          Size Type    Bind   Vis      Ndx Name
     0: 0000000000000000     0 NOTYPE  LOCAL  DEFAULT  UND 
     1: 0000000000000000     0 FILE    LOCAL  DEFAULT  ABS uptime.c
     2: 0000000000000000     0 NOTYPE  LOCAL  DEFAULT    4 __initcall__kmod_proc__275_49_proc_uptime_init5
     3: 0000000000000010    49 FUNC    LOCAL  DEFAULT    7 proc_uptime_init
     4: 0000000000000000     0 SECTION LOCAL  DEFAULT    6 .rodata.str1.1
     5: 0000000000000000     0 SECTION LOCAL  DEFAULT    7 .init.text
     6: 0000000000000010   354 FUNC    LOCAL  DEFAULT   14 uptime_proc_show
     7: 0000000000000000     0 SECTION LOCAL  DEFAULT   13 .rodata.uptime_proc_show.str1.1
     8: 0000000000000000     0 SECTION LOCAL  DEFAULT   14 .text.uptime_proc_show
     9: 0000000000000000     0 SECTION LOCAL  DEFAULT   18 .discard.addressable
    10: 0000000000000000     8 OBJECT  LOCAL  DEFAULT   18 __UNIQUE_ID___addressable_proc_uptime_init276
    11: 0000000000000000     0 SECTION LOCAL  DEFAULT   20 .debug_info
    12: 0000000000000000     0 SECTION LOCAL  DEFAULT   22 .debug_abbrev
    13: 0000000000000000     0 SECTION LOCAL  DEFAULT   23 .debug_loclists
    14: 0000000000000000     0 SECTION LOCAL  DEFAULT   27 .debug_rnglists
    15: 0000000000000000     0 SECTION LOCAL  DEFAULT   29 .debug_line
    16: 0000000000000000     0 SECTION LOCAL  DEFAULT   31 .debug_str
    17: 0000000000000000     0 SECTION LOCAL  DEFAULT   32 .debug_line_str
    18: 0000000000000000     0 SECTION LOCAL  DEFAULT   36 .debug_frame
    19: 0000000000000000     0 NOTYPE  GLOBAL DEFAULT  UND __fentry__
    20: 0000000000000000     0 NOTYPE  GLOBAL DEFAULT  UND proc_create_single_data
    21: 0000000000000000     0 NOTYPE  GLOBAL DEFAULT  UND __x86_return_thunk
    22: 0000000000000000     0 NOTYPE  GLOBAL DEFAULT  UND kcpustat_cpu_fetch
    23: 0000000000000000     0 NOTYPE  GLOBAL DEFAULT  UND get_idle_time
    24: 0000000000000000     0 NOTYPE  GLOBAL DEFAULT  UND nr_cpu_ids
    25: 0000000000000000     0 NOTYPE  GLOBAL DEFAULT  UND __cpu_possible_mask
    26: 0000000000000000     0 NOTYPE  GLOBAL DEFAULT  UND _find_next_bit
    27: 0000000000000000     0 NOTYPE  GLOBAL DEFAULT  UND ktime_get_with_offset
    28: 0000000000000000     0 NOTYPE  GLOBAL DEFAULT  UND ns_to_timespec64
    29: 0000000000000000     0 NOTYPE  GLOBAL DEFAULT  UND pcpu_hot
    30: 0000000000000000     0 NOTYPE  GLOBAL DEFAULT  UND set_normalized_timespec64
    31: 0000000000000000     0 NOTYPE  GLOBAL DEFAULT  UND seq_printf
    32: 0000000000000000     0 NOTYPE  GLOBAL DEFAULT  UND __stack_chk_fail

The problem here is the symbol table doesn't have __pfx symbol, so commit 3e54c63 can't handle it.

@jpoimboe
Copy link
Member

Thanks for the patch! This is similar to an issue we've discused in #1320. Currently kpatch-build doesn't support running on a .o file that hasn't yet been processed by objtool. That can happen for certain config options (IBT, LTO) for which objtool runs at link time, on vmlinux.o, instead of on each individual translation unit. I'm working on a complete solution to this now.

Copy link

This PR has been open for 60 days with no activity and no assignee. It will be closed in 7 days unless a comment is added.

@github-actions github-actions bot added the stale label Apr 21, 2024
Copy link

This PR was closed because it was inactive for 7 days after being marked stale.

@github-actions github-actions bot closed this Apr 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants