-
Notifications
You must be signed in to change notification settings - Fork 305
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
Need suggestions for CONFIG_LTO_CLANG support #1320
Comments
@sm00th : This sounds familiar from your previous study of clang / LTO. Did you have any notes or branches left over from that investigation that may help @liu-song-6 ? |
Hi there. Yeah, I ran into this issue as well and used this hacky commit to alleviate the problem during development. However I didn't think that is a proper solution to find_symbol_by_index() slowness. The might be some other parts you might find usable on that branch: https://github.com/sm00th/kpatch/commits/lto It is all very raw though. |
@joe-lawrence @sm00th Thanks for sharing this work. I tried the lto branch (on top of some of our patches for pgo), and it seems there is still some issues. @sm00th , do you have plan to continue working on this? We are hoping to use kpatch on our LTO kernel in 2023 (hopefully H1). Thanks! |
Yes, there are. I worked on this a while ago and there definitely were issues left, but afaik it produced loadable and working (but unwieldy, they were roughly the same size as vmlinux because of all the debuginfo and rodata sections being copied over) kpatches for simple patches for vmlinux (maybe not for modules). The better parts of the branch were merged to mainline and what is left over are hacky commits that probably shouldn't be used as is but might give you some insights and early warnings about things I encountered.
No, I currently don't continue any work on this nor there are any plans to do so in near future. |
Thanks for the recap, @sm00th @liu-song-6 : since our day job work targets RHEL, we're not actively pursuing LTO / clang until the RHEL kernel goes that direction, but that wouldn't stop us from reviewing patches and integrating pieces upstream here. |
@sm00th thanks for these information. Do you, by any chance, have some design doc, discussion thread, etc. about the LTO work? I tried to understand the commits in your lto branch, but failed to construct the logic of the design. Some documentations may save me days of work on the wrong direction... Thanks again! |
I'm sorry, I don't have anything usable |
@sm00th Thanks! Let me see what I can get.. |
Some updates on this. @yonghong-song showed me how to use The LTO flow works like the following:
I tried to run create-diff-object on these thinlto files, and they seem to work (with some minor changes). But I haven't finished the whole process in kpatch-build. I would like some comments before developing all the logic. Specifically:
Thanks! |
CDOing full vmlinux is very unpleasant so I'd vote for anything that will allow to avoid this. I have limited understanding of ThinLTO but it sounds like there are no further optimizations on step 4 so we should be ok. |
@sm00th Thanks for the quick feedback! I will give thinlto a try. |
While I think that approach is fine as a stopgap (if it's not too much effort), pretty soon we will need CDO to support vmlinux.o anyway. Not only for LTO but also for CONFIG_X86_KERNEL_IBT and CONFIG_FINEIBT which are very useful hardening features which will soon become commonplace, IMO. |
Hmm... In this case, I guess CDOing the whole vmlinux.o makes more sense? |
Long term, yes. For the short term, maybe that can be delayed for your use here, if you get the thinlto thing figured out, and if the changes aren't too disruptive. I can pick up the work for CDO support for vmlinux.o, unless somebody else wants to do it. We (mostly peterz) had to do something similar for objtool upstream to get the performance under control.
If I understand you correctly:
In my experience I think such a patch would be very rare, but yes we'd need to make sure that case is detected and results in either a) build-time error or b) runtime ENDBR patching. |
Hm, thinking about this some more, I'm no longer sure that IBT will require CDO to support vmlinux.o. It's possible that CDO could just be ignorant and assume the indirect-branched-to function has ENDBR, and instead have livepatch do the ENDBR patching if needed. |
Let me take a closer look at both options (and our timeline). CDOing vmlinux.o sounds interesting to me. |
How shall we do runtime ENDBR patching? Do we need some type of trampoline for that? |
In this case, I guess livepatch has to use the original function to decide whether the KLP'ed function need ENDBR. And I guess this won't work if we are patching both funcA and funcB, and the new funcB requires endbr only because of patched funcA calls it with an indirect call. Did I understand this correctly? |
x86 kernel has text_poke_bp()
Hm, I guess there are multiple scenarios, including:
|
I think ENDBR is actually removed at compile time (by objtool?), so there may not be space to patch ENDBR at runtime?
Yeah, panic at testing time is sufficient for very rare case.
I think klp_ftrace_handler() always indirect-calls patched function? So 2 and 3 should always carry ENDBR anyway? |
The ENDBRs to be removed (aka "sealed") are annotated by objtool with the creation of the .ibt_endbr_seal section and then patched into NOPs by the kernel with apply_ibt_endbr(). For global functions, and for static functions which are referenced via function pointer in the same translation unit, the compiler emits ENDBR (which may be later converted to NOP by apply_ibt_endbr). But I forgot about static functions which aren't referenced via function pointer in the same TU. The compiler optimizes them by not emitting ENDBR. You're right, I don't think there are any good solutions for patching those.
Just to clarify this scenario, the compiler will already emit ENDBR for funcA (both original and replacement versions), because in this case either funcA will be global, or it will be static with funcB referencing its pointer. in the same TU. And objtool won't try to seal it. So I believe there's nothing to do here on our side.
Thinking again about this, this could actually be a little tricky. The way livepatching works, all function calls are routed to the original function first. So if the original funcA did not have ENDBR (i.e., the original funcA was not indirect-branched-to by any function including the original funcB), this wouldn't work unless the new funcB bypassed livepatching and called the replacement function directly. The alternative would be to do CDO on vmlinux.o and warn about this scenario at kpatch-build time. But again it seems like a rare issue and maybe not worth the effort. We could instead maybe consider adding a warning about any new function pointer references added by a patch if IBT is enabled, something like "be sure to test this and make sure this indirect branch to this function doesn't cause an IBT panic". So CDO on vmlinux.o might help enable/improve a few esoteric warnings, but I'm not sure that justifies the effort. At least not yet.
No, it actually returns to the patched function. I say this as the co-inventer of the patent ;-) |
Can we solve this with a wrapper function (or trampoline)? Something like:
If this works, we can also use the same mechanism to patch static functions which aren't referenced via function pointer in the same TU.
I didn't expect IBT support to be this tricky... I will first try support LTO without CDOing vmlinux.o.
E.. that's right. This is so tricky. |
Here is what I have so far. It seems to work on simpler patches (as the one in nfs and raid10). But failed with the following for the bpf change:
Any suggestions on what to fix/try from here? Thanks in advance! |
I looked a little more into this. It seems that CDO didn't export the symbol perf_event_bpf_event because of the following check.
I think we probably need some special handling for this case, I am not quite sure how. Any suggestions...? |
Hi @liu-song-6 : Does the |
@joe-lawrence yes, it is still in vmlinux but not in Module.symvers
|
Comparing LTO with non-LTO build: LTO:
non-LTO:
So the same symbol is GLOBAL in non-LTO kernel, but LOCAL in LTO kernel. With similar .config, vmlinux from LTO build has 2074 GLOBAL symbols; while vmlinux from non-LTO build has 34550 GLOBAL symbols. Can we just consider all symbols as GLOBAL for the LTO build? Will this break anything? |
Some hack like the following seems to work.
|
Support CONFIG_LTO_CLANG_THIN with ld.lld --lto-obj-path option. With CONFIG_LTO_CLANG_THIN, .o files are LLVM IR binary, so CDO doesn't work on .o file. To solve this issue, we CDO the thinlto files generated by the --lto-obj-path option. Clang LTO generates the thinlto files after cross file inline, so they are good candidates for CDO. See [1] for more discussions about this. To achieve this, we need: 1. kpatch-build to update kernel Makefile(s) so it generates thinlto files; 2. kpatch-build and kpatch-cc to save the thinlto file properly; 3. kpatch-build to feed these thinlto files to CDO; 4. A small workaround in CDO that ignores changes in init.text for vmlinux.o.thinlto.* [1] dynup#1320 Signed-off-by: Song Liu <[email protected]>
Support CONFIG_LTO_CLANG_THIN with ld.lld --lto-obj-path option. With CONFIG_LTO_CLANG_THIN, .o files are LLVM IR binary, so CDO doesn't work on .o file. To solve this issue, we CDO the thinlto files generated by the --lto-obj-path option. Clang LTO generates the thinlto files after cross file inline, so they are good candidates for CDO. See [1] for more discussions about this. To achieve this, we need: 1. kpatch-build to update kernel Makefile(s) so it generates thinlto files; 2. kpatch-build and kpatch-cc to save the thinlto file properly; 3. kpatch-build to feed these thinlto files to CDO; 4. The user need to supply vmlinux.o, from which we generate the symtab file. We need this because GLOBAL symbols may be marked as LOCAL in LTO vmlinux; 4. A small workaround in CDO that ignores changes in init.text for vmlinux.o.thinlto.* [1] dynup#1320 Signed-off-by: Song Liu <[email protected]>
Support CONFIG_LTO_CLANG_THIN with ld.lld --lto-obj-path option. With CONFIG_LTO_CLANG_THIN, .o files are LLVM IR binary, so CDO doesn't work on .o file. To solve this issue, we CDO the thinlto files generated by the --lto-obj-path option. Clang LTO generates the thinlto files after cross file inline, so they are good candidates for CDO. See [1] for more discussions about this. To achieve this, we need: 1. kpatch-build to update kernel Makefile(s) so it generates thinlto files; 2. kpatch-build and kpatch-cc to save the thinlto file properly; 3. kpatch-build to feed these thinlto files to CDO; 4. The user need to supply vmlinux.o, from which we generate the symtab file. We need this because GLOBAL symbols may be marked as LOCAL in LTO vmlinux; [1] dynup#1320 Signed-off-by: Song Liu <[email protected]>
Support CONFIG_LTO_CLANG_THIN with ld.lld --lto-obj-path option. With CONFIG_LTO_CLANG_THIN, .o files are LLVM IR binary, so CDO doesn't work on .o file. To solve this issue, we CDO the thinlto files generated by the --lto-obj-path option. Clang LTO generates the thinlto files after cross file inline, so they are good candidates for CDO. See [1] for more discussions about this. To achieve this, we need: 1. kpatch-build to update kernel Makefile(s) so it generates thinlto files; 2. kpatch-build and kpatch-cc to save the thinlto file properly; 3. kpatch-build to feed these thinlto files to CDO; 4. The user need to supply vmlinux.o, from which we generate the symtab file. We need this because GLOBAL symbols may be marked as LOCAL in LTO vmlinux; [1] dynup#1320 Signed-off-by: Song Liu <[email protected]>
Support CONFIG_LTO_CLANG_THIN with ld.lld --lto-obj-path option. With CONFIG_LTO_CLANG_THIN, .o files are LLVM IR binary, so CDO doesn't work on .o file. To solve this issue, we CDO the thinlto files generated by the --lto-obj-path option. Clang LTO generates the thinlto files after cross file inline, so they are good candidates for CDO. See [1] for more discussions about this. To achieve this, we need: 1. kpatch-build to update kernel Makefile(s) so it generates thinlto files; 2. kpatch-build and kpatch-cc to save the thinlto file properly; 3. kpatch-build to feed these thinlto files to CDO; 4. The user need to supply vmlinux.o, from which we generate the symtab file. We need this because GLOBAL symbols may be marked as LOCAL in LTO vmlinux; [1] dynup#1320 Signed-off-by: Song Liu <[email protected]>
Support CONFIG_LTO_CLANG_THIN with ld.lld --lto-obj-path option. With CONFIG_LTO_CLANG_THIN, .o files are LLVM IR binary, so CDO doesn't work on .o file. To solve this issue, we CDO the thinlto files generated by the --lto-obj-path option. Clang LTO generates the thinlto files after cross file inline, so they are good candidates for CDO. See [1] for more discussions about this. To achieve this, we need: 1. kpatch-build to update kernel Makefile(s) so it generates thinlto files; 2. kpatch-build and kpatch-cc to save the thinlto file properly; 3. kpatch-build to feed these thinlto files to CDO; 4. The user need to supply vmlinux.o, from which we generate the symtab file. We need this because GLOBAL symbols may be marked as LOCAL in LTO vmlinux; [1] dynup#1320 Signed-off-by: Song Liu <[email protected]>
Support CONFIG_LTO_CLANG_THIN with ld.lld --lto-obj-path option. With CONFIG_LTO_CLANG_THIN, .o files are LLVM IR binary, so CDO doesn't work on .o file. To solve this issue, we CDO the thinlto files generated by the --lto-obj-path option. Clang LTO generates the thinlto files after cross file inline, so they are good candidates for CDO. See [1] for more discussions about this. To achieve this, we need: 1. kpatch-build to update kernel Makefile(s) so it generates thinlto files; 2. kpatch-build and kpatch-cc to save the thinlto file properly; 3. kpatch-build to feed these thinlto files to CDO; 4. The user need to supply vmlinux.o, from which we generate the symtab file. We need this because GLOBAL symbols may be marked as LOCAL in LTO vmlinux; [1] dynup#1320 Signed-off-by: Song Liu <[email protected]>
Support CONFIG_LTO_CLANG_THIN with ld.lld --lto-obj-path option. With CONFIG_LTO_CLANG_THIN, .o files are LLVM IR binary, so CDO doesn't work on .o file. To solve this issue, we CDO the thinlto files generated by the --lto-obj-path option. Clang LTO generates the thinlto files after cross file inline, so they are good candidates for CDO. See [1] for more discussions about this. To achieve this, we need: 1. kpatch-build to update kernel Makefile(s) so it generates thinlto files; 2. kpatch-build and kpatch-cc to save the thinlto file properly; 3. kpatch-build to feed these thinlto files to CDO; 4. The user need to supply vmlinux.o, from which we generate the symtab file. We need this because GLOBAL symbols may be marked as LOCAL in LTO vmlinux; 4. A small workaround in CDO that ignores changes in init.text for vmlinux.o.thinlto.* [1] dynup#1320 Signed-off-by: Song Liu <[email protected]>
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. |
This issue was closed because it was inactive for 7 days after being marked stale. |
Reopening, IMHO the bot was a bit premature in closing this one. |
Drive-by comment: There's a public tool to generate livepatches for LLVM. It might be useful to you. https://github.com/google/llpatch |
Bill,
LLpatch is a good solution in the long term, as it takes direction from the
arch-independent LLVM metadata where programmer intent can be more visible,
rather than reverse-engineering from the evidence which survives the
generation of per-arch ELF objects. So there's a much smaller arch-specific
component, and less guesswork.
But it's not been updated to understand the changes in symbol/section
naming of the last year's evolution of clang or kernel.
Until we've done more thorough testing, I see it more as an adjunct to
kpatch's create-diff-object, rather than an alternative - llvm-diff can
filter out inconsequential object-pair differences leaving a hit-list of
object-pairs to be processed by c-d-o.
We created LLpatch as a more arch-portable tool than c-d-o, in the absence
of an active effort to port kpatch to arm64.
But since Surajj's work surfaced it seems more prudent to concentrate on a
gcc+clang solution in kpatch/arm64, at least until the 3 strands (kernel,
kpatch, objtool) have all upstreamed their klp/arm64 contributions.
Juggling a 4th strand muddies the waters
But if you _are_ a clang-built-linux shop, especially using LTO, where the
split-section cflags are in place for the base kernel build, LLpatch may be
a useful tool
…-pete
On Thu, Aug 31, 2023 at 12:45 PM Bill Wendling ***@***.***> wrote:
Drive-by comment: There's a public tool to generate livepatches for LLVM.
It might be useful to you. https://github.com/google/llpatch
—
Reply to this email directly, view it on GitHub
<#1320 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAH4XE2K7A7UB35TM4LT77TXYDSTXANCNFSM6AAAAAASZYQKBQ>
.
You are receiving this because you are subscribed to this thread.Message
ID: ***@***.***>
|
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. |
@liu-song-6 I found that the thinlto-6.2 branch can create live patches for kernel v6.2. However, attempting to use the fb-6.4 branch to generate a live patch for kernel v6.4 results in the following error.
The
The
I'm wondering if I'm doing something wrong, or if there are additional patches for kernel v6.4. Could you share your thoughts? Additionally, have you tried the thin LTO live patch on kernel v5.15? Any insights would be greatly appreciated. Thanks, |
@fazlamehrab Thanks for testing this out. The config looks correct to me. Have you tried to use |
@liu-song-6 For v5.15, I get the following error.
The
And, for v6.4, I get the following error.
In this case, the
Please share your thoughts on these. |
Support CONFIG_LTO_CLANG_THIN with ld.lld --lto-obj-path option. With CONFIG_LTO_CLANG_THIN, .o files are LLVM IR binary, so CDO doesn't work on .o file. To solve this issue, we CDO the thinlto files generated by the --lto-obj-path option. Clang LTO generates the thinlto files after cross file inline, so they are good candidates for CDO. See [1] for more discussions about this. To achieve this, we need: 1. kpatch-build to update kernel Makefile(s) so it generates thinlto files; 2. kpatch-build and kpatch-cc to save the thinlto file properly; 3. kpatch-build to feed these thinlto files to CDO; 4. The user need to supply vmlinux.o, from which we generate the symtab file. We need this because GLOBAL symbols may be marked as LOCAL in LTO vmlinux; [1] dynup#1320 Signed-off-by: Song Liu <[email protected]>
@fazlamehrab I hit something recently. I "fixed" it disabling some configs. You can probably try disable the following: CONFIG_CALL_THUNKS Note that, they have some cross dependency, so you want to confirm they actually got disabled. |
@liu-song-6 I tried this, but no luck with v5.15 kernel, yet. I must be missing something. Can you please see if you get the same error that I mentioned earlier with the attached config? It is based on default config using LLVM 11. I also tried with LLVM 14, which gives me following errors.
Also, please share the config that works for you, if possible. Thanks! |
As a general update, we have no plans of solving this problem in kpatch-build. In fact, kpatch-build will eventually be deprecated in favor of klp-build, which is currently under development and will be merged into the upstream kernel. |
I am try to build livepatch for kernel built with CONFIG_LTO_CLANG.
The first issue was these .o files are not ELF files, but LLVM IR bitcode. To resolve this, I made some hack so that we run create-diff-object on vmlinux.o, which is ELF. However, create-diff-object spent hours in the following stack:
which makes it unusable.
So my question is: Shall we optimize create-diff-object to be faster? Or shall we try some other solutions?
Thanks in advance!
The text was updated successfully, but these errors were encountered: