dynup / kpatch

kpatch - live kernel patching
GNU General Public License v2.0
1.49k stars 305 forks source link

Outstanding clang issues for 6.2 #1340

Closed jpoimboe closed 1 year ago

jpoimboe commented 1 year ago

As reported by Joe in #1322:

Yulia's latest tests look good for gcc x86_64, s390x and ppc64le. Clang tests on x86_64 show two failures.

The first is related to the __fentry__ relocation type and the new __pfx_<func> nops:

create-diff-object: ERROR: cmdline.o: kpatch_create_mcount_sections: 3754: kpatch_print_message: unexpected instruction at the start of the function
cmdline.o: new function: kpatch_print_message

In kpatch_create_mcount_sections() , if the rela is R_X86_64_NONE, it assumes the beginning of the section will start the 0f 1f 44 00 00 nopl 0x0(%rax,%rax,1) sequence. For clang version 15.0.7 (Fedora 15.0.7-2.fc37), we see something like:

Disassembly of section .text.kpatch_print_message:

0000000000000000 <__pfx_kpatch_print_message>:
   0:   90                      nop
   1:   90                      nop
   2:   90                      nop
   3:   90                      nop
   4:   90                      nop
   5:   90                      nop
   6:   90                      nop
   7:   90                      nop
   8:   90                      nop
   9:   90                      nop
   a:   90                      nop
   b:   90                      nop
   c:   90                      nop
   d:   90                      nop
   e:   90                      nop
   f:   90                      nop

0000000000000010 <kpatch_print_message>:
  10:   0f 1f 44 00 00          nopl   0x0(%rax,%rax,1)
                        11: R_X86_64_NONE       __fentry__-0x4

So I think it will need to handle the __pfx_<func> nops case well and make the comparison at the real function (not section) start. I assume an older gcc compiler would generate the same code and relocation type as well?

repro-instr-start.tar.gz

Originally posted by @joe-lawrence in https://github.com/dynup/kpatch/issues/1322#issuecomment-1492651250

jpoimboe commented 1 year ago

The second clang failure occurs for .L.str[.X] symbols (added debugging):

create-diff-object: ERROR: af_netlink.o: find_local_syms: 191: couldn't find matching af_netlink.c local symbols in vmlinux symbol table
  locals_match failed for table_sym: .L.str
create-diff-object: ERROR: fork.o: find_local_syms: 191: couldn't find matching fork.c local symbols in vmlinux symbol table
  locals_match failed for table_sym: .L.str
create-diff-object: ERROR: aio.o: find_local_syms: 191: couldn't find matching aio.c local symbols in vmlinux symbol table
  locals_match failed for table_sym: .L.str
create-diff-object: ERROR: exit.o: find_local_syms: 191: couldn't find matching exit.c local symbols in vmlinux symbol table
  locals_match failed for table_sym: .L.str.33
create-diff-object: ERROR: sys.o: find_local_syms: 191: couldn't find matching sys.c local symbols in vmlinux symbol table
  locals_match failed for table_sym: .L.str.105
create-diff-object: ERROR: n_tty.o: find_local_syms: 191: couldn't find matching n_tty.c local symbols in vmlinux symbol table
  locals_match failed for table_sym: .L.str.6

These symbols appear in the vmlinux.symtab for their given FILE, but not in the individual original or patched object files:

$ grep 'fork\.c' vmlinux.symtab 
 14271: 0000000000000000     0 FILE    LOCAL  DEFAULT  ABS fork.c
 ...
 14464: ffffffff82804060    14 OBJECT  LOCAL  DEFAULT    3 .L.str
 14465: ffffffff82804058     0 NOTYPE  LOCAL  DEFAULT    3 .LCPI20_0

$ readelf --wide --symbols ~/repro-local_syms/fork.PATCHED.o | grep '.L.str'
$ readelf --wide --symbols ~/repro-local_syms/fork.ORIG.o | grep '.L.str'

Skipping these symbols in the table_sym check in locals_match() does appease the build.

github-actions[bot] commented 1 year ago

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

github-actions[bot] commented 1 year ago

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