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

What do I need to do to support arm64? #1216

Closed
keiya-nobuta opened this issue Aug 31, 2021 · 11 comments
Closed

What do I need to do to support arm64? #1216

keiya-nobuta opened this issue Aug 31, 2021 · 11 comments
Labels

Comments

@keiya-nobuta
Copy link
Contributor

Discussions about enabling Live patch on arm64 are underway on the Linux mailing list:

In addition to the above patches, it is said that the following changes are still needed:

  • Reworking for making patching safe on arm64.
  • Objtool (or alternative method) for stack validation. (Is this also needed by kpatch?)

So Live patch for arm64 is still a little ahead, but I think kpatch will need to support arm64 in the future.

What should I start with if I'm trying to port for arm64?
(Should I ask to reopen the pull-request [#1010] ?)

Thanks

@joe-lawrence
Copy link
Contributor

Hi @keiya-nobuta

Yes, the upstream livepatching effort for arm64 is definitely gaining steam. In fact, Madhavan T. Venkataraman recently inquired about using kpatch on arm64. (I'll point him here to your interest as well.)

As far the additional livepatching work that remains outstanding, we would need those to be implemented prior to declaring kpatch support. This is mostly since the old kpatch.ko that #1010 updated is deprecated. Kpatch relies on upstreamed (kernel livepatch) code now.

Additional thoughts:

  • The recent S390x kpatch support #1203 looks to be a reasonable reference for adding a new arch. The PR notes any prerequisite kernel and gcc patches, and then the commits build up support in the tooling and finally enable it all at the end.
  • A good starting point would be to get the integration tests to build/load/function as expected. For example, the cmdline-string.patch is the canonical demo patch.
  • I'm in the middle of rebasing the integration tests for rhel-9 beta, which will be based on v5.14... that effort is nearly done, I'm still working out some build issues on ppc64le. I can update this thread with a branch reference when that's ready.
  • Lastly, I would think it would be possible to develop kpatch support in parallel with the upstream kernel code. Madhavan can better answer what is and what isn't yet implemented. For example, it might be possible to start on create-diff-object and friends before livepatching is completely safe. Better yet, the two efforts may help drive the other (ie, kpatch to test some real-world livepatches).

@keiya-nobuta
Copy link
Contributor Author

Hi @joe-lawrence ,

Thank you for the information.
OK, I will first investigate #1203 and create-diff-object.

@keiya-nobuta
Copy link
Contributor Author

keiya-nobuta commented Oct 15, 2021

Hi.
I'm trying to support create-diff-object and friends for arm64, working on my branch. This branch based on #1203.

I haven't fully understood it yet, but for now I'll write what I know:

$(CC_FLAGS_FTRACE) on arm64:
am64 using -fpatchable-function-entry=2 instead of -pg -mrecord-mcount -mfentry, it has the following characteristics:

  • 2 nop instructions are inserted at the beginning of the function, a symbol such as fentry and mcount is not recorded to its relocation section.
  • The position that nop is inserted is recorded to __patchable_function_entries section, this section has almost the same role as __mcount_loc section.

So for arm64, the functions that can be profiled are findable with symbols information in __patchable_function_entries section and its relocation section.

CONFIG_ARM64_BTI_KERNEL:
If this configuration is enabled, -mbranch-protection=pac-ret+leaf+bti is added and bti instruction is inserted before nop.
This should be taken into consideration in kpatch_create_mcount_sections().

CONFIG_PARAVIRT:

  • arm64 has CONFIG_PARAVIRT but has no struct paravirt_patch_site. This check can be skipped.

Special Sections:

  • arm64 also has .altinstructions section.

Mapping Symbols:

  • arm64 ELF format may be inserted that special symbols '$d' and '$x. These symbols can not correlate.

Skip files:

  • The objects that in arch/arm64/kernel/vdso/*.o are not build with CC_FLAGS_FTRACE. So they are not patchable, can add to skip list. This can be also confirmed by the kernel build results, that output to arch/arm64/kernel/vdso/.*.cmd files.

Currently, I'm trying implement kpatch_line_macro_change_only() for arm64, this implementation based on the implementation of ppc64.

@keiya-nobuta
Copy link
Contributor Author

keiya-nobuta commented Oct 15, 2021

@joe-lawrence
Copy link
Contributor

Hi @keiya-nobuta : that sounds like progress. Are you currently testing against any of the integration tests, and then if so, loading the resulting kpatch on a kernel with Madhavan's patchset?

@keiya-nobuta
Copy link
Contributor Author

Hi @joe-lawrence,

My testing kernel is based on v5.15-rc1, including Madhavan's patchset (v8) and Suraj's, with a few fixes.
I'm still in the trial-and-error stage, and I haven't done much testing.

I tried cmdline-string.patch for the time begin, it seems to work:

ubuntu@ubuntu:~/github/keiya-nobuta/kpatch$ kpatch-build/kpatch-build -d -s $KERNEL_SRC -v $KERNEL_SRC/vmlinux cmdline-string.patch
DEBUG mode enabled
Skipping cleanup
+ [[ -n '' ]]
[...]
+ [[ 1 -eq 0 ]]
+ echo SUCCESS
SUCCESS
ubuntu@ubuntu:~/github/keiya-nobuta/kpatch$ sudo kpatch/kpatch load livepatch-cmdline-string.ko
loading patch module: livepatch-cmdline-string.ko
waiting (up to 15 seconds) for patch transition to complete...
transition complete (2 seconds)
ubuntu@ubuntu:~/github/keiya-nobuta/kpatch$
ubuntu@ubuntu:~/github/keiya-nobuta/kpatch$ cat /proc/cmdline
  dma.dmachans=0x37f5 bcm2709.boardrev=0xd03114 bcm2709.serial=0xff852c78 bcm2709.uart_clock=48000000 bcm2709.disk_led_gpio=42 bcm2709.disk_led_active_low=0 smsc95xx.macaddr=DC:A6:32:E7:7A:AC vc_mem.mem_base=0x3ec00000 vc_mem.mem_size=0x40000000  net.ifnames=0 dwc_otg.lpm_enable=0 console=ttyS0,115200 console=tty1 root=LABEL=writable rootfstype=ext4 elevator=deadline rootwait fixrtc quiet splash kpatch=1

I modified cmdline-string.patch for v5.15-rc1:

diff --git a/fs/proc/cmdline.c b/fs/proc/cmdline.c
index fa762c5fbcb2..bd6602769a93 100644
--- a/fs/proc/cmdline.c
+++ b/fs/proc/cmdline.c
@@ -6,8 +6,7 @@

 static int cmdline_proc_show(struct seq_file *m, void *v)
 {
-       seq_puts(m, saved_command_line);
-       seq_putc(m, '\n');
+       seq_printf(m, "%s kpatch=1\n", saved_command_line);
        return 0;
 }

@keiya-nobuta
Copy link
Contributor Author

Sorry for the late work.

I ran the tests and found some issues, so make a note of them:

Livepatch relocation issue on Arm64
There seems to be a problem with relocate the .klp.rela.<module name> section on Arm64. This section is relocation section to resolve symbols in other modules, and relocate is performed after the referenced module load. At this point, text section is read-only, so it needs to be remapped so that it can be written. However it's not supported in the current Arm64 linux implementation.

Specifically, when a patch module is loaded, Mem Abort occurs as follows:

[ 8741.489794] livepatch_gcc_static_local_var_6: loading out-of-tree module taints kernel.
[ 8741.489834] livepatch_gcc_static_local_var_6: tainting kernel with TAINT_LIVEPATCH
[ 8741.494103] Unable to handle kernel write to read-only memory at virtual address ffff80000af9027c
[ 8741.494158] Mem abort info:
[ 8741.494184]   ESR = 0x9600004f
<...>
[ 8741.494994] Call trace:
[ 8741.495000]  apply_relocate_add+0xcc8/0xda0
[ 8741.495013]  klp_apply_section_relocs+0x110/0x140
[ 8741.495026]  klp_init_object_loaded+0x7c/0x160
[ 8741.495038]  klp_enable_patch+0x3b8/0x810
[ 8741.495050]  patch_init+0x330/0x10000 [livepatch_gcc_static_local_var_6]
[ 8741.495068]  do_one_initcall+0x50/0x2a0
[ 8741.495078]  do_init_module+0x60/0x290
[ 8741.495088]  load_module+0x240c/0x2770
[ 8741.495097]  __do_sys_finit_module+0xbc/0x130
[ 8741.495107]  __arm64_sys_finit_module+0x2c/0x40
[ 8741.495117]  invoke_syscall+0x50/0x120
[ 8741.495128]  el0_svc_common.constprop.0+0x68/0x130
[ 8741.495139]  do_el0_svc+0x30/0xa0
[ 8741.495149]  el0_svc+0x24/0x70
[ 8741.495160]  el0t_64_sync_handler+0x1a4/0x1b0
[ 8741.495171]  el0t_64_sync+0x1a0/0x1a4
[ 8741.495183] Code: cb090073 d34c8262 942c6434 9360fe61 (b8366aa0)
[ 8741.495193] ---[ end trace 07d74716bfd62eed ]---

This issue needs to be addressed on the kernel side.
I found this morning that Suraj Jitindar Singh sends patch fixing it to kernel community:
[PATCH] arm64: module: Use aarch64_insn_write when updating relocations later on

Reliable stack trace issue
I think Madhavan's patch series v10 has issue that stack trace is always regarded as unreliable (when trace on exceptions from EL0), so livepatch always fail. I will report this to kernel community.

@keiya-nobuta
Copy link
Contributor Author

Oh, Suraj has also a pull-requested #1236.

@swine
Copy link
Contributor

swine commented Sep 15, 2022

I've rebased & extended Suraj's arm64 work in #1302

It's marked Draft for now, until tested with vanilla kernel/clang/gcc (with Madhavan's kernel patches)

@github-actions
Copy link

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 github-actions bot added the stale label Aug 11, 2023
@github-actions
Copy link

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants