[PATCH v2 0/5] livepatch-klp-build: small fixups and enhancements

[PATCH v2 0/5] livepatch-klp-build: small fixups and enhancements

[Joe Lawrence]

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

[PATCH v2 1/5] objtool/klp: Fix mkstemp() failure with long paths

[Joe Lawrence]

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

[PATCH v2 2/5] livepatch/klp-build: handle patches that add/remove files

[Joe Lawrence]

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

[PATCH v2 3/5] livepatch/klp-build: switch to GNU patch and recountdiff

[Joe Lawrence]

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

[PATCH v2 4/5] livepatch/klp-build: minor short-circuiting tweaks

[Joe Lawrence]

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

[PATCH v2 5/5] livepatch/klp-build: provide friendlier error messages

[Joe Lawrence]

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

Re: [PATCH v2 3/5] livepatch/klp-build: switch to GNU patch and recountdiff

[Joe Lawrence]

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

Re: [PATCH v2 1/5] objtool/klp: Fix mkstemp() failure with long paths

[Josh Poimboeuf]

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

Re: [PATCH v2 2/5] livepatch/klp-build: handle patches that add/remove files

[Josh Poimboeuf]

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

Re: [PATCH v2 3/5] livepatch/klp-build: switch to GNU patch and recountdiff

[Josh Poimboeuf]

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

Re: [PATCH v2 4/5] livepatch/klp-build: minor short-circuiting tweaks

[Josh Poimboeuf]

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

Re: [PATCH v2 1/5] objtool/klp: Fix mkstemp() failure with long paths

[Joe Lawrence]

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

Re: [PATCH v2 2/5] livepatch/klp-build: handle patches that add/remove files

[Joe Lawrence]

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

Re: [PATCH v2 2/5] livepatch/klp-build: handle patches that add/remove files

[Josh Poimboeuf]

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

Re: [PATCH v2 3/5] livepatch/klp-build: switch to GNU patch and recountdiff

[Joe Lawrence]

On Wed, Feb 04, 2026 at 10:35:07AM -0800, Josh Poimboeuf wrote:
> 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.
> 

Ok, I'll take a look at fix-patch-lines and see how complicated it might
turn out to incorporate a recount.

> >   - 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.
> 

To keep it succinct, the script could check for offset/fuzz and only
report it, including the "patching file ..." part, if there is any.

> 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.
> 

In the past I've used a little function like:

  indent() {
      local num="${1:-0}"
      sed "s/^/$(printf '%*s' "$num" '')/"
  }

so I could just pipe in echo or command output like: `./cmd | indent 2`.
Good enough or maybe you have one?

> 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.
> 

While I do kinda like the recent color coded output from the compilers,
I don't know if I'm ready for a full-color livepatch build experience :D

I wouldn't be against it, but my vote leans towards the indentation
since it leaves prettier log files, even if the color codes are filtered
out.  Then again, the color scheme bikeshedding we could look forward
to!

> >  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?
> 

I'll double check.  I recall having difficulties with recountdiff when
taking these out, but can't reproduce that by hand at the moment.

> > @@ -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?
> 

Yeah I think it's redundant as long as recountdiff doesn't emit
something weird, but if it did, it's probably already screwed up the
patch.

--
Joe

Re: [PATCH v2 4/5] livepatch/klp-build: minor short-circuiting tweaks

[Joe Lawrence]

On Wed, Feb 04, 2026 at 10:40:14AM -0800, Josh Poimboeuf wrote:
> 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 :-)
> 

Alrighty, I'll drop this part unless somebody asks for it.

--
Joe

Re: [PATCH v2 3/5] livepatch/klp-build: switch to GNU patch and recountdiff

[Josh Poimboeuf]

On Thu, Feb 05, 2026 at 12:27:25PM -0500, Joe Lawrence wrote:
> On Wed, Feb 04, 2026 at 10:35:07AM -0800, Josh Poimboeuf wrote:
> > On Tue, Feb 03, 2026 at 09:51:38PM -0500, Joe Lawrence wrote:
> > >   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.
> > 
> 
> To keep it succinct, the script could check for offset/fuzz and only
> report it, including the "patching file ..." part, if there is any.

Maybe?  Only if it's not too complicated.

> > 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.
> > 
> 
> In the past I've used a little function like:
> 
>   indent() {
>       local num="${1:-0}"
>       sed "s/^/$(printf '%*s' "$num" '')/"
>   }
> 
> so I could just pipe in echo or command output like: `./cmd | indent 2`.
> Good enough or maybe you have one?

Sounds good, it probably needs a "return true" at the end of the function.

> > 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.
> > 
> 
> While I do kinda like the recent color coded output from the compilers,
> I don't know if I'm ready for a full-color livepatch build experience :D
> 
> I wouldn't be against it, but my vote leans towards the indentation
> since it leaves prettier log files, even if the color codes are filtered
> out.  Then again, the color scheme bikeshedding we could look forward
> to!

:-)

-- 
Josh

Re: [PATCH v2 2/5] livepatch/klp-build: handle patches that add/remove files

[Joe Lawrence]

On Thu, Feb 05, 2026 at 08:53:42AM -0800, Josh Poimboeuf wrote:
> 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.
> > > 

How about defining our own grep in the script that intercepts the call
and throws an error:

  grep() {
  	echo "error: $SCRIPT: use grep0 or 'command grep' instead of bare grep" >&2
  	exit 1
  }

That seems easier than trying to externally parse the script to figure
out what's a command, comment, word-match, legit grep, etc.

--
Joe

Re: [PATCH v2 2/5] livepatch/klp-build: handle patches that add/remove files

[Josh Poimboeuf]

On Tue, Feb 10, 2026 at 02:54:59PM -0500, Joe Lawrence wrote:
> On Thu, Feb 05, 2026 at 08:53:42AM -0800, Josh Poimboeuf wrote:
> > 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.
> > > > 
> 
> How about defining our own grep in the script that intercepts the call
> and throws an error:
> 
>   grep() {
>   	echo "error: $SCRIPT: use grep0 or 'command grep' instead of bare grep" >&2
>   	exit 1
>   }
> 
> That seems easier than trying to externally parse the script to figure
> out what's a command, comment, word-match, legit grep, etc.

Ack, sounds good.

-- 
Josh