* Re: [PATCH v2 2/5] livepatch/klp-build: handle patches that add/remove files
From: Josh Poimboeuf @ 2026-02-05 16:53 UTC (permalink / raw)
To: Joe Lawrence
Cc: live-patching, Song Liu, Jiri Kosina, Miroslav Benes, Petr Mladek
In-Reply-To: <aYTGtI41jhDSm5gf@redhat.com>
On Thu, Feb 05, 2026 at 11:35:00AM -0500, Joe Lawrence wrote:
> On Wed, Feb 04, 2026 at 10:02:38AM -0800, Josh Poimboeuf wrote:
> > On Tue, Feb 03, 2026 at 09:51:37PM -0500, Joe Lawrence wrote:
> > > The klp-build script prepares a clean patch by populating two temporary
> > > directories ('a' and 'b') with source files and diffing the result.
> > > However, this process currently fails when a patch introduces a new
> > > source file as the script attempts to copy files that do not yet exist
> > > in the original source tree. Likewise, there is a similar limitation
> > > when a patch removes a source file and the script tries to copy files
> > > that no longer exist.
> > >
> > > Refactor the file-gathering logic to distinguish between original input
> > > files and patched output files:
> > >
> > > - Split get_patch_files() into get_patch_input_files() and
> > > get_patch_output_files() to identify which files exist before and
> > > after patch application.
> > > - Filter out "/dev/null" from both to handle file creation/deletion
> > > - Update refresh_patch() to only copy existing input files to the 'a'
> > > directory and the resulting output files to the 'b' directory.
> > >
> > > This allows klp-build to successfully process patches that add or remove
> > > source files.
> > >
> > > Signed-off-by: Joe Lawrence <joe.lawrence@redhat.com>
> > > ---
> > > scripts/livepatch/klp-build | 34 +++++++++++++++++++++++++++-------
> > > 1 file changed, 27 insertions(+), 7 deletions(-)
> > >
> > > Lightly tested with patches that added or removed a source file, as
> > > generated by `git diff`, `git format-patch`, and `diff -Nupr`.
> > >
> > > diff --git a/scripts/livepatch/klp-build b/scripts/livepatch/klp-build
> > > index 9f1b77c2b2b7..5a99ff4c4729 100755
> > > --- a/scripts/livepatch/klp-build
> > > +++ b/scripts/livepatch/klp-build
> > > @@ -299,15 +299,33 @@ set_kernelversion() {
> > > sed -i "2i echo $localversion; exit 0" scripts/setlocalversion
> > > }
> > >
> > > -get_patch_files() {
> > > +get_patch_input_files() {
> > > + local patch="$1"
> > > +
> > > + grep0 -E '^--- ' "$patch" \
> > > + | gawk '{print $2}' \
> > > + | grep -v '^/dev/null$' \
> >
> > Because pipefail is enabled, the grep0 helper should be used instead of
> > grep, otherwise a failed match can propagate to an error. Maybe we need
> > a "make check" or something which enforces that and runs shellcheck.
> >
>
> Good catch. So your idea is to drop a Makefile in scripts/livepatch
> with a check target that runs shellcheck and then a klp-build specific
> check for any non-grep0 grep? (like `grep -w 'grep' klp-build`). If
> so, any other things to should check for?
That's all I can think of, hopefully shellcheck would catch most of the
other footguns.
--
Josh
^ permalink raw reply
* Re: [PATCH] klp-build: Support clang/llvm built kernel
From: Song Liu @ 2026-02-05 16:50 UTC (permalink / raw)
To: Josh Poimboeuf
Cc: live-patching, kernel-team, jikos, mbenes, pmladek, joe.lawrence
In-Reply-To: <gqqr3ulqbhecvev36ry7nlra3fysgltlpiv2lzsil7ewrwy7qx@dlp77z56npqc>
On Thu, Feb 5, 2026 at 8:25 AM Josh Poimboeuf <jpoimboe@kernel.org> wrote:
>
> On Thu, Jan 29, 2026 at 09:03:21AM -0800, Song Liu wrote:
> > When the kernel is built with clang/llvm, it is expected to run make
> > with "make LLVM=1 ...". The same is needed when building livepatches.
> >
> > Use CONFIG_CC_IS_CLANG as the flag to detect kernel built with
> > clang/llvm, and add LLVM=1 to make commands from klp-build
> >
> > Signed-off-by: Song Liu <song@kernel.org>
>
> Peter informed me that "LLVM=" has different syntaxes:
>
> LLVM=1
> LLVM=-22
> LLVM=/opt/llvm/
>
> Debian has parallel llvm (and gcc) toolchains, and suffixes them with
> -$ver.
>
> So we dropped this patch for now. "export LLVM=1" still works. Not
> sure if you have any other ideas?
Hmm... I guess "export LLVM=something" is probably the best option
for now.
Thanks,
Song
^ permalink raw reply
* Re: [PATCH v2 2/5] livepatch/klp-build: handle patches that add/remove files
From: Joe Lawrence @ 2026-02-05 16:35 UTC (permalink / raw)
To: Josh Poimboeuf
Cc: live-patching, Song Liu, Jiri Kosina, Miroslav Benes, Petr Mladek
In-Reply-To: <ywooax5vfkh7k7h4mjpxfhtbkr3rdcvi5sjqmnjgcmxrc4ykwa@mk6z5zosbuvz>
On Wed, Feb 04, 2026 at 10:02:38AM -0800, Josh Poimboeuf wrote:
> On Tue, Feb 03, 2026 at 09:51:37PM -0500, Joe Lawrence wrote:
> > The klp-build script prepares a clean patch by populating two temporary
> > directories ('a' and 'b') with source files and diffing the result.
> > However, this process currently fails when a patch introduces a new
> > source file as the script attempts to copy files that do not yet exist
> > in the original source tree. Likewise, there is a similar limitation
> > when a patch removes a source file and the script tries to copy files
> > that no longer exist.
> >
> > Refactor the file-gathering logic to distinguish between original input
> > files and patched output files:
> >
> > - Split get_patch_files() into get_patch_input_files() and
> > get_patch_output_files() to identify which files exist before and
> > after patch application.
> > - Filter out "/dev/null" from both to handle file creation/deletion
> > - Update refresh_patch() to only copy existing input files to the 'a'
> > directory and the resulting output files to the 'b' directory.
> >
> > This allows klp-build to successfully process patches that add or remove
> > source files.
> >
> > Signed-off-by: Joe Lawrence <joe.lawrence@redhat.com>
> > ---
> > scripts/livepatch/klp-build | 34 +++++++++++++++++++++++++++-------
> > 1 file changed, 27 insertions(+), 7 deletions(-)
> >
> > Lightly tested with patches that added or removed a source file, as
> > generated by `git diff`, `git format-patch`, and `diff -Nupr`.
> >
> > diff --git a/scripts/livepatch/klp-build b/scripts/livepatch/klp-build
> > index 9f1b77c2b2b7..5a99ff4c4729 100755
> > --- a/scripts/livepatch/klp-build
> > +++ b/scripts/livepatch/klp-build
> > @@ -299,15 +299,33 @@ set_kernelversion() {
> > sed -i "2i echo $localversion; exit 0" scripts/setlocalversion
> > }
> >
> > -get_patch_files() {
> > +get_patch_input_files() {
> > + local patch="$1"
> > +
> > + grep0 -E '^--- ' "$patch" \
> > + | gawk '{print $2}' \
> > + | grep -v '^/dev/null$' \
>
> Because pipefail is enabled, the grep0 helper should be used instead of
> grep, otherwise a failed match can propagate to an error. Maybe we need
> a "make check" or something which enforces that and runs shellcheck.
>
Good catch. So your idea is to drop a Makefile in scripts/livepatch
with a check target that runs shellcheck and then a klp-build specific
check for any non-grep0 grep? (like `grep -w 'grep' klp-build`). If
so, any other things to should check for?
--
Joe
^ permalink raw reply
* Re: [PATCH] klp-build: Support clang/llvm built kernel
From: Josh Poimboeuf @ 2026-02-05 16:25 UTC (permalink / raw)
To: Song Liu; +Cc: live-patching, kernel-team, jikos, mbenes, pmladek, joe.lawrence
In-Reply-To: <20260129170321.700854-1-song@kernel.org>
On Thu, Jan 29, 2026 at 09:03:21AM -0800, Song Liu wrote:
> When the kernel is built with clang/llvm, it is expected to run make
> with "make LLVM=1 ...". The same is needed when building livepatches.
>
> Use CONFIG_CC_IS_CLANG as the flag to detect kernel built with
> clang/llvm, and add LLVM=1 to make commands from klp-build
>
> Signed-off-by: Song Liu <song@kernel.org>
Peter informed me that "LLVM=" has different syntaxes:
LLVM=1
LLVM=-22
LLVM=/opt/llvm/
Debian has parallel llvm (and gcc) toolchains, and suffixes them with
-$ver.
So we dropped this patch for now. "export LLVM=1" still works. Not
sure if you have any other ideas?
At the very least we should probably check that LLVM=<whatever> and/or
CC=clang are set appropriately before doing a CONFIG_CC_IS_CLANG build,
and error out if not.
--
Josh
^ permalink raw reply
* Re: [PATCH v2 1/5] objtool/klp: Fix mkstemp() failure with long paths
From: Joe Lawrence @ 2026-02-05 15:53 UTC (permalink / raw)
To: Josh Poimboeuf
Cc: live-patching, Song Liu, Jiri Kosina, Miroslav Benes, Petr Mladek
In-Reply-To: <jwj6k6bbvga3uaaj2hhtau256t7mvcg65wsv5cpsdrx7cpt4zd@knbng27js6t5>
On Wed, Feb 04, 2026 at 08:47:41AM -0800, Josh Poimboeuf wrote:
> On Tue, Feb 03, 2026 at 09:51:36PM -0500, Joe Lawrence wrote:
> > @@ -1219,13 +1221,17 @@ struct elf *elf_create_file(GElf_Ehdr *ehdr, const char *name)
> >
> > base = basename(base);
> >
> > - tmp_name = malloc(256);
> > + tmp_name = malloc(PATH_MAX);
>
> The allocation size can be more precise with something like
>
> tmp_name = malloc(strlen(name) + 8);
>
> Also, I'm scratching my head at the existing code and wondering why we
> are splitting out the dirname() and the basename() just to paste them
> back together again?? Can you simplify that while you're at it?
>
> > if (!tmp_name) {
> > ERROR_GLIBC("malloc");
> > return NULL;
> > }
> >
> > - snprintf(tmp_name, 256, "%s/%s.XXXXXX", dir, base);
> > + path_len = snprintf(tmp_name, PATH_MAX, "%s/%s.XXXXXX", dir, base);
> > + if (path_len >= PATH_MAX) {
> > + ERROR_GLIBC("snprintf");
> > + return NULL;
> > + }
>
> Checking for all the snprintf() cases can be a pain so we have a
> snprintf_check() for a streamlined error checking experience.
>
Ah, completely missed that, thanks for pointing out. I'll incorporate
both suggestions into v3.
--
Joe
^ permalink raw reply
* Re: [PATCH v2 4/5] livepatch/klp-build: minor short-circuiting tweaks
From: Josh Poimboeuf @ 2026-02-04 18:40 UTC (permalink / raw)
To: Joe Lawrence
Cc: live-patching, Song Liu, Jiri Kosina, Miroslav Benes, Petr Mladek
In-Reply-To: <20260204025140.2023382-5-joe.lawrence@redhat.com>
On Tue, Feb 03, 2026 at 09:51:39PM -0500, Joe Lawrence wrote:
> Update SHORT_CIRCUIT behavior to better handle patch validation and
> argument processing in later klp-build steps.
>
> Perform patch validation for both step 1 (building original kernel)
> and step 2 (building patched kernel) to ensure patches are verified
> before any compilation occurs.
>
> Additionally, allow the user to omit input patches when skipping past
> step 2, while noting that any specified patches will be ignored in that
> case if they were provided.
>
> Signed-off-by: Joe Lawrence <joe.lawrence@redhat.com>
> ---
> scripts/livepatch/klp-build | 17 +++++++++++++----
> 1 file changed, 13 insertions(+), 4 deletions(-)
>
> diff --git a/scripts/livepatch/klp-build b/scripts/livepatch/klp-build
> index ee43a9caa107..df3a0fa031a6 100755
> --- a/scripts/livepatch/klp-build
> +++ b/scripts/livepatch/klp-build
> @@ -214,12 +214,18 @@ process_args() {
> done
>
> if [[ $# -eq 0 ]]; then
> - usage
> - exit 1
> + if (( SHORT_CIRCUIT <= 2 )); then
> + usage
> + exit 1
> + fi
Ack
> + else
> + if (( SHORT_CIRCUIT >= 3 )); then
> + status "note: patch arguments ignored at step $SHORT_CIRCUIT"
> + fi
Personally I don't care to see this status, but maybe I'm biased from
writing the --short-circuit feature and not being confused by this :-)
--
Josh
^ permalink raw reply
* Re: [PATCH v2 3/5] livepatch/klp-build: switch to GNU patch and recountdiff
From: Josh Poimboeuf @ 2026-02-04 18:35 UTC (permalink / raw)
To: Joe Lawrence
Cc: live-patching, Song Liu, Jiri Kosina, Miroslav Benes, Petr Mladek
In-Reply-To: <20260204025140.2023382-4-joe.lawrence@redhat.com>
On Tue, Feb 03, 2026 at 09:51:38PM -0500, Joe Lawrence wrote:
> I think this does simplify things, but:
>
> - introduces a dependency on patchutil's recountdiff
I was wondering if we could instead just update fix-patch-lines to fix
the line numbers/counts, but I get the feeling that would require
rewriting the whole script and may not be worth the complexity. That
script is nice and simple and robust at the moment.
> - requires goofy epoch timestamp filtering as `diff -N` doesn't use
> the `git diff` /dev/null, but a localized beginining of time epoch
> that may be 1969 or 1970 depending on the local timezone
> - can be *really* chatty, for example:
>
> Validating patch(es)
> patching file fs/proc/cmdline.c
> Hunk #1 succeeded at 7 (offset 1 line).
> Fixing patch(es)
> patching file fs/proc/cmdline.c
> Hunk #1 succeeded at 7 (offset 1 line).
> patching file fs/proc/cmdline.c
> patching file fs/proc/cmdline.c
> Building patched kernel
> Copying patched object files
> Diffing objects
> vmlinux.o: changed function: override_release
> vmlinux.o: changed function: cmdline_proc_show
> Building patch module: livepatch-cmdline-string.ko
> SUCCESS
>
> My initial thought was that I'd only be interested in knowing about
> patch offset/fuzz during the validation phase. And in the interest of
> clarifying some of the output messages, it would be nice to know the
> patch it was referring to, so how about a follow up patch
> pretty-formatting with some indentation like:
>
> Validating patch(es)
> cmdline-string.patch
> patching file fs/proc/cmdline.c
> Hunk #1 succeeded at 7 (offset 1 line).
> Fixing patch(es)
> Building patched kernel
> Copying patched object files
> Diffing objects
> vmlinux.o: changed function: override_release
> vmlinux.o: changed function: cmdline_proc_show
> Building patch module: livepatch-cmdline-string.ko
> SUCCESS
>
> That said, Song suggested using --silent across the board, so maybe
> tie that into the existing --verbose option?
Hm. Currently we go to considerable effort to make klp-build's output
as concise as possible, which is good. On the other hand, it might be
important to know the patch has fuzz.
I'm thinking I would agree that maybe it should be verbose when
validating patches and silent elsewhere. And the pretty formatting is a
nice upgrade to that.
We might also consider indenting the outputs of the other steps. For
example:
Building patched kernel
vmlinux.o: some warning
Copying patched object files
Diffing objects
vmlinux.o: changed function: override_release
vmlinux.o: changed function: cmdline_proc_show
Or alternatively, print the step names in ASCII bold or something.
> apply_patch() {
> local patch="$1"
> - shift
> - local extra_args=("$@")
>
> [[ ! -f "$patch" ]] && die "$patch doesn't exist"
>
> (
> cd "$SRC"
> -
> - # The sed strips the version signature from 'git format-patch',
> - # otherwise 'git apply --recount' warns.
> - sed -n '/^-- /q;p' "$patch" |
> - git apply "${extra_args[@]}"
> + # The sed strips the version signature from 'git format-patch'.
> + sed -n '/^-- /q;p' "$patch" | \
> + patch -p1 --no-backup-if-mismatch -r /dev/null
Is this still needed now that we don't use git apply --recount?
> @@ -490,7 +468,7 @@ fix_patches() {
>
> cp -f "$old_patch" "$tmp_patch"
> refresh_patch "$tmp_patch"
> - "$FIX_PATCH_LINES" "$tmp_patch" > "$new_patch"
> + "$FIX_PATCH_LINES" "$tmp_patch" | recountdiff > "$new_patch"
> refresh_patch "$new_patch"
Do we still need to refresh after the recountdiff?
--
Josh
^ permalink raw reply
* Re: [PATCH v2 2/5] livepatch/klp-build: handle patches that add/remove files
From: Josh Poimboeuf @ 2026-02-04 18:02 UTC (permalink / raw)
To: Joe Lawrence
Cc: live-patching, Song Liu, Jiri Kosina, Miroslav Benes, Petr Mladek
In-Reply-To: <20260204025140.2023382-3-joe.lawrence@redhat.com>
On Tue, Feb 03, 2026 at 09:51:37PM -0500, Joe Lawrence wrote:
> The klp-build script prepares a clean patch by populating two temporary
> directories ('a' and 'b') with source files and diffing the result.
> However, this process currently fails when a patch introduces a new
> source file as the script attempts to copy files that do not yet exist
> in the original source tree. Likewise, there is a similar limitation
> when a patch removes a source file and the script tries to copy files
> that no longer exist.
>
> Refactor the file-gathering logic to distinguish between original input
> files and patched output files:
>
> - Split get_patch_files() into get_patch_input_files() and
> get_patch_output_files() to identify which files exist before and
> after patch application.
> - Filter out "/dev/null" from both to handle file creation/deletion
> - Update refresh_patch() to only copy existing input files to the 'a'
> directory and the resulting output files to the 'b' directory.
>
> This allows klp-build to successfully process patches that add or remove
> source files.
>
> Signed-off-by: Joe Lawrence <joe.lawrence@redhat.com>
> ---
> scripts/livepatch/klp-build | 34 +++++++++++++++++++++++++++-------
> 1 file changed, 27 insertions(+), 7 deletions(-)
>
> Lightly tested with patches that added or removed a source file, as
> generated by `git diff`, `git format-patch`, and `diff -Nupr`.
>
> diff --git a/scripts/livepatch/klp-build b/scripts/livepatch/klp-build
> index 9f1b77c2b2b7..5a99ff4c4729 100755
> --- a/scripts/livepatch/klp-build
> +++ b/scripts/livepatch/klp-build
> @@ -299,15 +299,33 @@ set_kernelversion() {
> sed -i "2i echo $localversion; exit 0" scripts/setlocalversion
> }
>
> -get_patch_files() {
> +get_patch_input_files() {
> + local patch="$1"
> +
> + grep0 -E '^--- ' "$patch" \
> + | gawk '{print $2}' \
> + | grep -v '^/dev/null$' \
Because pipefail is enabled, the grep0 helper should be used instead of
grep, otherwise a failed match can propagate to an error. Maybe we need
a "make check" or something which enforces that and runs shellcheck.
--
Josh
^ permalink raw reply
* Re: [PATCH v2 1/5] objtool/klp: Fix mkstemp() failure with long paths
From: Josh Poimboeuf @ 2026-02-04 16:47 UTC (permalink / raw)
To: Joe Lawrence
Cc: live-patching, Song Liu, Jiri Kosina, Miroslav Benes, Petr Mladek
In-Reply-To: <20260204025140.2023382-2-joe.lawrence@redhat.com>
On Tue, Feb 03, 2026 at 09:51:36PM -0500, Joe Lawrence wrote:
> @@ -1219,13 +1221,17 @@ struct elf *elf_create_file(GElf_Ehdr *ehdr, const char *name)
>
> base = basename(base);
>
> - tmp_name = malloc(256);
> + tmp_name = malloc(PATH_MAX);
The allocation size can be more precise with something like
tmp_name = malloc(strlen(name) + 8);
Also, I'm scratching my head at the existing code and wondering why we
are splitting out the dirname() and the basename() just to paste them
back together again?? Can you simplify that while you're at it?
> if (!tmp_name) {
> ERROR_GLIBC("malloc");
> return NULL;
> }
>
> - snprintf(tmp_name, 256, "%s/%s.XXXXXX", dir, base);
> + path_len = snprintf(tmp_name, PATH_MAX, "%s/%s.XXXXXX", dir, base);
> + if (path_len >= PATH_MAX) {
> + ERROR_GLIBC("snprintf");
> + return NULL;
> + }
Checking for all the snprintf() cases can be a pain so we have a
snprintf_check() for a streamlined error checking experience.
--
Josh
^ permalink raw reply
* Re: [PATCH] klp-build: Update demangle_name for LTO
From: Song Liu @ 2026-02-04 6:38 UTC (permalink / raw)
To: Josh Poimboeuf
Cc: live-patching, kernel-team, jikos, mbenes, pmladek, joe.lawrence
In-Reply-To: <ojtyhae2qexuvdogiwvja24g7dh7jhe6epl44wupicgigq7qkf@e4t7mvkycex3>
On Tue, Feb 3, 2026 at 5:24 PM Josh Poimboeuf <jpoimboe@kernel.org> wrote:
>
> On Tue, Feb 03, 2026 at 04:24:06PM -0800, Song Liu wrote:
> > On Tue, Feb 3, 2026 at 3:53 PM Josh Poimboeuf <jpoimboe@kernel.org> wrote:
> > >
> > > On Tue, Feb 03, 2026 at 01:40:06PM -0800, Song Liu wrote:
> > > > With CONFIG_LTO_CLANG_THIN, __UNIQUE_ID_* can be global. Therefore, it
> > > > is necessary to demangle global symbols.
> > >
> > > Ouch, so LTO is changing symbol bindings :-/
> > >
> > > If a patch causes a symbol to change from LOCAL to GLOBAL between the
> > > original and patched builds, that will break some fundamental
> > > assumptions in the correlation logic.
> >
> > This can indeed happen. A function can be "LOCAL DEFAULT" XXXX
> > in original, and "GLOBAL HIDDEN" XXXX.llvm.<hash> in patched.
> >
> > I am trying to fix this with incremental steps.
> >
> > > Also, notice sym->demangled_name isn't used for correlating global
> > > symbols in correlate_symbols(). That code currently assumes all global
> > > symbols are uniquely named (and don't change between orig and patched).
> > > So this first fix seems incomplete.
> >
> > We still need to fix correlate_symbols(). I am not 100% sure how to do
> > that part yet.
> >
> > OTOH, this part still helps. This is because checksum_update_insn()
> > uses demangled_name. After the fix, if a function is renamed from
> > XXXX to XXXX.llvm.<hash> after the patch, functions that call the
> > function are not considered changed.
>
> Hm, wouldn't that still leave the .llvm at the end?
E.. Right. I described a different case.
The actual case being fixed here is when foo.llvm.<hash_1> got
changed to foo.llvm.<hash_2>.
Let me think more about all the cases and have a better solution.
Thanks,
Song
^ permalink raw reply
* Re: [PATCH v2 3/5] livepatch/klp-build: switch to GNU patch and recountdiff
From: Joe Lawrence @ 2026-02-04 2:58 UTC (permalink / raw)
To: live-patching
Cc: Josh Poimboeuf, Song Liu, Jiri Kosina, Miroslav Benes,
Petr Mladek
In-Reply-To: <20260204025140.2023382-4-joe.lawrence@redhat.com>
On Tue, Feb 03, 2026 at 09:51:38PM -0500, Joe Lawrence wrote:
> The klp-build script is currently very strict with input patches,
> requiring them to apply cleanly via `git apply --recount`. This
> prevents the use of patches with minor contextual fuzz relative to the
> target kernel sources.
>
> To allow users to reuse a patch across similar kernel streams, switch to
> using GNU patch and patchutils for intermediate patch manipulation.
> Update the logic for applying, reverting, and regenerating patches:
>
> - Use 'patch -p1' for better handling of context fuzz.
> - Use 'recountdiff' to update line counts after FIX_PATCH_LINES.
> - Drop git_refresh() and related git-specific logic.
>
> Signed-off-by: Joe Lawrence <joe.lawrence@redhat.com>
> ---
> scripts/livepatch/klp-build | 50 +++++++++++--------------------------
> 1 file changed, 14 insertions(+), 36 deletions(-)
>
Forgot to mention this was tested as per Josh's suggestion (slightly
modified*):
$ find . -type f -name '*.c' ! -path "./lib/*" -print0 | xargs -0 sed -i '1iasm("nop");'
$ git checkout tools arch/x86/lib/inat.c arch/x86/lib/insn.c kernel/configs.c
$ git diff > /tmp/oneline.patch
$ ./scripts/livepatch/klp-build /tmp/oneline.patch
...
error: klp-build: no changes detected
* modified to exclude lib/ to dodge unsupported patch complaints, avoid
symlinks (GNU patch does not like), and kernel/configs.o which objtool
couldn't find its .discard.sym_checksum section
--
Joe
^ permalink raw reply
* [PATCH v2 5/5] livepatch/klp-build: provide friendlier error messages
From: Joe Lawrence @ 2026-02-04 2:51 UTC (permalink / raw)
To: live-patching
Cc: Josh Poimboeuf, Song Liu, Jiri Kosina, Miroslav Benes,
Petr Mladek
In-Reply-To: <20260204025140.2023382-1-joe.lawrence@redhat.com>
Provide a little bit more context behind some of the klp-build failure
modes clarify which of the user-provided patches is unsupported,
doesn't apply, and which kernel build failed.
Signed-off-by: Joe Lawrence <joe.lawrence@redhat.com>
---
scripts/livepatch/klp-build | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/scripts/livepatch/klp-build b/scripts/livepatch/klp-build
index df3a0fa031a6..f8ce456523fe 100755
--- a/scripts/livepatch/klp-build
+++ b/scripts/livepatch/klp-build
@@ -343,7 +343,7 @@ check_unsupported_patches() {
for file in "${files[@]}"; do
case "$file" in
lib/*|*.S)
- die "unsupported patch to $file"
+ die "$patch unsupported patch to $file"
;;
esac
done
@@ -359,7 +359,7 @@ apply_patch() {
cd "$SRC"
# The sed strips the version signature from 'git format-patch'.
sed -n '/^-- /q;p' "$patch" | \
- patch -p1 --no-backup-if-mismatch -r /dev/null
+ patch -p1 --no-backup-if-mismatch -r /dev/null || die "$patch doesn't apply"
)
APPLIED_PATCHES+=("$patch")
@@ -500,6 +500,7 @@ clean_kernel() {
}
build_kernel() {
+ local build="$1"
local log="$TMP_DIR/build.log"
local objtool_args=()
local cmd=()
@@ -538,7 +539,7 @@ build_kernel() {
"${cmd[@]}" \
1> >(tee -a "$log") \
2> >(tee -a "$log" | grep0 -v "modpost.*undefined!" >&2)
- )
+ ) || die "$build kernel build failed"
}
find_objects() {
@@ -815,7 +816,7 @@ fi
if (( SHORT_CIRCUIT <= 1 )); then
status "Building original kernel"
clean_kernel
- build_kernel
+ build_kernel "original"
status "Copying original object files"
copy_orig_objects
fi
@@ -825,7 +826,7 @@ if (( SHORT_CIRCUIT <= 2 )); then
fix_patches
apply_patches
status "Building patched kernel"
- build_kernel
+ build_kernel "patched"
revert_patches
status "Copying patched object files"
copy_patched_objects
--
2.52.0
^ permalink raw reply related
* [PATCH v2 4/5] livepatch/klp-build: minor short-circuiting tweaks
From: Joe Lawrence @ 2026-02-04 2:51 UTC (permalink / raw)
To: live-patching
Cc: Josh Poimboeuf, Song Liu, Jiri Kosina, Miroslav Benes,
Petr Mladek
In-Reply-To: <20260204025140.2023382-1-joe.lawrence@redhat.com>
Update SHORT_CIRCUIT behavior to better handle patch validation and
argument processing in later klp-build steps.
Perform patch validation for both step 1 (building original kernel)
and step 2 (building patched kernel) to ensure patches are verified
before any compilation occurs.
Additionally, allow the user to omit input patches when skipping past
step 2, while noting that any specified patches will be ignored in that
case if they were provided.
Signed-off-by: Joe Lawrence <joe.lawrence@redhat.com>
---
scripts/livepatch/klp-build | 17 +++++++++++++----
1 file changed, 13 insertions(+), 4 deletions(-)
diff --git a/scripts/livepatch/klp-build b/scripts/livepatch/klp-build
index ee43a9caa107..df3a0fa031a6 100755
--- a/scripts/livepatch/klp-build
+++ b/scripts/livepatch/klp-build
@@ -214,12 +214,18 @@ process_args() {
done
if [[ $# -eq 0 ]]; then
- usage
- exit 1
+ if (( SHORT_CIRCUIT <= 2 )); then
+ usage
+ exit 1
+ fi
+ else
+ if (( SHORT_CIRCUIT >= 3 )); then
+ status "note: patch arguments ignored at step $SHORT_CIRCUIT"
+ fi
fi
+ PATCHES=("$@")
KEEP_TMP="$keep_tmp"
- PATCHES=("$@")
}
# temporarily disable xtrace for especially verbose code
@@ -801,9 +807,12 @@ build_patch_module() {
process_args "$@"
do_init
-if (( SHORT_CIRCUIT <= 1 )); then
+if (( SHORT_CIRCUIT <= 2 )); then
status "Validating patch(es)"
validate_patches
+fi
+
+if (( SHORT_CIRCUIT <= 1 )); then
status "Building original kernel"
clean_kernel
build_kernel
--
2.52.0
^ permalink raw reply related
* [PATCH v2 3/5] livepatch/klp-build: switch to GNU patch and recountdiff
From: Joe Lawrence @ 2026-02-04 2:51 UTC (permalink / raw)
To: live-patching
Cc: Josh Poimboeuf, Song Liu, Jiri Kosina, Miroslav Benes,
Petr Mladek
In-Reply-To: <20260204025140.2023382-1-joe.lawrence@redhat.com>
The klp-build script is currently very strict with input patches,
requiring them to apply cleanly via `git apply --recount`. This
prevents the use of patches with minor contextual fuzz relative to the
target kernel sources.
To allow users to reuse a patch across similar kernel streams, switch to
using GNU patch and patchutils for intermediate patch manipulation.
Update the logic for applying, reverting, and regenerating patches:
- Use 'patch -p1' for better handling of context fuzz.
- Use 'recountdiff' to update line counts after FIX_PATCH_LINES.
- Drop git_refresh() and related git-specific logic.
Signed-off-by: Joe Lawrence <joe.lawrence@redhat.com>
---
scripts/livepatch/klp-build | 50 +++++++++++--------------------------
1 file changed, 14 insertions(+), 36 deletions(-)
I think this does simplify things, but:
- introduces a dependency on patchutil's recountdiff
- requires goofy epoch timestamp filtering as `diff -N` doesn't use
the `git diff` /dev/null, but a localized beginining of time epoch
that may be 1969 or 1970 depending on the local timezone
- can be *really* chatty, for example:
Validating patch(es)
patching file fs/proc/cmdline.c
Hunk #1 succeeded at 7 (offset 1 line).
Fixing patch(es)
patching file fs/proc/cmdline.c
Hunk #1 succeeded at 7 (offset 1 line).
patching file fs/proc/cmdline.c
patching file fs/proc/cmdline.c
Building patched kernel
Copying patched object files
Diffing objects
vmlinux.o: changed function: override_release
vmlinux.o: changed function: cmdline_proc_show
Building patch module: livepatch-cmdline-string.ko
SUCCESS
My initial thought was that I'd only be interested in knowing about
patch offset/fuzz during the validation phase. And in the interest of
clarifying some of the output messages, it would be nice to know the
patch it was referring to, so how about a follow up patch
pretty-formatting with some indentation like:
Validating patch(es)
cmdline-string.patch
patching file fs/proc/cmdline.c
Hunk #1 succeeded at 7 (offset 1 line).
Fixing patch(es)
Building patched kernel
Copying patched object files
Diffing objects
vmlinux.o: changed function: override_release
vmlinux.o: changed function: cmdline_proc_show
Building patch module: livepatch-cmdline-string.ko
SUCCESS
That said, Song suggested using --silent across the board, so maybe
tie that into the existing --verbose option?
diff --git a/scripts/livepatch/klp-build b/scripts/livepatch/klp-build
index 5a99ff4c4729..ee43a9caa107 100755
--- a/scripts/livepatch/klp-build
+++ b/scripts/livepatch/klp-build
@@ -96,7 +96,7 @@ restore_files() {
cleanup() {
set +o nounset
- revert_patches "--recount"
+ revert_patches
restore_files
[[ "$KEEP_TMP" -eq 0 ]] && rm -rf "$TMP_DIR"
return 0
@@ -285,7 +285,7 @@ set_module_name() {
}
# Hardcode the value printed by the localversion script to prevent patch
-# application from appending it with '+' due to a dirty git working tree.
+# application from appending it with '+' due to a dirty working tree.
set_kernelversion() {
local file="$SRC/scripts/setlocalversion"
local localversion
@@ -303,8 +303,8 @@ get_patch_input_files() {
local patch="$1"
grep0 -E '^--- ' "$patch" \
+ | grep -v -e '/dev/null' -e '1969-12-31' -e '1970-01-01' \
| gawk '{print $2}' \
- | grep -v '^/dev/null$' \
| sed 's|^[^/]*/||' \
| sort -u
}
@@ -313,8 +313,8 @@ get_patch_output_files() {
local patch="$1"
grep0 -E '^\+\+\+ ' "$patch" \
+ | grep -v -e '/dev/null' -e '1969-12-31' -e '1970-01-01' \
| gawk '{print $2}' \
- | grep -v '^/dev/null$' \
| sed 's|^[^/]*/||' \
| sort -u
}
@@ -326,21 +326,6 @@ get_patch_files() {
| sort -u
}
-# Make sure git re-stats the changed files
-git_refresh() {
- local patch="$1"
- local files=()
-
- [[ ! -e "$SRC/.git" ]] && return
-
- get_patch_input_files "$patch" | mapfile -t files
-
- (
- cd "$SRC"
- git update-index -q --refresh -- "${files[@]}"
- )
-}
-
check_unsupported_patches() {
local patch
@@ -361,18 +346,14 @@ check_unsupported_patches() {
apply_patch() {
local patch="$1"
- shift
- local extra_args=("$@")
[[ ! -f "$patch" ]] && die "$patch doesn't exist"
(
cd "$SRC"
-
- # The sed strips the version signature from 'git format-patch',
- # otherwise 'git apply --recount' warns.
- sed -n '/^-- /q;p' "$patch" |
- git apply "${extra_args[@]}"
+ # The sed strips the version signature from 'git format-patch'.
+ sed -n '/^-- /q;p' "$patch" | \
+ patch -p1 --no-backup-if-mismatch -r /dev/null
)
APPLIED_PATCHES+=("$patch")
@@ -380,17 +361,13 @@ apply_patch() {
revert_patch() {
local patch="$1"
- shift
- local extra_args=("$@")
local tmp=()
(
cd "$SRC"
-
- sed -n '/^-- /q;p' "$patch" |
- git apply --reverse "${extra_args[@]}"
+ sed -n '/^-- /q;p' "$patch" | \
+ patch -p1 -R --silent --no-backup-if-mismatch -r /dev/null
)
- git_refresh "$patch"
for p in "${APPLIED_PATCHES[@]}"; do
[[ "$p" == "$patch" ]] && continue
@@ -437,6 +414,7 @@ do_init() {
APPLIED_PATCHES=()
[[ -x "$FIX_PATCH_LINES" ]] || die "can't find fix-patch-lines"
+ command -v recountdiff &>/dev/null || die "recountdiff not found (install patchutils)"
validate_config
set_module_name
@@ -462,12 +440,12 @@ refresh_patch() {
( cd "$SRC" && echo "${input_files[@]}" | xargs cp --parents --target-directory="$tmpdir/a" )
# Copy patched source files to 'b'
- apply_patch "$patch" --recount
+ apply_patch "$patch"
( cd "$SRC" && echo "${output_files[@]}" | xargs cp --parents --target-directory="$tmpdir/b" )
- revert_patch "$patch" --recount
+ revert_patch "$patch"
# Diff 'a' and 'b' to make a clean patch
- ( cd "$tmpdir" && git diff --no-index --no-prefix a b > "$patch" ) || true
+ ( cd "$tmpdir" && diff -Nupr a b > "$patch" ) || true
}
# Copy the patches to a temporary directory, fix their lines so as not to
@@ -490,7 +468,7 @@ fix_patches() {
cp -f "$old_patch" "$tmp_patch"
refresh_patch "$tmp_patch"
- "$FIX_PATCH_LINES" "$tmp_patch" > "$new_patch"
+ "$FIX_PATCH_LINES" "$tmp_patch" | recountdiff > "$new_patch"
refresh_patch "$new_patch"
PATCHES[i]="$new_patch"
--
2.52.0
^ permalink raw reply related
* [PATCH v2 2/5] livepatch/klp-build: handle patches that add/remove files
From: Joe Lawrence @ 2026-02-04 2:51 UTC (permalink / raw)
To: live-patching
Cc: Josh Poimboeuf, Song Liu, Jiri Kosina, Miroslav Benes,
Petr Mladek
In-Reply-To: <20260204025140.2023382-1-joe.lawrence@redhat.com>
The klp-build script prepares a clean patch by populating two temporary
directories ('a' and 'b') with source files and diffing the result.
However, this process currently fails when a patch introduces a new
source file as the script attempts to copy files that do not yet exist
in the original source tree. Likewise, there is a similar limitation
when a patch removes a source file and the script tries to copy files
that no longer exist.
Refactor the file-gathering logic to distinguish between original input
files and patched output files:
- Split get_patch_files() into get_patch_input_files() and
get_patch_output_files() to identify which files exist before and
after patch application.
- Filter out "/dev/null" from both to handle file creation/deletion
- Update refresh_patch() to only copy existing input files to the 'a'
directory and the resulting output files to the 'b' directory.
This allows klp-build to successfully process patches that add or remove
source files.
Signed-off-by: Joe Lawrence <joe.lawrence@redhat.com>
---
scripts/livepatch/klp-build | 34 +++++++++++++++++++++++++++-------
1 file changed, 27 insertions(+), 7 deletions(-)
Lightly tested with patches that added or removed a source file, as
generated by `git diff`, `git format-patch`, and `diff -Nupr`.
diff --git a/scripts/livepatch/klp-build b/scripts/livepatch/klp-build
index 9f1b77c2b2b7..5a99ff4c4729 100755
--- a/scripts/livepatch/klp-build
+++ b/scripts/livepatch/klp-build
@@ -299,15 +299,33 @@ set_kernelversion() {
sed -i "2i echo $localversion; exit 0" scripts/setlocalversion
}
-get_patch_files() {
+get_patch_input_files() {
+ local patch="$1"
+
+ grep0 -E '^--- ' "$patch" \
+ | gawk '{print $2}' \
+ | grep -v '^/dev/null$' \
+ | sed 's|^[^/]*/||' \
+ | sort -u
+}
+
+get_patch_output_files() {
local patch="$1"
- grep0 -E '^(--- |\+\+\+ )' "$patch" \
+ grep0 -E '^\+\+\+ ' "$patch" \
| gawk '{print $2}' \
+ | grep -v '^/dev/null$' \
| sed 's|^[^/]*/||' \
| sort -u
}
+get_patch_files() {
+ local patch="$1"
+
+ { get_patch_input_files "$patch"; get_patch_output_files "$patch"; } \
+ | sort -u
+}
+
# Make sure git re-stats the changed files
git_refresh() {
local patch="$1"
@@ -315,7 +333,7 @@ git_refresh() {
[[ ! -e "$SRC/.git" ]] && return
- get_patch_files "$patch" | mapfile -t files
+ get_patch_input_files "$patch" | mapfile -t files
(
cd "$SRC"
@@ -429,21 +447,23 @@ do_init() {
refresh_patch() {
local patch="$1"
local tmpdir="$PATCH_TMP_DIR"
- local files=()
+ local input_files=()
+ local output_files=()
rm -rf "$tmpdir"
mkdir -p "$tmpdir/a"
mkdir -p "$tmpdir/b"
# Get all source files affected by the patch
- get_patch_files "$patch" | mapfile -t files
+ get_patch_input_files "$patch" | mapfile -t input_files
+ get_patch_output_files "$patch" | mapfile -t output_files
# Copy orig source files to 'a'
- ( cd "$SRC" && echo "${files[@]}" | xargs cp --parents --target-directory="$tmpdir/a" )
+ ( cd "$SRC" && echo "${input_files[@]}" | xargs cp --parents --target-directory="$tmpdir/a" )
# Copy patched source files to 'b'
apply_patch "$patch" --recount
- ( cd "$SRC" && echo "${files[@]}" | xargs cp --parents --target-directory="$tmpdir/b" )
+ ( cd "$SRC" && echo "${output_files[@]}" | xargs cp --parents --target-directory="$tmpdir/b" )
revert_patch "$patch" --recount
# Diff 'a' and 'b' to make a clean patch
--
2.52.0
^ permalink raw reply related
* [PATCH v2 1/5] objtool/klp: Fix mkstemp() failure with long paths
From: Joe Lawrence @ 2026-02-04 2:51 UTC (permalink / raw)
To: live-patching
Cc: Josh Poimboeuf, Song Liu, Jiri Kosina, Miroslav Benes,
Petr Mladek
In-Reply-To: <20260204025140.2023382-1-joe.lawrence@redhat.com>
klp-build's elf_create_file() fails with EINVAL when the build directory
path is long enough to truncate the "XXXXXX" suffix in the 256-byte
tmp_name buffer. Increase the buffer to PATH_MAX and add a truncation
check to ensure a valid template string for mkstemp().
Signed-off-by: Joe Lawrence <joe.lawrence@redhat.com>
---
tools/objtool/elf.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
Repro notes:
# Consider a looooong path like:
$ LONG_DIR=tests/gitlab.com/joe.lawrence/kernel-tests/-/archive/klp-build/kernel-tests-klp-build.tar.gz/general/kpatch/patch-upgrade/kpatch/kernel/BUILD/kernel-6.12.0-178.1964_2250006255.el10/linux-6.12.0-178.1964_2250006255.el10.x86_64
# Place the source repo within the long path
$ mkdir -p /tmp/"$LONG_DIR"
$ cd /tmp/"$LONG_DIR"
$ git clone --depth=1 --branch=v6.19-rc4 git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
$ cd linux
# Grab a sample patch
$ wget https://raw.githubusercontent.com/dynup/kpatch/refs/heads/master/examples/cmdline-string.patch
# Basic config for livepatching ...
$ make -j$(nproc) defconfig
$ ./scripts/config --file .config \
--set-val CONFIG_FTRACE y \
--set-val CONFIG_KALLSYMS_ALL y \
--set-val CONFIG_FUNCTION_TRACER y \
--set-val CONFIG_DYNAMIC_FTRACE y \
--set-val CONFIG_DYNAMIC_DEBUG y \
--set-val CONFIG_LIVEPATCH y
$ make olddefconfig
# BAD BUILD, mkstemp() unhappy:
$ ./scripts/livepatch/klp-build -T cmdline-string.patch
Validating patch(es)
Building original kernel
Copying original object files
Fixing patch(es)
Building patched kernel
Copying patched object files
Diffing objects
vmlinux.o: changed function: cmdline_proc_show
vmlinux.o: error: objtool [elf.c:1233]: elf_create_file: can't create tmp file failed: Invalid argument
error: klp-build: objtool klp diff failed
error: klp-build: line 657: '( cd "$ORIG_DIR"; "${cmd[@]}" > >(tee -a "$log") 2> >(tee -a "$log" | "${filter[@]}" 1>&2) || die "objtool klp diff failed" )'
# GOOD BUILD, with PATH_MAX buffer for mkstemp():
$ ./scripts/livepatch/klp-build -S 3 -T cmdline-string.patch
Diffing objects
vmlinux.o: changed function: cmdline_proc_show
Building patch module: livepatch-cmdline-string.ko
SUCCESS
diff --git a/tools/objtool/elf.c b/tools/objtool/elf.c
index 2c02c7b49265..836575876741 100644
--- a/tools/objtool/elf.c
+++ b/tools/objtool/elf.c
@@ -15,6 +15,7 @@
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
+#include <limits.h>
#include <errno.h>
#include <libgen.h>
#include <ctype.h>
@@ -1192,6 +1193,7 @@ struct elf *elf_create_file(GElf_Ehdr *ehdr, const char *name)
char *dir, *base, *tmp_name;
struct symbol *sym;
struct elf *elf;
+ int path_len;
elf_version(EV_CURRENT);
@@ -1219,13 +1221,17 @@ struct elf *elf_create_file(GElf_Ehdr *ehdr, const char *name)
base = basename(base);
- tmp_name = malloc(256);
+ tmp_name = malloc(PATH_MAX);
if (!tmp_name) {
ERROR_GLIBC("malloc");
return NULL;
}
- snprintf(tmp_name, 256, "%s/%s.XXXXXX", dir, base);
+ path_len = snprintf(tmp_name, PATH_MAX, "%s/%s.XXXXXX", dir, base);
+ if (path_len >= PATH_MAX) {
+ ERROR_GLIBC("snprintf");
+ return NULL;
+ }
elf->fd = mkstemp(tmp_name);
if (elf->fd == -1) {
--
2.52.0
^ permalink raw reply related
* [PATCH v2 0/5] livepatch-klp-build: small fixups and enhancements
From: Joe Lawrence @ 2026-02-04 2:51 UTC (permalink / raw)
To: live-patching
Cc: Josh Poimboeuf, Song Liu, Jiri Kosina, Miroslav Benes,
Petr Mladek
Here is v2, a quick iteration on v1 posted lasted week. I expect some
comments on verbosity and some code stylings, so fully expect to roll at
least another version.
See also some per-patch inline diffstat area commentary.
v2:
- Update patch subject prefixes accordingly [Josh]
- Added a small objtool/klp patch. Test systems setup crazy long
pathnames :D
- Removed patch ("limit parent .git directory search") as this version
replaces the use of git apply --recount with patch and recountdiff.
A side effect of this simplification was no longer needing this weird
hack. [Josh]
- Updated the patch that handles input patches that add files to also
support removing files, implement this by directly inspecting the
.patch +++ and --- header lines via two file lists [Josh]
- Implement two short-circuiting updates: validate patches for steps 1
and 2, and allow the user to omit patches for steps 3 and 4. This
combines the original 'fail-fast' patch and some related notes on the
v1 thread. [Josh]
- Since v2 replaces git apply with patch and recountdiff, there is no
need for a -z/--fuzz argument, it comes with GNU patch for free.
v1: https://lore.kernel.org/live-patching/CAPhsuW5qrueccM123YbTo2ZvP-Rf+0UT-goG6c5A8gXw7BsF3w@mail.gmail.com/T/#t
Joe Lawrence (5):
objtool/klp: Fix mkstemp() failure with long paths
livepatch/klp-build: handle patches that add/remove files
livepatch/klp-build: switch to GNU patch and recountdiff
livepatch/klp-build: minor short-circuiting tweaks
livepatch/klp-build: provide friendlier error messages
scripts/livepatch/klp-build | 92 ++++++++++++++++++++-----------------
tools/objtool/elf.c | 10 +++-
2 files changed, 58 insertions(+), 44 deletions(-)
--
2.52.0
^ permalink raw reply
* Re: [PATCH] klp: use stop machine to check and expedite transition for running tasks
From: Li Zhe @ 2026-02-04 2:47 UTC (permalink / raw)
To: jpoimboe
Cc: jikos, joe.lawrence, linux-kernel, live-patching, lizhe.67,
mbenes, peterz, pmladek, qirui.001
In-Reply-To: <5vctv762jvnxiselc3vwattfsgegw6uv7kltsp27qtoajel2rl@kjrg4ko74gcn>
On Tue, 3 Feb 2026 18:20:22 -0800, jpoimboe@kernel.org wrote:
> On Mon, Feb 02, 2026 at 05:13:34PM +0800, Li Zhe wrote:
> > In the current KLP transition implementation, the strategy for running
> > tasks relies on waiting for a context switch to attempt to clear the
> > TIF_PATCH_PENDING flag. Alternatively, determine whether the
> > TIF_PATCH_PENDING flag can be cleared by inspecting the stack once the
> > process has yielded the CPU. However, this approach proves problematic
> > in certain environments.
> >
> > Consider a scenario where the majority of system CPUs are configured
> > with nohzfull and isolcpus, each dedicated to a VM with a vCPU pinned
> > to that physical core and configured with idle=poll within the guest.
> > Under such conditions, these vCPUs rarely leave the CPU. Combined with
> > the high core counts typical of modern server platforms, this results
> > in transition completion times that are not only excessively prolonged
> > but also highly unpredictable.
> >
> > This patch resolves this issue by registering a callback with
> > stop_machine. The callback attempts to transition the associated running
> > task. In a VM environment configured with 32 CPUs, the live patching
> > operation completes promptly after the SIGNALS_TIMEOUT period with this
> > patch applied; without it, the process nearly fails to complete under
> > the same scenario.
> >
> > Co-developed-by: Rui Qi <qirui.001@bytedance.com>
> > Signed-off-by: Rui Qi <qirui.001@bytedance.com>
> > Signed-off-by: Li Zhe <lizhe.67@bytedance.com>
>
> PeterZ, what's your take on this?
>
> I wonder if we could instead do resched_cpu() or something similar to
> trigger the call to klp_sched_try_switch() in __schedule()?
klp_sched_try_switch() only invokes __klp_sched_try_switch() after
verifying that the corresponding task has the TASK_FREEZABLE flag
set. I remain uncertain whether this approach adequately resolves
the issue.
Thanks,
Zhe
^ permalink raw reply
* Re: [PATCH] klp: use stop machine to check and expedite transition for running tasks
From: Josh Poimboeuf @ 2026-02-04 2:20 UTC (permalink / raw)
To: Li Zhe
Cc: jikos, mbenes, pmladek, joe.lawrence, live-patching, linux-kernel,
qirui.001, Peter Zijlstra
In-Reply-To: <20260202091334.60881-1-lizhe.67@bytedance.com>
On Mon, Feb 02, 2026 at 05:13:34PM +0800, Li Zhe wrote:
> In the current KLP transition implementation, the strategy for running
> tasks relies on waiting for a context switch to attempt to clear the
> TIF_PATCH_PENDING flag. Alternatively, determine whether the
> TIF_PATCH_PENDING flag can be cleared by inspecting the stack once the
> process has yielded the CPU. However, this approach proves problematic
> in certain environments.
>
> Consider a scenario where the majority of system CPUs are configured
> with nohzfull and isolcpus, each dedicated to a VM with a vCPU pinned
> to that physical core and configured with idle=poll within the guest.
> Under such conditions, these vCPUs rarely leave the CPU. Combined with
> the high core counts typical of modern server platforms, this results
> in transition completion times that are not only excessively prolonged
> but also highly unpredictable.
>
> This patch resolves this issue by registering a callback with
> stop_machine. The callback attempts to transition the associated running
> task. In a VM environment configured with 32 CPUs, the live patching
> operation completes promptly after the SIGNALS_TIMEOUT period with this
> patch applied; without it, the process nearly fails to complete under
> the same scenario.
>
> Co-developed-by: Rui Qi <qirui.001@bytedance.com>
> Signed-off-by: Rui Qi <qirui.001@bytedance.com>
> Signed-off-by: Li Zhe <lizhe.67@bytedance.com>
PeterZ, what's your take on this?
I wonder if we could instead do resched_cpu() or something similar to
trigger the call to klp_sched_try_switch() in __schedule()?
> ---
> kernel/livepatch/transition.c | 62 ++++++++++++++++++++++++++++++++---
> 1 file changed, 58 insertions(+), 4 deletions(-)
>
> diff --git a/kernel/livepatch/transition.c b/kernel/livepatch/transition.c
> index 2351a19ac2a9..9c078b9bd755 100644
> --- a/kernel/livepatch/transition.c
> +++ b/kernel/livepatch/transition.c
> @@ -10,6 +10,7 @@
> #include <linux/cpu.h>
> #include <linux/stacktrace.h>
> #include <linux/static_call.h>
> +#include <linux/stop_machine.h>
> #include "core.h"
> #include "patch.h"
> #include "transition.h"
> @@ -297,6 +298,61 @@ static int klp_check_and_switch_task(struct task_struct *task, void *arg)
> return 0;
> }
>
> +enum klp_stop_work_bit {
> + KLP_STOP_WORK_PENDING_BIT,
> +};
> +
> +struct klp_stop_work_info {
> + struct task_struct *task;
> + unsigned long flag;
> +};
> +
> +static DEFINE_PER_CPU(struct cpu_stop_work, klp_transition_stop_work);
> +static DEFINE_PER_CPU(struct klp_stop_work_info, klp_stop_work_info);
> +
> +static int klp_check_task(struct task_struct *task, void *old_name)
> +{
> + if (task == current)
> + return klp_check_and_switch_task(current, old_name);
> + else
> + return task_call_func(task, klp_check_and_switch_task, old_name);
> +}
> +
> +static int klp_transition_stop_work_fn(void *arg)
> +{
> + struct klp_stop_work_info *info = (struct klp_stop_work_info *)arg;
> + struct task_struct *task = info->task;
> + const char *old_name;
> +
> + clear_bit(KLP_STOP_WORK_PENDING_BIT, &info->flag);
> +
> + if (likely(klp_patch_pending(task)))
> + klp_check_task(task, &old_name);
> +
> + put_task_struct(task);
> +
> + return 0;
> +}
> +
> +static void klp_try_transition_running_task(struct task_struct *task)
> +{
> + int cpu = task_cpu(task);
> +
> + if (klp_signals_cnt && !(klp_signals_cnt % SIGNALS_TIMEOUT)) {
> + struct klp_stop_work_info *info =
> + per_cpu_ptr(&klp_stop_work_info, cpu);
> +
> + if (test_and_set_bit(KLP_STOP_WORK_PENDING_BIT, &info->flag))
> + return;
> +
> + info->task = get_task_struct(task);
> + if (!stop_one_cpu_nowait(cpu, klp_transition_stop_work_fn, info,
> + per_cpu_ptr(&klp_transition_stop_work,
> + cpu)))
> + put_task_struct(task);
> + }
> +}
> +
> /*
> * Try to safely switch a task to the target patch state. If it's currently
> * running, or it's sleeping on a to-be-patched or to-be-unpatched function, or
> @@ -323,10 +379,7 @@ static bool klp_try_switch_task(struct task_struct *task)
> * functions. If all goes well, switch the task to the target patch
> * state.
> */
> - if (task == current)
> - ret = klp_check_and_switch_task(current, &old_name);
> - else
> - ret = task_call_func(task, klp_check_and_switch_task, &old_name);
> + ret = klp_check_task(task, &old_name);
>
> switch (ret) {
> case 0: /* success */
> @@ -335,6 +388,7 @@ static bool klp_try_switch_task(struct task_struct *task)
> case -EBUSY: /* klp_check_and_switch_task() */
> pr_debug("%s: %s:%d is running\n",
> __func__, task->comm, task->pid);
> + klp_try_transition_running_task(task);
> break;
> case -EINVAL: /* klp_check_and_switch_task() */
> pr_debug("%s: %s:%d has an unreliable stack\n",
> --
> 2.20.1
--
Josh
^ permalink raw reply
* Re: [PATCH] klp-build: Update demangle_name for LTO
From: Josh Poimboeuf @ 2026-02-04 1:24 UTC (permalink / raw)
To: Song Liu; +Cc: live-patching, kernel-team, jikos, mbenes, pmladek, joe.lawrence
In-Reply-To: <CAPhsuW7tSyGVBBMOV2bc7gvRXCUbeEETUM7qcZ4HU+Z3D=8SEQ@mail.gmail.com>
On Tue, Feb 03, 2026 at 04:24:06PM -0800, Song Liu wrote:
> On Tue, Feb 3, 2026 at 3:53 PM Josh Poimboeuf <jpoimboe@kernel.org> wrote:
> >
> > On Tue, Feb 03, 2026 at 01:40:06PM -0800, Song Liu wrote:
> > > With CONFIG_LTO_CLANG_THIN, __UNIQUE_ID_* can be global. Therefore, it
> > > is necessary to demangle global symbols.
> >
> > Ouch, so LTO is changing symbol bindings :-/
> >
> > If a patch causes a symbol to change from LOCAL to GLOBAL between the
> > original and patched builds, that will break some fundamental
> > assumptions in the correlation logic.
>
> This can indeed happen. A function can be "LOCAL DEFAULT" XXXX
> in original, and "GLOBAL HIDDEN" XXXX.llvm.<hash> in patched.
>
> I am trying to fix this with incremental steps.
>
> > Also, notice sym->demangled_name isn't used for correlating global
> > symbols in correlate_symbols(). That code currently assumes all global
> > symbols are uniquely named (and don't change between orig and patched).
> > So this first fix seems incomplete.
>
> We still need to fix correlate_symbols(). I am not 100% sure how to do
> that part yet.
>
> OTOH, this part still helps. This is because checksum_update_insn()
> uses demangled_name. After the fix, if a function is renamed from
> XXXX to XXXX.llvm.<hash> after the patch, functions that call the
> function are not considered changed.
Hm, wouldn't that still leave the .llvm at the end?
>
> > > Also, LTO may generate symbols like:
> >
> > The "also" is a clue that this should probably be two separate patches.
> >
> > Also, for objtool patches, please prefix the subject with "objtool:", or
> > in this case, for klp-specific code, "objtool/klp:".
> >
> > > __UNIQUE_ID_addressable___UNIQUE_ID_pci_invalid_bar_694_695
> > >
> > > Remove trailing '_' together with numbers and '.' so that both numbers
> > > added to the end of the symbol are removed. For example, the above s
> > > ymbol will be demangled as
> > >
> > > __UNIQUE_ID_addressable___UNIQUE_ID_pci_invalid_bar
> >
> > This is indeed a bug in demangle_name(), but not specific to LTO. The
> > unique number is added by the __UNIQUE_ID() macro.
> >
> > I guess in this case LTO is doing some kind of nested __UNIQUE_ID() to
> > get two "__UNIQUE_ID" strings and two numbers? But the bug still exists
> > for the non-nested case.
>
> I don't see nested __UNIQUE_ID() without LTO. Both gcc and clang without
> LTO only sees one level of __UNIQUE_ID.
>
> With one level of __UNIQUE_ID(), existing code works fine. We just get one
> extra "_" at the end of the demanged_name.
Ah, I see.
--
Josh
^ permalink raw reply
* Re: [PATCH] klp-build: Update demangle_name for LTO
From: Song Liu @ 2026-02-04 0:24 UTC (permalink / raw)
To: Josh Poimboeuf
Cc: live-patching, kernel-team, jikos, mbenes, pmladek, joe.lawrence
In-Reply-To: <3rufoy2rjvt4apzwplyn6g6cafrz5hxh2b2ug3cmljndctauo5@bskwjecforne>
On Tue, Feb 3, 2026 at 3:53 PM Josh Poimboeuf <jpoimboe@kernel.org> wrote:
>
> On Tue, Feb 03, 2026 at 01:40:06PM -0800, Song Liu wrote:
> > With CONFIG_LTO_CLANG_THIN, __UNIQUE_ID_* can be global. Therefore, it
> > is necessary to demangle global symbols.
>
> Ouch, so LTO is changing symbol bindings :-/
>
> If a patch causes a symbol to change from LOCAL to GLOBAL between the
> original and patched builds, that will break some fundamental
> assumptions in the correlation logic.
This can indeed happen. A function can be "LOCAL DEFAULT" XXXX
in original, and "GLOBAL HIDDEN" XXXX.llvm.<hash> in patched.
I am trying to fix this with incremental steps.
> Also, notice sym->demangled_name isn't used for correlating global
> symbols in correlate_symbols(). That code currently assumes all global
> symbols are uniquely named (and don't change between orig and patched).
> So this first fix seems incomplete.
We still need to fix correlate_symbols(). I am not 100% sure how to do
that part yet.
OTOH, this part still helps. This is because checksum_update_insn()
uses demangled_name. After the fix, if a function is renamed from
XXXX to XXXX.llvm.<hash> after the patch, functions that call the
function are not considered changed.
> > Also, LTO may generate symbols like:
>
> The "also" is a clue that this should probably be two separate patches.
>
> Also, for objtool patches, please prefix the subject with "objtool:", or
> in this case, for klp-specific code, "objtool/klp:".
>
> > __UNIQUE_ID_addressable___UNIQUE_ID_pci_invalid_bar_694_695
> >
> > Remove trailing '_' together with numbers and '.' so that both numbers
> > added to the end of the symbol are removed. For example, the above s
> > ymbol will be demangled as
> >
> > __UNIQUE_ID_addressable___UNIQUE_ID_pci_invalid_bar
>
> This is indeed a bug in demangle_name(), but not specific to LTO. The
> unique number is added by the __UNIQUE_ID() macro.
>
> I guess in this case LTO is doing some kind of nested __UNIQUE_ID() to
> get two "__UNIQUE_ID" strings and two numbers? But the bug still exists
> for the non-nested case.
I don't see nested __UNIQUE_ID() without LTO. Both gcc and clang without
LTO only sees one level of __UNIQUE_ID.
With one level of __UNIQUE_ID(), existing code works fine. We just get one
extra "_" at the end of the demanged_name.
>
> > Signed-off-by: Song Liu <song@kernel.org>
> > ---
> > tools/objtool/elf.c | 11 +++++++----
> > 1 file changed, 7 insertions(+), 4 deletions(-)
> >
> > diff --git a/tools/objtool/elf.c b/tools/objtool/elf.c
> > index 2c02c7b49265..b4a7ea4720e1 100644
> > --- a/tools/objtool/elf.c
> > +++ b/tools/objtool/elf.c
> > @@ -445,9 +445,6 @@ static const char *demangle_name(struct symbol *sym)
> > {
> > char *str;
> >
> > - if (!is_local_sym(sym))
> > - return sym->name;
> > -
> > if (!is_func_sym(sym) && !is_object_sym(sym))
> > return sym->name;
> >
> > @@ -463,7 +460,13 @@ static const char *demangle_name(struct symbol *sym)
> > for (int i = strlen(str) - 1; i >= 0; i--) {
> > char c = str[i];
> >
> > - if (!isdigit(c) && c != '.') {
> > + /*
> > + * With CONFIG_LTO_CLANG_THIN, the UNIQUE_ID field could
> > + * be like:
> > + * __UNIQUE_ID_addressable___UNIQUE_ID_<name>_628_629
> > + * Remove all the trailing number, '.', and '_'.
> > + */
>
> A comment is indeed probably warranted, though I'm thinking it should
> instead go above the function, with examples of both __UNIQUE_ID and "."
> symbols.
I will split the patch into two, and fix the comment and commit log.
Thanks,
Song
^ permalink raw reply
* Re: [PATCH] klp-build: Update demangle_name for LTO
From: Josh Poimboeuf @ 2026-02-03 23:52 UTC (permalink / raw)
To: Song Liu; +Cc: live-patching, kernel-team, jikos, mbenes, pmladek, joe.lawrence
In-Reply-To: <20260203214006.741364-1-song@kernel.org>
On Tue, Feb 03, 2026 at 01:40:06PM -0800, Song Liu wrote:
> With CONFIG_LTO_CLANG_THIN, __UNIQUE_ID_* can be global. Therefore, it
> is necessary to demangle global symbols.
Ouch, so LTO is changing symbol bindings :-/
If a patch causes a symbol to change from LOCAL to GLOBAL between the
original and patched builds, that will break some fundamental
assumptions in the correlation logic.
Also, notice sym->demangled_name isn't used for correlating global
symbols in correlate_symbols(). That code currently assumes all global
symbols are uniquely named (and don't change between orig and patched).
So this first fix seems incomplete.
> Also, LTO may generate symbols like:
The "also" is a clue that this should probably be two separate patches.
Also, for objtool patches, please prefix the subject with "objtool:", or
in this case, for klp-specific code, "objtool/klp:".
> __UNIQUE_ID_addressable___UNIQUE_ID_pci_invalid_bar_694_695
>
> Remove trailing '_' together with numbers and '.' so that both numbers
> added to the end of the symbol are removed. For example, the above s
> ymbol will be demangled as
>
> __UNIQUE_ID_addressable___UNIQUE_ID_pci_invalid_bar
This is indeed a bug in demangle_name(), but not specific to LTO. The
unique number is added by the __UNIQUE_ID() macro.
I guess in this case LTO is doing some kind of nested __UNIQUE_ID() to
get two "__UNIQUE_ID" strings and two numbers? But the bug still exists
for the non-nested case.
> Signed-off-by: Song Liu <song@kernel.org>
> ---
> tools/objtool/elf.c | 11 +++++++----
> 1 file changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/tools/objtool/elf.c b/tools/objtool/elf.c
> index 2c02c7b49265..b4a7ea4720e1 100644
> --- a/tools/objtool/elf.c
> +++ b/tools/objtool/elf.c
> @@ -445,9 +445,6 @@ static const char *demangle_name(struct symbol *sym)
> {
> char *str;
>
> - if (!is_local_sym(sym))
> - return sym->name;
> -
> if (!is_func_sym(sym) && !is_object_sym(sym))
> return sym->name;
>
> @@ -463,7 +460,13 @@ static const char *demangle_name(struct symbol *sym)
> for (int i = strlen(str) - 1; i >= 0; i--) {
> char c = str[i];
>
> - if (!isdigit(c) && c != '.') {
> + /*
> + * With CONFIG_LTO_CLANG_THIN, the UNIQUE_ID field could
> + * be like:
> + * __UNIQUE_ID_addressable___UNIQUE_ID_<name>_628_629
> + * Remove all the trailing number, '.', and '_'.
> + */
A comment is indeed probably warranted, though I'm thinking it should
instead go above the function, with examples of both __UNIQUE_ID and "."
symbols.
> + if (!isdigit(c) && c != '.' && c != '_') {
Ack.
--
Josh
^ permalink raw reply
* [PATCH] klp-build: Update demangle_name for LTO
From: Song Liu @ 2026-02-03 21:40 UTC (permalink / raw)
To: live-patching
Cc: jpoimboe, kernel-team, jikos, mbenes, pmladek, joe.lawrence,
Song Liu
With CONFIG_LTO_CLANG_THIN, __UNIQUE_ID_* can be global. Therefore, it
is necessary to demangle global symbols.
Also, LTO may generate symbols like:
__UNIQUE_ID_addressable___UNIQUE_ID_pci_invalid_bar_694_695
Remove trailing '_' together with numbers and '.' so that both numbers
added to the end of the symbol are removed. For example, the above s
ymbol will be demangled as
__UNIQUE_ID_addressable___UNIQUE_ID_pci_invalid_bar
Signed-off-by: Song Liu <song@kernel.org>
---
tools/objtool/elf.c | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/tools/objtool/elf.c b/tools/objtool/elf.c
index 2c02c7b49265..b4a7ea4720e1 100644
--- a/tools/objtool/elf.c
+++ b/tools/objtool/elf.c
@@ -445,9 +445,6 @@ static const char *demangle_name(struct symbol *sym)
{
char *str;
- if (!is_local_sym(sym))
- return sym->name;
-
if (!is_func_sym(sym) && !is_object_sym(sym))
return sym->name;
@@ -463,7 +460,13 @@ static const char *demangle_name(struct symbol *sym)
for (int i = strlen(str) - 1; i >= 0; i--) {
char c = str[i];
- if (!isdigit(c) && c != '.') {
+ /*
+ * With CONFIG_LTO_CLANG_THIN, the UNIQUE_ID field could
+ * be like:
+ * __UNIQUE_ID_addressable___UNIQUE_ID_<name>_628_629
+ * Remove all the trailing number, '.', and '_'.
+ */
+ if (!isdigit(c) && c != '.' && c != '_') {
str[i + 1] = '\0';
break;
}
--
2.47.3
^ permalink raw reply related
* Re: [PATCH 3/5] objtool/klp: validate patches with git apply --recount
From: Josh Poimboeuf @ 2026-02-03 19:47 UTC (permalink / raw)
To: Song Liu
Cc: Joe Lawrence, live-patching, Jiri Kosina, Miroslav Benes,
Petr Mladek
In-Reply-To: <CAPhsuW4oQfWTPd7jQKhd8Ff-9gWW9GMKyA9HpUCxF2F0KXecEA@mail.gmail.com>
On Tue, Feb 03, 2026 at 09:53:04AM -0800, Song Liu wrote:
> On Tue, Feb 3, 2026 at 8:45 AM Joe Lawrence <joe.lawrence@redhat.com> wrote:
> [...]
> > > Or at least validate_patches() could be replaced with
> > > check_unsupported_patches(), as the apply/revert test wouldn't be needed
> > > since the actual apply/revert would happen immediately after that in
> > > fix_patches().
> > >
> >
> > Currently fix_patches runs in short-circuit step (2) after building the
> > original kernel. But what if the user runs:
> >
> > $ klp-build -T 0001.patch
> > $ klp-build -S 2 0002.patch
>
> On one hand, I think this is a user mistake that we need the users
> to avoid by themselves. If the user do
>
> $ klp-build -T 0001.patch
> $ klp-build -S 3 0002.patch
>
> Even when 0001.patch and 0002.patch are totally valid, the end
> result will be very confusing (it is the result of 0001.patch, not 0002).
Right, I consider it a power tool, use at your own discretion...
starting at step 2 overwrites the previous step 2.
> > If we move fix_patches() to step (1) to fail fast and eliminate a
> > redundant apply/revert, aren't we then going to miss it if the user
> > jumps to step (2)?
fix_patches() would still *conceptually* be part of step 2. But as an
implementation detail (so that it fail fasts with a bad patch), step 2
would be split into two phases: before step 1 and after step 1.
if (( SHORT_CIRCUIT <= 2 )); then
# validate and fix patches
fi
if (( SHORT_CIRCUIT <= 1 )); then
# build orig kernel
fi
if (( SHORT_CIRCUIT <= 2 )); then
# apply patches and build patched kernel
fi
> > Is there a way to check without actually doing it if we're going to
> > build the original kernel first?
> >
> > And while we're here, doesn't this mean that we're currently not running
> > validate_patches() when skipping to step (2)?
No, it would still run for -S2, see above.
> On the other hand, I guess we can always run fix_patches. If any
> -S is given, we compare the fixed new patches against fixed saved
> patches. If they are not identical, we fail fast.
I don't think that's worth the trouble. If you want to change the
patches, re-run step 2. Otherwise, just skip to step 3 or 4 (where the
patch argument just gets ignored). Again, just a power tool for those
of us who are impatient ;-)
--
Josh
^ permalink raw reply
* Re: [PATCH 3/5] objtool/klp: validate patches with git apply --recount
From: Song Liu @ 2026-02-03 17:53 UTC (permalink / raw)
To: Joe Lawrence
Cc: Josh Poimboeuf, live-patching, Jiri Kosina, Miroslav Benes,
Petr Mladek
In-Reply-To: <604a8b96-47f2-4986-b602-c7bdf3de7cca@redhat.com>
On Tue, Feb 3, 2026 at 8:45 AM Joe Lawrence <joe.lawrence@redhat.com> wrote:
[...]
> > Or at least validate_patches() could be replaced with
> > check_unsupported_patches(), as the apply/revert test wouldn't be needed
> > since the actual apply/revert would happen immediately after that in
> > fix_patches().
> >
>
> Currently fix_patches runs in short-circuit step (2) after building the
> original kernel. But what if the user runs:
>
> $ klp-build -T 0001.patch
> $ klp-build -S 2 0002.patch
On one hand, I think this is a user mistake that we need the users
to avoid by themselves. If the user do
$ klp-build -T 0001.patch
$ klp-build -S 3 0002.patch
Even when 0001.patch and 0002.patch are totally valid, the end
result will be very confusing (it is the result of 0001.patch, not 0002).
> If we move fix_patches() to step (1) to fail fast and eliminate a
> redundant apply/revert, aren't we then going to miss it if the user
> jumps to step (2)?
>
> Is there a way to check without actually doing it if we're going to
> build the original kernel first?
>
> And while we're here, doesn't this mean that we're currently not running
> validate_patches() when skipping to step (2)?
On the other hand, I guess we can always run fix_patches. If any
-S is given, we compare the fixed new patches against fixed saved
patches. If they are not identical, we fail fast.
Thanks,
Song
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox