Add a klp-build script which automates the generation of a livepatch
module from a source .patch file by performing the following steps:

  - Builds an original kernel with -function-sections and
    -fdata-sections, plus objtool function checksumming.

  - Applies the .patch file and rebuilds the kernel using the same
    options.

  - Runs 'objtool klp diff' to detect changed functions and generate
    intermediate binary diff objects.

  - Builds a kernel module which links the diff objects with some
    livepatch module init code (scripts/livepatch/init.c).

  - Finalizes the livepatch module (aka work around linker wreckage)
    using 'objtool klp post-link'.

Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
---
 scripts/livepatch/klp-build | 697 ++++++++++++++++++++++++++++++++++++
 1 file changed, 697 insertions(+)
 create mode 100755 scripts/livepatch/klp-build

diff --git a/scripts/livepatch/klp-build b/scripts/livepatch/klp-build
new file mode 100755
index 000000000000..ebbece6f6b8d
--- /dev/null
+++ b/scripts/livepatch/klp-build
@@ -0,0 +1,697 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0
+#
+# Build a livepatch module
+#
+
+# shellcheck disable=SC1090
+
+if (( BASH_VERSINFO[0] < 4 \
+	|| (BASH_VERSINFO[0] == 4 && BASH_VERSINFO[1] < 4) )); then
+		echo "error: this script requires bash 4.4+" >&2
+	exit 1
+fi
+
+set -o errexit
+set -o errtrace
+set -o pipefail
+set -o nounset
+
+# Allow doing 'cmd | mapfile -t array' instead of 'mapfile -t array < <(cmd)'.
+# This helps keep execution in pipes so pipefail+errexit can catch errors.
+shopt -s lastpipe
+
+unset SKIP_CLEANUP XTRACE
+REPLACE=1
+SHORT_CIRCUIT=0
+shopt -o xtrace | grep -q 'on' && XTRACE=1
+# Avoid removing the previous $TMP_DIR until args have been fully processed.
+KEEP_TMP=1
+
+CPUS="$(getconf _NPROCESSORS_ONLN)"
+VERBOSE="-s"
+
+
+SCRIPT="$(basename "$0")"
+SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
+FIX_PATCH_LINES="$SCRIPT_DIR/fix-patch-lines"
+
+SRC="$(pwd)"
+OBJ="$(pwd)"
+
+CONFIG="$OBJ/.config"
+TMP_DIR="$OBJ/klp-tmp"
+
+ORIG_DIR="$TMP_DIR/orig"
+PATCHED_DIR="$TMP_DIR/patched"
+DIFF_DIR="$TMP_DIR/diff"
+KMOD_DIR="$TMP_DIR/kmod"
+
+STASH_DIR="$TMP_DIR/stash"
+TIMESTAMP="$TMP_DIR/timestamp"
+PATCH_TMP_DIR="$TMP_DIR/tmp"
+
+KLP_DIFF_LOG="$DIFF_DIR/diff.log"
+
+grep0() {
+	command grep "$@" || true
+}
+
+status() {
+	echo "$*"
+}
+
+warn() {
+	echo "error: $(basename "$SCRIPT"): $*" >&2
+}
+
+die() {
+	warn "$@"
+	exit 1
+}
+
+declare -a STASHED_FILES
+
+stash_file() {
+	local file="$1"
+	local rel_file="${file#"$SRC"/}"
+
+	[[ ! -e "$file" ]] && die "no file to stash: $file"
+
+	mkdir -p "$STASH_DIR/$(dirname "$rel_file")"
+	cp -f "$file" "$STASH_DIR/$rel_file"
+
+	STASHED_FILES+=("$rel_file")
+}
+
+restore_files() {
+	local file
+
+	for file in "${STASHED_FILES[@]}"; do
+		mv -f "$STASH_DIR/$file" "$SRC/$file" || warn "can't restore file: $file"
+	done
+
+	STASHED_FILES=()
+}
+
+cleanup() {
+	set +o nounset
+	revert_patches "--recount"
+	restore_files
+	[[ "$KEEP_TMP" -eq 0 ]] && rm -rf "$TMP_DIR"
+	true
+}
+
+trap_err() {
+	warn "line ${BASH_LINENO[0]}: '$BASH_COMMAND'"
+}
+
+trap cleanup  EXIT INT TERM HUP
+trap trap_err ERR
+
+__usage() {
+	cat <<EOF
+Usage: $SCRIPT [OPTIONS] PATCH_FILE(s)
+Generate a livepatch module.
+
+Options:
+   -o, --output <file.ko>	Output file [default: livepatch-<patch-name>.ko]
+       --no-replace		Disable livepatch atomic replace
+   -v, --verbose		Pass V=1 to kernel/module builds
+
+Advanced Options:
+   -S, --short-circuit=STEP	Start at build step (requires prior --keep-tmp)
+				   1|orig	Build original kernel (default)
+				   2|patched	Build patched kernel
+				   3|diff	Diff objects
+				   4|kmod	Build patch module
+   -T, --keep-tmp		Preserve tmp dir on exit
+
+EOF
+}
+
+usage() {
+	__usage >&2
+}
+
+process_args() {
+	local keep_tmp=0
+	local short
+	local long
+	local args
+
+	short="ho:vS:T"
+	long="help,output:,no-replace,verbose,short-circuit:,keep-tmp"
+
+	args=$(getopt --options "$short" --longoptions "$long" -- "$@") || {
+		echo; usage; exit
+	}
+	eval set -- "$args"
+
+	while true; do
+		case "$1" in
+			-h | --help)
+				usage
+				exit 0
+				;;
+			-o | --output)
+				[[ "$2" != *.ko ]] && die "output filename should end with .ko"
+				OUTFILE="$2"
+				NAME="$(basename "$OUTFILE")"
+				NAME="${NAME%.ko}"
+				NAME="$(module_name_string "$NAME")"
+				shift 2
+				;;
+			--no-replace)
+				REPLACE=0
+				shift
+				;;
+			-v | --verbose)
+				VERBOSE="V=1"
+				shift
+				;;
+			-S | --short-circuit)
+				[[ ! -d "$TMP_DIR" ]] && die "--short-circuit requires preserved klp-tmp dir"
+				keep_tmp=1
+				case "$2" in
+					1 | orig)	SHORT_CIRCUIT=1; ;;
+					2 | patched)	SHORT_CIRCUIT=2; ;;
+					3 | diff)	SHORT_CIRCUIT=3; ;;
+					4 | mod)	SHORT_CIRCUIT=4; ;;
+					*)		die "invalid short-circuit step '$2'" ;;
+				esac
+				shift 2
+				;;
+			-T | --keep-tmp)
+				keep_tmp=1
+				shift
+				;;
+			--)
+				shift
+				break
+				;;
+			*)
+				usage
+				exit 1
+				;;
+		esac
+	done
+
+	if [[ $# -eq 0 ]]; then
+		usage
+		exit 1
+	fi
+
+	KEEP_TMP="$keep_tmp"
+	PATCHES=("$@")
+}
+
+# temporarily disable xtrace for especially verbose code
+xtrace_save() {
+	[[ -v XTRACE ]] && set +x
+	return 0
+}
+
+xtrace_restore() {
+	[[ -v XTRACE ]] && set -x
+	return 0
+}
+
+validate_config() {
+	xtrace_save "reading .config"
+	source "$CONFIG" || die "no .config file in $(dirname "$CONFIG")"
+	xtrace_restore
+
+	[[ -v CONFIG_LIVEPATCH ]]			\
+		|| die "CONFIG_LIVEPATCH not enabled"
+
+	[[ -v CONFIG_GCC_PLUGIN_LATENT_ENTROPY ]]	\
+		&& die "kernel option 'CONFIG_GCC_PLUGIN_LATENT_ENTROPY' not supported"
+
+	[[ -v CONFIG_GCC_PLUGIN_RANDSTRUCT ]]		\
+		&& die "kernel option 'CONFIG_GCC_PLUGIN_RANDSTRUCT' not supported"
+
+	return 0
+}
+
+# Only allow alphanumerics and '_' and '-' in the module name.  Everything else
+# is replaced with '-'.  Also truncate to 55 chars so the full name + NUL
+# terminator fits in the kernel's 56-byte module name array.
+module_name_string() {
+	echo "${1//[^a-zA-Z0-9_-]/-}" | cut -c 1-55
+}
+
+# If the module name wasn't specified on the cmdline with --output, give it a
+# name based on the patch name.
+set_module_name() {
+	[[ -v NAME ]] && return 0
+
+	if [[ "${#PATCHES[@]}" -eq 1 ]]; then
+		NAME="$(basename "${PATCHES[0]}")"
+		NAME="${NAME%.*}"
+	else
+		NAME="patch"
+	fi
+
+	NAME="livepatch-$NAME"
+	NAME="$(module_name_string "$NAME")"
+
+	OUTFILE="$NAME.ko"
+}
+
+# Hardcode the value printed by the localversion script to prevent patch
+# application from appending it with '+' due to a dirty git working tree.
+set_kernelversion() {
+	local file="$SRC/scripts/setlocalversion"
+	local localversion
+
+	stash_file "$file"
+
+	localversion="$(cd "$SRC" && make --no-print-directory kernelversion)"
+	localversion="$(cd "$SRC" && KERNELVERSION="$localversion" ./scripts/setlocalversion)"
+	[[ -z "$localversion" ]] && die "setlocalversion failed"
+
+	echo "echo $localversion" > "$file"
+}
+
+get_patch_files() {
+	local patch="$1"
+
+	grep0 -E '^(--- |\+\+\+ )' "$patch"			\
+		| gawk '{print $2}'				\
+		| sed 's|^[^/]*/||'				\
+		| sort -u
+}
+
+# Make sure git re-stats the changed files
+git_refresh() {
+	local patch="$1"
+	local files=()
+
+	[[ ! -d "$SRC/.git" ]] && return
+
+	get_patch_files "$patch" | mapfile -t files
+
+	(
+		cd "$SRC"
+		git update-index -q --refresh -- "${files[@]}"
+	)
+}
+
+check_unsupported_patches() {
+	local patch
+
+	for patch in "${PATCHES[@]}"; do
+		local files=()
+
+		get_patch_files "$patch" | mapfile -t files
+
+		for file in "${files[@]}"; do
+			case "$file" in
+				lib/*|*.S)
+					die "unsupported patch to $file"
+					;;
+			esac
+		done
+	done
+}
+
+apply_patch() {
+	local patch="$1"
+	shift
+	local extra_args=("$@")
+
+	[[ ! -f "$patch" ]] && die "$patch doesn't exist"
+
+	( cd "$SRC" && git apply "${extra_args[@]}" "$patch" )
+
+	APPLIED_PATCHES+=("$patch")
+}
+
+revert_patch() {
+	local patch="$1"
+	shift
+	local extra_args=("$@")
+	local tmp=()
+
+	( cd "$SRC" && git apply --reverse "${extra_args[@]}" "$patch" )
+	git_refresh "$patch"
+
+	for p in "${APPLIED_PATCHES[@]}"; do
+		[[ "$p" == "$patch" ]] && continue
+		tmp+=("$p")
+	done
+
+	APPLIED_PATCHES=("${tmp[@]}")
+}
+
+apply_patches() {
+	local patch
+
+	for patch in "${PATCHES[@]}"; do
+		apply_patch "$patch"
+	done
+}
+
+revert_patches() {
+	local extra_args=("$@")
+	local patches=("${APPLIED_PATCHES[@]}")
+
+	for (( i=${#patches[@]}-1 ; i>=0 ; i-- )) ; do
+		revert_patch "${patches[$i]}" "${extra_args[@]}"
+	done
+
+	APPLIED_PATCHES=()
+
+	# Make sure git actually sees the patches have been reverted.
+	[[ -d "$SRC/.git" ]] && (cd "$SRC" && git update-index -q --refresh)
+}
+
+validate_patches() {
+	check_unsupported_patches
+	apply_patches
+	revert_patches
+}
+
+do_init() {
+	# We're not yet smart enough to handle anything other than in-tree
+	# builds in pwd.
+	[[ ! "$SRC" -ef "$SCRIPT_DIR/../.." ]] && die "please run from the kernel root directory"
+	[[ ! "$OBJ" -ef "$SCRIPT_DIR/../.." ]] && die "please run from the kernel root directory"
+
+	(( SHORT_CIRCUIT <= 1 )) && rm -rf "$TMP_DIR"
+	mkdir -p "$TMP_DIR"
+
+	APPLIED_PATCHES=()
+
+	[[ -x "$FIX_PATCH_LINES" ]] || die "can't find fix-patch-lines"
+
+	validate_config
+	set_module_name
+	set_kernelversion
+}
+
+# Refresh the patch hunk headers, specifically the line numbers and counts.
+refresh_patch() {
+	local patch="$1"
+	local tmpdir="$PATCH_TMP_DIR"
+	local files=()
+
+	rm -rf "$tmpdir"
+	mkdir -p "$tmpdir/a"
+	mkdir -p "$tmpdir/b"
+
+	# Find all source files affected by the patch
+	grep0 -E '^(--- |\+\+\+ )[^ /]+' "$patch"	|
+		sed -E 's/(--- |\+\+\+ )[^ /]+\///'	|
+		sort | uniq | mapfile -t files
+
+	# Copy orig source files to 'a'
+	( cd "$SRC" && echo "${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" )
+	revert_patch "$patch" --recount
+
+	# Diff 'a' and 'b' to make a clean patch
+	( cd "$tmpdir" && git diff --no-index --no-prefix a b > "$patch" ) || true
+}
+
+# Copy the patches to a temporary directory, fix their lines so as not to
+# affect the __LINE__ macro for otherwise unchanged functions further down the
+# file, and update $PATCHES to point to the fixed patches.
+fix_patches() {
+	local idx
+	local i
+
+	rm -f "$TMP_DIR"/*.patch
+
+	idx=0001
+	for i in "${!PATCHES[@]}"; do
+		local old_patch="${PATCHES[$i]}"
+		local tmp_patch="$TMP_DIR/tmp.patch"
+		local patch="${PATCHES[$i]}"
+		local new_patch
+
+		new_patch="$TMP_DIR/$idx-fixed-$(basename "$patch")"
+
+		cp -f "$old_patch" "$tmp_patch"
+		refresh_patch "$tmp_patch"
+		"$FIX_PATCH_LINES" "$tmp_patch" > "$new_patch"
+		refresh_patch "$new_patch"
+
+		PATCHES[i]="$new_patch"
+
+		rm -f "$tmp_patch"
+		idx=$(printf "%04d" $(( 10#$idx + 1 )))
+	done
+}
+
+build_kernel() {
+	local log="$TMP_DIR/build.log"
+	local objtool_args=()
+	local cmd=()
+
+	objtool_args=("--checksum")
+	[[ -v OBJTOOL_ARGS ]] && objtool_args+=("${OBJTOOL_ARGS}")
+
+	cmd=("make")
+
+	# When a patch to a kernel module references a newly created unexported
+	# symbol which lives in vmlinux or another kernel module, the patched
+	# kernel build fails with the following error:
+	#
+	#   ERROR: modpost: "klp_string" [fs/xfs/xfs.ko] undefined!
+	#
+	# The undefined symbols are working as designed in that case.  They get
+	# resolved later when the livepatch module build link pulls all the
+	# disparate objects together into the same kernel module.
+	#
+	# It would be good to have a way to tell modpost to skip checking for
+	# undefined symbols altogether.  For now, just convert the error to a
+	# warning with KBUILD_MODPOST_WARN, and grep out the warning to avoid
+	# confusing the user.
+	#
+	cmd+=("KBUILD_MODPOST_WARN=1")
+
+	cmd+=("$VERBOSE")
+	cmd+=("-j$CPUS")
+	cmd+=("KCFLAGS=-ffunction-sections -fdata-sections")
+	cmd+=("OBJTOOL_ARGS=${objtool_args[*]}")
+	cmd+=("vmlinux")
+	cmd+=("modules")
+
+	(
+		cd "$SRC"
+		"${cmd[@]}"							\
+			> >(tee -a "$log")					\
+			2> >(tee -a "$log" | grep0 -v "modpost.*undefined!" >&2)
+	)
+}
+
+find_objects() {
+	local opts=("$@")
+
+	# Find root-level vmlinux.o and non-root-level .ko files,
+	# excluding klp-tmp/ and .git/
+	find "$OBJ" \( -path "$TMP_DIR" -o -path "$OBJ/.git" -o	-regex "$OBJ/[^/][^/]*\.ko" \) -prune -o \
+		    -type f "${opts[@]}"				\
+		    \( -name "*.ko" -o -path "$OBJ/vmlinux.o" \)	\
+		    -printf '%P\n'
+}
+
+# Copy all objects (.o archives) to $ORIG_DIR
+copy_orig_objects() {
+
+	rm -rf "$ORIG_DIR"
+	mkdir -p "$ORIG_DIR"
+
+	(
+		cd "$OBJ"
+		find_objects						\
+			| sed 's/\.ko$/.o/'				\
+			| xargs cp --parents --target-directory="$ORIG_DIR"
+	)
+
+	mv -f "$TMP_DIR/build.log" "$ORIG_DIR"
+	touch "$TIMESTAMP"
+}
+
+# Copy all changed objects to $PATCHED_DIR
+copy_patched_objects() {
+	local found
+	local files=()
+	local opts=()
+
+	rm -rf "$PATCHED_DIR"
+	mkdir -p "$PATCHED_DIR"
+
+	# Note this doesn't work with some configs, thus the 'cmp' below.
+	opts=("-newer")
+	opts+=("$TIMESTAMP")
+
+	find_objects "${opts[@]}" | mapfile -t files
+
+	xtrace_save "processing all objects"
+	for _file in "${files[@]}"; do
+		local rel_file="${_file/.ko/.o}"
+		local file="$OBJ/$rel_file"
+		local orig_file="$ORIG_DIR/$rel_file"
+		local patched_file="$PATCHED_DIR/$rel_file"
+
+		[[ ! -f "$file" ]] && die "missing $(basename "$file") for $_file"
+
+		cmp -s "$orig_file" "$file" && continue
+
+		mkdir -p "$(dirname "$patched_file")"
+		cp -f "$file" "$patched_file"
+		found=1
+	done
+	xtrace_restore
+
+	[[ -n "$found" ]] || die "no changes detected"
+
+	mv -f "$TMP_DIR/build.log" "$PATCHED_DIR"
+}
+
+# Diff changed objects, writing output object to $DIFF_DIR
+diff_objects() {
+	local log="$KLP_DIFF_LOG"
+	local files=()
+
+	rm -rf "$DIFF_DIR"
+	mkdir -p "$DIFF_DIR"
+
+	find "$PATCHED_DIR" -type f -name "*.o" | mapfile -t files
+	[[ ${#files[@]} -eq 0 ]] && die "no changes detected"
+
+	# Diff all changed objects
+	for file in "${files[@]}"; do
+		local rel_file="${file#"$PATCHED_DIR"/}"
+		local orig_file="$rel_file"
+		local patched_file="$PATCHED_DIR/$rel_file"
+		local out_file="$DIFF_DIR/$rel_file"
+		local cmd=()
+
+		mkdir -p "$(dirname "$out_file")"
+
+		cmd=("$SRC/tools/objtool/objtool")
+		cmd+=("klp")
+		cmd+=("diff")
+		cmd+=("$orig_file")
+		cmd+=("$patched_file")
+		cmd+=("$out_file")
+
+		(
+			cd "$ORIG_DIR"
+			"${cmd[@]}"							\
+				> >(tee -a "$log")					\
+				2> >(tee -a "$log" >&2)					\
+				|| die "objtool klp diff failed"
+		)
+	done
+}
+
+# Build and post-process livepatch module in $KMOD_DIR
+build_patch_module() {
+	local makefile="$KMOD_DIR/Kbuild"
+	local log="$KMOD_DIR/build.log"
+	local cflags=()
+	local files=()
+	local cmd=()
+
+	rm -rf "$KMOD_DIR"
+	mkdir -p "$KMOD_DIR"
+
+	cp -f "$SRC/scripts/livepatch/init.c" "$KMOD_DIR"
+
+	echo "obj-m := $NAME.o" > "$makefile"
+	echo -n "$NAME-y := init.o" >> "$makefile"
+
+	find "$DIFF_DIR" -type f -name "*.o" | mapfile -t files
+	[[ ${#files[@]} -eq 0 ]] && die "no changes detected"
+
+	for file in "${files[@]}"; do
+		local rel_file="${file#"$DIFF_DIR"/}"
+		local kmod_file="$KMOD_DIR/$rel_file"
+		local cmd_file
+
+		mkdir -p "$(dirname "$kmod_file")"
+		cp -f "$file" "$kmod_file"
+
+		# Tell kbuild this is a prebuilt object
+		cp -f "$file" "${kmod_file}_shipped"
+
+		echo -n " $rel_file" >> "$makefile"
+
+		cmd_file="$ORIG_DIR/$(dirname "$rel_file")/.$(basename "$rel_file").cmd"
+		[[ -e "$cmd_file" ]] && cp -f "$cmd_file" "$(dirname "$kmod_file")"
+	done
+
+	echo >> "$makefile"
+
+	cflags=("-ffunction-sections")
+	cflags+=("-fdata-sections")
+	[[ $REPLACE -eq 0 ]] && cflags+=("-DKLP_NO_REPLACE")
+
+	cmd=("make")
+	cmd+=("$VERBOSE")
+	cmd+=("-j$CPUS")
+	cmd+=("--directory=.")
+	cmd+=("M=$KMOD_DIR")
+	cmd+=("KCFLAGS=${cflags[*]}")
+
+	# Build a "normal" kernel module with init.c and the diffed objects
+	(
+		cd "$SRC"
+		"${cmd[@]}"							\
+			>  >(tee -a "$log")					\
+			2> >(tee -a "$log" >&2)
+	)
+
+	# Save off the intermediate binary for debugging
+	cp -f "$KMOD_DIR/$NAME.ko" "$KMOD_DIR/$NAME.ko.orig"
+
+	# Fix (and work around) linker wreckage for klp syms / relocs
+	"$SRC/tools/objtool/objtool" klp post-link "$KMOD_DIR/$NAME.ko" || die "objtool klp post-link failed"
+
+	cp -f "$KMOD_DIR/$NAME.ko" "$OUTFILE"
+}
+
+
+################################################################################
+
+process_args "$@"
+do_init
+
+if (( SHORT_CIRCUIT <= 1 )); then
+	status "Building original kernel"
+	validate_patches
+	build_kernel
+	status "Copying original object files"
+	copy_orig_objects
+fi
+
+if (( SHORT_CIRCUIT <= 2 )); then
+	status "Fixing patches"
+	fix_patches
+	apply_patches
+	status "Building patched kernel"
+	build_kernel
+	revert_patches
+	status "Copying patched object files"
+	copy_patched_objects
+fi
+
+if (( SHORT_CIRCUIT <= 3 )); then
+	status "Diffing objects"
+	diff_objects
+fi
+
+if (( SHORT_CIRCUIT <= 4 )); then
+	status "Building patch module: $OUTFILE"
+	build_patch_module
+fi
+
+status "SUCCESS"
-- 
2.49.0

On Fri, May 9, 2025 at 1:30 PM Josh Poimboeuf <jpoimboe@kernel.org> wrote:
>
> +
> +# Make sure git re-stats the changed files
> +git_refresh() {
> +       local patch="$1"
> +       local files=()
> +
> +       [[ ! -d "$SRC/.git" ]] && return

As a user of git worktrees, my $SRC/.git is a file containing a key:
value pair "gitdir: <path>", causing this script to fail on a [[ ! -d
"$SRC/.git" ]] check. Can this be handled, perhaps with a check if
.git is a file?

It seems like the check is just to confirm the $SRC directory is still
a git tree, in which case maybe adding a -f check would fix this:

[[ ! -d "$SRC/.git" ]] && [[ ! -f "$SRC/.git" ]] && return

Or if the actual git directory is needed for something, maybe it can
be located ahead of time:

GITDIR="$SRC/.git"
[[ -f $GITDIR ]] && GITDIR=$(sed -n
's/^gitdir[[:space:]]*:[[:space:]]*//p' $GITDIR)

Thanks,
Dylan

On Wed, Jun 18, 2025 at 05:38:07PM -0500, Dylan Hatch wrote:
> On Fri, May 9, 2025 at 1:30 PM Josh Poimboeuf <jpoimboe@kernel.org> wrote:
> >
> > +
> > +# Make sure git re-stats the changed files
> > +git_refresh() {
> > +       local patch="$1"
> > +       local files=()
> > +
> > +       [[ ! -d "$SRC/.git" ]] && return
> 
> As a user of git worktrees, my $SRC/.git is a file containing a key:
> value pair "gitdir: <path>", causing this script to fail on a [[ ! -d
> "$SRC/.git" ]] check. Can this be handled, perhaps with a check if
> .git is a file?
> 
> It seems like the check is just to confirm the $SRC directory is still
> a git tree, in which case maybe adding a -f check would fix this:
> 
> [[ ! -d "$SRC/.git" ]] && [[ ! -f "$SRC/.git" ]] && return
> 
> Or if the actual git directory is needed for something, maybe it can
> be located ahead of time:
> 
> GITDIR="$SRC/.git"
> [[ -f $GITDIR ]] && GITDIR=$(sed -n
> 's/^gitdir[[:space:]]*:[[:space:]]*//p' $GITDIR)

I believe the subsequent "git update-index" operation should work on git
worktrees as well, so I changed that to use '-e':

	[[ ! -e "$SRC/.git" ]] && return

-- 
Josh

On 5/9/25 4:17 PM, Josh Poimboeuf wrote:
> +revert_patches() {
> +	local extra_args=("$@")
> +	local patches=("${APPLIED_PATCHES[@]}")
> +
> +	for (( i=${#patches[@]}-1 ; i>=0 ; i-- )) ; do
> +		revert_patch "${patches[$i]}" "${extra_args[@]}"
> +	done
> +
> +	APPLIED_PATCHES=()
> +
> +	# Make sure git actually sees the patches have been reverted.
> +	[[ -d "$SRC/.git" ]] && (cd "$SRC" && git update-index -q --refresh)
> +}

< warning: testing entropy field fully engaged :D >

Minor usability nit: I had run `klp-build --help` while inside a VM that
didn't seem to have r/w access to .git/.  Since the cleanup code is
called unconditionally, it gave me a strange error when running this
`git update-index` when I never supplied any patches to operate on. I
just wanted to see the usage.

Could this git update-index be made conditional?

  if (( ${#APPLIED_PATCHES[@]} > 0 )); then
      ([[ -d "$SRC/.git" ]] && cd "$SRC" && git update-index -q --refresh)
      APPLIED_PATCHES=()
  fi

Another way to find yourself in this function is to move .git/ out of
the way.  In that case, since it's the last line of revert_patches(), I
think the failure of [[ -d "$SRC/.git" ]] causes the script to
immediately exit:

  - for foo.patch, at the validate_patches() -> revert_patches() call
  - for --help, at the cleanup() -> revert_patches() call

So if you don't like the conditional change above, should
revert_patches() end with `true` to eat the [[ -d "$SRC/.git" ]] status?
 Or does that interfere with other calls to that function throughout the
script?

FWIW, with either adjustment, the script seems happy to operate on a
plain ol' kernel source tree without git.

-- 
Joe

On Wed, Jun 11, 2025 at 05:44:23PM -0400, Joe Lawrence wrote:
> On 5/9/25 4:17 PM, Josh Poimboeuf wrote:
> > +revert_patches() {
> > +	local extra_args=("$@")
> > +	local patches=("${APPLIED_PATCHES[@]}")
> > +
> > +	for (( i=${#patches[@]}-1 ; i>=0 ; i-- )) ; do
> > +		revert_patch "${patches[$i]}" "${extra_args[@]}"
> > +	done
> > +
> > +	APPLIED_PATCHES=()
> > +
> > +	# Make sure git actually sees the patches have been reverted.
> > +	[[ -d "$SRC/.git" ]] && (cd "$SRC" && git update-index -q --refresh)
> > +}
> 
> < warning: testing entropy field fully engaged :D >
> 
> Minor usability nit: I had run `klp-build --help` while inside a VM that
> didn't seem to have r/w access to .git/.  Since the cleanup code is
> called unconditionally, it gave me a strange error when running this
> `git update-index` when I never supplied any patches to operate on. I
> just wanted to see the usage.
> 
> Could this git update-index be made conditional?
> 
>   if (( ${#APPLIED_PATCHES[@]} > 0 )); then
>       ([[ -d "$SRC/.git" ]] && cd "$SRC" && git update-index -q --refresh)
>       APPLIED_PATCHES=()
>   fi
> 
> Another way to find yourself in this function is to move .git/ out of
> the way.  In that case, since it's the last line of revert_patches(), I
> think the failure of [[ -d "$SRC/.git" ]] causes the script to
> immediately exit:
> 
>   - for foo.patch, at the validate_patches() -> revert_patches() call
>   - for --help, at the cleanup() -> revert_patches() call
> 
> So if you don't like the conditional change above, should
> revert_patches() end with `true` to eat the [[ -d "$SRC/.git" ]] status?
>  Or does that interfere with other calls to that function throughout the
> script?
> 
> FWIW, with either adjustment, the script seems happy to operate on a
> plain ol' kernel source tree without git.

Hm, revert_patch() already has a call to git_refresh(), so there doesn't
appear to be any point to that extra refresh in revert_patches().

Does this fix?

diff --git a/scripts/livepatch/klp-build b/scripts/livepatch/klp-build
index 7ec07c4079f7..e6dbac89ab03 100755
--- a/scripts/livepatch/klp-build
+++ b/scripts/livepatch/klp-build
@@ -388,9 +388,6 @@ revert_patches() {
 	done
 
 	APPLIED_PATCHES=()
-
-	# Make sure git actually sees the patches have been reverted.
-	[[ -d "$SRC/.git" ]] && (cd "$SRC" && git update-index -q --refresh)
 }
 
 validate_patches() {

On 6/11/25 7:12 PM, Josh Poimboeuf wrote:
> 
> Hm, revert_patch() already has a call to git_refresh(), so there doesn't
> appear to be any point to that extra refresh in revert_patches().
> 
> Does this fix?
> 
> diff --git a/scripts/livepatch/klp-build b/scripts/livepatch/klp-build
> index 7ec07c4079f7..e6dbac89ab03 100755
> --- a/scripts/livepatch/klp-build
> +++ b/scripts/livepatch/klp-build
> @@ -388,9 +388,6 @@ revert_patches() {
>  	done
>  
>  	APPLIED_PATCHES=()
> -
> -	# Make sure git actually sees the patches have been reverted.
> -	[[ -d "$SRC/.git" ]] && (cd "$SRC" && git update-index -q --refresh)
>  }
>  
>  validate_patches() {
> 

Ah yes, even better.

-- 
Joe

On 5/9/25 4:17 PM, Josh Poimboeuf wrote:
> Add a klp-build script which automates the generation of a livepatch
> module from a source .patch file by performing the following steps:
> 
>   - Builds an original kernel with -function-sections and
>     -fdata-sections, plus objtool function checksumming.
> 
>   - Applies the .patch file and rebuilds the kernel using the same
>     options.
> 
>   - Runs 'objtool klp diff' to detect changed functions and generate
>     intermediate binary diff objects.
> 
>   - Builds a kernel module which links the diff objects with some
>     livepatch module init code (scripts/livepatch/init.c).
> 
>   - Finalizes the livepatch module (aka work around linker wreckage)
>     using 'objtool klp post-link'.
> 
> Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
> ---
>  scripts/livepatch/klp-build | 697 ++++++++++++++++++++++++++++++++++++
> ...
> +get_patch_files() {
> +	local patch="$1"
> +
> +	grep0 -E '^(--- |\+\+\+ )' "$patch"			\
> +		| gawk '{print $2}'				\

If we split the rest of this line on the tab character and print the
first part of $2:

  gawk '{ split($2, a, "\t"); print a[1] }'

then it can additionally handle patches generated by `diff -Nupr` with a
timepstamp ("--- <filepath>\t<timestamp>").

> +# Refresh the patch hunk headers, specifically the line numbers and counts.
> +refresh_patch() {
> +	local patch="$1"
> +	local tmpdir="$PATCH_TMP_DIR"
> +	local files=()
> +
> +	rm -rf "$tmpdir"
> +	mkdir -p "$tmpdir/a"
> +	mkdir -p "$tmpdir/b"
> +
> +	# Find all source files affected by the patch
> +	grep0 -E '^(--- |\+\+\+ )[^ /]+' "$patch"	|
> +		sed -E 's/(--- |\+\+\+ )[^ /]+\///'	|
> +		sort | uniq | mapfile -t files
> +

Should just call `get_patch_files() here?

-- 
Joe

On Wed, Jun 11, 2025 at 02:44:35PM -0400, Joe Lawrence wrote:
> > +get_patch_files() {
> > +	local patch="$1"
> > +
> > +	grep0 -E '^(--- |\+\+\+ )' "$patch"			\
> > +		| gawk '{print $2}'				\
> 
> If we split the rest of this line on the tab character and print the
> first part of $2:
> 
>   gawk '{ split($2, a, "\t"); print a[1] }'
> 
> then it can additionally handle patches generated by `diff -Nupr` with a
> timepstamp ("--- <filepath>\t<timestamp>").

Hm?  The default gawk behavior is to treat both tabs and groups of
spaces as field separators:

  https://www.gnu.org/software/gawk/manual/html_node/Default-Field-Splitting.html

And it works for me:

  $ diff -Nupr /tmp/meminfo.c fs/proc/meminfo.c > /tmp/a.patch
  $ grep -E '^(--- |\+\+\+ )' /tmp/a.patch | gawk '{print $2}'
  /tmp/meminfo.c
  fs/proc/meminfo.c

Or did I miss something?

> > +# Refresh the patch hunk headers, specifically the line numbers and counts.
> > +refresh_patch() {
> > +	local patch="$1"
> > +	local tmpdir="$PATCH_TMP_DIR"
> > +	local files=()
> > +
> > +	rm -rf "$tmpdir"
> > +	mkdir -p "$tmpdir/a"
> > +	mkdir -p "$tmpdir/b"
> > +
> > +	# Find all source files affected by the patch
> > +	grep0 -E '^(--- |\+\+\+ )[^ /]+' "$patch"	|
> > +		sed -E 's/(--- |\+\+\+ )[^ /]+\///'	|
> > +		sort | uniq | mapfile -t files
> > +
> 
> Should just call `get_patch_files() here?

Indeed.

-- 
Josh

On 6/11/25 3:08 PM, Josh Poimboeuf wrote:
> On Wed, Jun 11, 2025 at 02:44:35PM -0400, Joe Lawrence wrote:
>>> +get_patch_files() {
>>> +	local patch="$1"
>>> +
>>> +	grep0 -E '^(--- |\+\+\+ )' "$patch"			\
>>> +		| gawk '{print $2}'				\
>> If we split the rest of this line on the tab character and print the
>> first part of $2:
>>
>>   gawk '{ split($2, a, "\t"); print a[1] }'
>>
>> then it can additionally handle patches generated by `diff -Nupr` with a
>> timepstamp ("--- <filepath>\t<timestamp>").
> Hm?  The default gawk behavior is to treat both tabs and groups of
> spaces as field separators:
> 
>   https://www.gnu.org/software/gawk/manual/html_node/Default-Field-
> Splitting.html
> 
> And it works for me:
> 
>   $ diff -Nupr /tmp/meminfo.c fs/proc/meminfo.c > /tmp/a.patch
>   $ grep -E '^(--- |\+\+\+ )' /tmp/a.patch | gawk '{print $2}'
>   /tmp/meminfo.c
>   fs/proc/meminfo.c
> 
> Or did I miss something?

Ah hah, I fixed up the code in refresh_patch() with that double gawk,
and then noticed it could have just called get_patch_files() instead.

So yeah, you're right, the simple gawk '{print $2}' handles both cases
already :)

-- 
Joe

On Fri, May 09, 2025 at 01:17:23PM -0700, Josh Poimboeuf wrote:
> +revert_patch() {
> +	local patch="$1"
> +	shift
> +	local extra_args=("$@")
> +	local tmp=()
> +
> +	( cd "$SRC" && git apply --reverse "${extra_args[@]}" "$patch" )
> +	git_refresh "$patch"
> +
> +	for p in "${APPLIED_PATCHES[@]}"; do
> +		[[ "$p" == "$patch" ]] && continue
> +		tmp+=("$p")
> +	done
> +
> +	APPLIED_PATCHES=("${tmp[@]}")
> +}

You may consider a slight adjustment to revert_patch() to handle git
format-patch generated .patches?  The reversal trips up on the git
version trailer:

  warning: recount: unexpected line: 2.47.1

diff --git a/scripts/livepatch/klp-build b/scripts/livepatch/klp-build
index f7d88726ed4f..8cf0cd8f5fd3 100755
--- a/scripts/livepatch/klp-build
+++ b/scripts/livepatch/klp-build
@@ -345,7 +345,8 @@ revert_patch() {
 	local extra_args=("$@")
 	local tmp=()

-	( cd "$SRC" && git apply --reverse "${extra_args[@]}" "$patch" )
+	( cd "$SRC" && \
+	  git apply --reverse "${extra_args[@]}" <(sed -n '/^-- /q;p' "$patch") )
 	git_refresh "$patch"

 	for p in "${APPLIED_PATCHES[@]}"; do

On Mon, Jun 09, 2025 at 10:34:43PM -0400, Joe Lawrence wrote:
> On Fri, May 09, 2025 at 01:17:23PM -0700, Josh Poimboeuf wrote:
> > +revert_patch() {
> > +	local patch="$1"
> > +	shift
> > +	local extra_args=("$@")
> > +	local tmp=()
> > +
> > +	( cd "$SRC" && git apply --reverse "${extra_args[@]}" "$patch" )
> > +	git_refresh "$patch"
> > +
> > +	for p in "${APPLIED_PATCHES[@]}"; do
> > +		[[ "$p" == "$patch" ]] && continue
> > +		tmp+=("$p")
> > +	done
> > +
> > +	APPLIED_PATCHES=("${tmp[@]}")
> > +}
> 
> You may consider a slight adjustment to revert_patch() to handle git
> format-patch generated .patches?  The reversal trips up on the git
> version trailer:
> 
>   warning: recount: unexpected line: 2.47.1

Thanks.  Looks like the normal apply with --recount also trips it up.  I
have the below:

diff --git a/scripts/livepatch/klp-build b/scripts/livepatch/klp-build
index f689a4d143c6..1ff5e66f4c53 100755
--- a/scripts/livepatch/klp-build
+++ b/scripts/livepatch/klp-build
@@ -337,7 +337,14 @@ apply_patch() {
 
 	[[ ! -f "$patch" ]] && die "$patch doesn't exist"
 
-	( cd "$SRC" && git apply "${extra_args[@]}" "$patch" )
+	(
+		cd "$SRC"
+
+		# The sed removes the version signature from 'git format-patch',
+		# otherwise 'git apply --recount' warns.
+		sed -n '/^-- /q;p' "$patch" |
+			git apply "${extra_args[@]}"
+	)
 
 	APPLIED_PATCHES+=("$patch")
 }
@@ -348,7 +355,12 @@ revert_patch() {
 	local extra_args=("$@")
 	local tmp=()
 
-	( cd "$SRC" && git apply --reverse "${extra_args[@]}" "$patch" )
+	(
+		cd "$SRC"
+
+		sed -n '/^-- /q;p' "$patch" |
+			git apply --reverse "${extra_args[@]}"
+	)
 	git_refresh "$patch"
 
 	for p in "${APPLIED_PATCHES[@]}"; do

On Fri, May 09, 2025 at 01:17:23PM -0700, Josh Poimboeuf wrote:
> +# Build and post-process livepatch module in $KMOD_DIR
> +build_patch_module() {
> +	local makefile="$KMOD_DIR/Kbuild"
> +	local log="$KMOD_DIR/build.log"
> +	local cflags=()
> +	local files=()
> +	local cmd=()
> +
> +	rm -rf "$KMOD_DIR"
> +	mkdir -p "$KMOD_DIR"
> +
> +	cp -f "$SRC/scripts/livepatch/init.c" "$KMOD_DIR"
> +
> +	echo "obj-m := $NAME.o" > "$makefile"
> +	echo -n "$NAME-y := init.o" >> "$makefile"
> +
> +	find "$DIFF_DIR" -type f -name "*.o" | mapfile -t files
> +	[[ ${#files[@]} -eq 0 ]] && die "no changes detected"
> +
> +	for file in "${files[@]}"; do
> +		local rel_file="${file#"$DIFF_DIR"/}"
> +		local kmod_file="$KMOD_DIR/$rel_file"
> +		local cmd_file
> +
> +		mkdir -p "$(dirname "$kmod_file")"
> +		cp -f "$file" "$kmod_file"
> +
> +		# Tell kbuild this is a prebuilt object
> +		cp -f "$file" "${kmod_file}_shipped"
> +
> +		echo -n " $rel_file" >> "$makefile"
> +
> +		cmd_file="$ORIG_DIR/$(dirname "$rel_file")/.$(basename "$rel_file").cmd"
> +		[[ -e "$cmd_file" ]] && cp -f "$cmd_file" "$(dirname "$kmod_file")"
> +	done
> +
> +	echo >> "$makefile"
> +
> +	cflags=("-ffunction-sections")
> +	cflags+=("-fdata-sections")
> +	[[ $REPLACE -eq 0 ]] && cflags+=("-DKLP_NO_REPLACE")
> +
> +	cmd=("make")
> +	cmd+=("$VERBOSE")
> +	cmd+=("-j$CPUS")
> +	cmd+=("--directory=.")
> +	cmd+=("M=$KMOD_DIR")
> +	cmd+=("KCFLAGS=${cflags[*]}")
> +
> +	# Build a "normal" kernel module with init.c and the diffed objects
> +	(
> +		cd "$SRC"
> +		"${cmd[@]}"							\
> +			>  >(tee -a "$log")					\
> +			2> >(tee -a "$log" >&2)
> +	)
> +
> +	# Save off the intermediate binary for debugging
> +	cp -f "$KMOD_DIR/$NAME.ko" "$KMOD_DIR/$NAME.ko.orig"
> +
> +	# Fix (and work around) linker wreckage for klp syms / relocs
> +	"$SRC/tools/objtool/objtool" klp post-link "$KMOD_DIR/$NAME.ko" || die "objtool klp post-link failed"
> +
> +	cp -f "$KMOD_DIR/$NAME.ko" "$OUTFILE"
> +}

Hi Josh,

Another small bug feature? report: module symbol namespaces.

If you touch sound/soc/sof/intel/, klp-build will error out with:

  Building patch module: livepatch-unCVE-2024-58012.ko
  ERROR: modpost: module livepatch-unCVE-2024-58012 uses symbol hda_dai_config from namespace SND_SOC_SOF_INTEL_HDA_COMMON, but does not import it.
  ERROR: modpost: module livepatch-unCVE-2024-58012 uses symbol hdac_bus_eml_sdw_map_stream_ch from namespace SND_SOC_SOF_HDA_MLINK, but does not import it.
  make[2]: *** [scripts/Makefile.modpost:145: /home/jolawren/src/centos-stream-10/klp-tmp/kmod/Module.symvers] Error 1
  make[1]: *** [/home/jolawren/src/centos-stream-10/Makefile:1936: modpost] Error 2
  make: *** [Makefile:236: __sub-make] Error 2

since the diff objects do not necessarily carry forward the namespace
import.

There's several options to how to handle it (cross-reference with
Modules.symvers, copy out the .modinfo sections, include the section in
the diff .o, etc.) ... my late afternoon hack just snarfed it from the
original objects with a modinfo hack.  Anyway, you get the idea.

-- Joe

-->8-- -->8-- -->8-- -->8-- -->8-- -->8-- -->8-- -->8-- -->8-- -->8--

@@ -687,7 +700,9 @@ build_patch_module() {
 	cp -f "$SRC/scripts/livepatch/init.c" "$KMOD_DIR"
 
 	echo "obj-m := $NAME.o" > "$makefile"
-	echo -n "$NAME-y := init.o" >> "$makefile"
+
+	echo "#include <linux/module.h>" >> "$KMOD_DIR/namespaces.c"
+	echo -n "$NAME-y := init.o namespaces.o" >> "$makefile"
 
 	find "$DIFF_DIR" -type f -name "*.o" | mapfile -t files
 	[[ ${#files[@]} -eq 0 ]] && die "no changes detected"
@@ -697,6 +712,13 @@ build_patch_module() {
 		local kmod_file="$KMOD_DIR/$rel_file"
 		local cmd_file
 
+		# Symbol namespace hack
+		echo ln -s -f "$file" ns-temp.ko
+		ln -s -f "$ORIG_DIR/$rel_file" ns-temp.ko
+		for ns in $(modinfo ns-temp.ko -F import_ns); do
+			echo "MODULE_IMPORT_NS(\"$ns\");" >> "$KMOD_DIR/namespaces.c"
+		done
+
 		mkdir -p "$(dirname "$kmod_file")"
 		cp -f "$file" "$kmod_file"
 
--
Joe

On Mon, Jun 09, 2025 at 05:20:53PM -0400, Joe Lawrence wrote:
> If you touch sound/soc/sof/intel/, klp-build will error out with:
> 
>   Building patch module: livepatch-unCVE-2024-58012.ko
>   ERROR: modpost: module livepatch-unCVE-2024-58012 uses symbol hda_dai_config from namespace SND_SOC_SOF_INTEL_HDA_COMMON, but does not import it.
>   ERROR: modpost: module livepatch-unCVE-2024-58012 uses symbol hdac_bus_eml_sdw_map_stream_ch from namespace SND_SOC_SOF_HDA_MLINK, but does not import it.
>   make[2]: *** [scripts/Makefile.modpost:145: /home/jolawren/src/centos-stream-10/klp-tmp/kmod/Module.symvers] Error 1
>   make[1]: *** [/home/jolawren/src/centos-stream-10/Makefile:1936: modpost] Error 2
>   make: *** [Makefile:236: __sub-make] Error 2
> 
> since the diff objects do not necessarily carry forward the namespace
> import.

Nice, thanks for finding that.  I completely forgot about export
namespaces.

Can you send me the patch for testing?  Is this the default centos10
config?

-- 
Josh

On Mon, Jun 09, 2025 at 04:59:37PM -0700, Josh Poimboeuf wrote:
> On Mon, Jun 09, 2025 at 05:20:53PM -0400, Joe Lawrence wrote:
> > If you touch sound/soc/sof/intel/, klp-build will error out with:
> > 
> >   Building patch module: livepatch-unCVE-2024-58012.ko
> >   ERROR: modpost: module livepatch-unCVE-2024-58012 uses symbol hda_dai_config from namespace SND_SOC_SOF_INTEL_HDA_COMMON, but does not import it.
> >   ERROR: modpost: module livepatch-unCVE-2024-58012 uses symbol hdac_bus_eml_sdw_map_stream_ch from namespace SND_SOC_SOF_HDA_MLINK, but does not import it.
> >   make[2]: *** [scripts/Makefile.modpost:145: /home/jolawren/src/centos-stream-10/klp-tmp/kmod/Module.symvers] Error 1
> >   make[1]: *** [/home/jolawren/src/centos-stream-10/Makefile:1936: modpost] Error 2
> >   make: *** [Makefile:236: __sub-make] Error 2
> > 
> > since the diff objects do not necessarily carry forward the namespace
> > import.
> 
> Nice, thanks for finding that.  I completely forgot about export
> namespaces.
> 
> Can you send me the patch for testing?  Is this the default centos10
> config?
> 

Yeah, cs-10 sets CONFIG_NAMESPACES=y.

The hack I posted earlier abused modinfo to get the namespaces.  You
could just dump the import_ns= strings in the .modinfo section with
readelf like (lightly tested):

diff --git a/scripts/livepatch/klp-build b/scripts/livepatch/klp-build
index f7d88726ed4f..671d1d07fd08 100755
--- a/scripts/livepatch/klp-build
+++ b/scripts/livepatch/klp-build
@@ -687,7 +687,9 @@ build_patch_module() {
 	cp -f "$SRC/scripts/livepatch/init.c" "$KMOD_DIR"
 
 	echo "obj-m := $NAME.o" > "$makefile"
-	echo -n "$NAME-y := init.o" >> "$makefile"
+	echo -n "$NAME-y := init.o namespaces.o" >> "$makefile"
+
+	echo "#include <linux/module.h>" >> "$KMOD_DIR/namespaces.c"
 
 	find "$DIFF_DIR" -type f -name "*.o" | mapfile -t files
 	[[ ${#files[@]} -eq 0 ]] && die "no changes detected"
@@ -695,8 +697,16 @@ build_patch_module() {
 	for file in "${files[@]}"; do
 		local rel_file="${file#"$DIFF_DIR"/}"
 		local kmod_file="$KMOD_DIR/$rel_file"
+		local namespaces=()
 		local cmd_file
 
+		# Copy symbol namespace
+		readelf -p .modinfo "$ORIG_DIR/$rel_file" | \
+			gawk -F= '/\<import_ns=/ {print $2}' | mapfile -t namespaces
+		for ns in "${namespaces[@]}"; do
+			echo "MODULE_IMPORT_NS(\"$ns\");" >> "$KMOD_DIR/namespaces.c"
+		done
+
 		mkdir -p "$(dirname "$kmod_file")"
 		cp -f "$file" "$kmod_file"

On Mon, Jun 09, 2025 at 10:05:53PM -0400, Joe Lawrence wrote:
> On Mon, Jun 09, 2025 at 04:59:37PM -0700, Josh Poimboeuf wrote:
> > On Mon, Jun 09, 2025 at 05:20:53PM -0400, Joe Lawrence wrote:
> > > If you touch sound/soc/sof/intel/, klp-build will error out with:
> > > 
> > >   Building patch module: livepatch-unCVE-2024-58012.ko
> > >   ERROR: modpost: module livepatch-unCVE-2024-58012 uses symbol hda_dai_config from namespace SND_SOC_SOF_INTEL_HDA_COMMON, but does not import it.
> > >   ERROR: modpost: module livepatch-unCVE-2024-58012 uses symbol hdac_bus_eml_sdw_map_stream_ch from namespace SND_SOC_SOF_HDA_MLINK, but does not import it.
> > >   make[2]: *** [scripts/Makefile.modpost:145: /home/jolawren/src/centos-stream-10/klp-tmp/kmod/Module.symvers] Error 1
> > >   make[1]: *** [/home/jolawren/src/centos-stream-10/Makefile:1936: modpost] Error 2
> > >   make: *** [Makefile:236: __sub-make] Error 2
> > > 
> > > since the diff objects do not necessarily carry forward the namespace
> > > import.
> > 
> > Nice, thanks for finding that.  I completely forgot about export
> > namespaces.
> > 
> > Can you send me the patch for testing?  Is this the default centos10
> > config?
> > 
> 
> Yeah, cs-10 sets CONFIG_NAMESPACES=y.
> 
> The hack I posted earlier abused modinfo to get the namespaces.  You
> could just dump the import_ns= strings in the .modinfo section with
> readelf like (lightly tested):

Sorry, I wasn't clear, I meant the original .patch for recreating the
issue.  But that's ok, I think the below fix should work.

This is basically the same approach as yours, but in klp-diff.  It copies
from the patched object instead of the orig object in case the patch
needs to add an IMPORT_NS().

I also experimented with reading the namespaces from Module.symvers, and
then adding them on demand for exported symbols in clone_symbol().  But
this is simpler.

diff --git a/tools/objtool/klp-diff.c b/tools/objtool/klp-diff.c
index a1c72824f442..3139f1ebacce 100644
--- a/tools/objtool/klp-diff.c
+++ b/tools/objtool/klp-diff.c
@@ -1490,6 +1490,51 @@ static int create_klp_sections(struct elfs *e)
 	return 0;
 }
 
+/*
+ * Copy all .modinfo import_ns= tags to ensure all namespaced exported symbols
+ * can be accessed via normal relocs.
+ */
+static int copy_import_ns(struct elfs *e)
+{
+	struct section *patched_sec, *out_sec = NULL;
+	char *import_ns, *data_end;
+
+	patched_sec = find_section_by_name(e->patched, ".modinfo");
+	if (!patched_sec)
+		return 0;
+
+	import_ns = patched_sec->data->d_buf;
+	if (!import_ns)
+		return 0;
+
+	for (data_end = import_ns + sec_size(patched_sec);
+	     import_ns < data_end;
+	     import_ns += strlen(import_ns) + 1) {
+
+		import_ns = memmem(import_ns, data_end - import_ns, "import_ns=", 10);
+		if (!import_ns)
+			return 0;
+
+		if (!out_sec) {
+			out_sec = find_section_by_name(e->out, ".modinfo");
+			if (!out_sec) {
+				out_sec = elf_create_section(e->out, ".modinfo", 0,
+							     patched_sec->sh.sh_entsize,
+							     patched_sec->sh.sh_type,
+							     patched_sec->sh.sh_addralign,
+							     patched_sec->sh.sh_flags);
+				if (!out_sec)
+					return -1;
+			}
+		}
+
+		if (!elf_add_data(e->out, out_sec, import_ns, strlen(import_ns) + 1))
+			return -1;
+	}
+
+	return 0;
+}
+
 int cmd_klp_diff(int argc, const char **argv)
 {
 	struct elfs e = {0};
@@ -1539,6 +1584,9 @@ int cmd_klp_diff(int argc, const char **argv)
 	if (create_klp_sections(&e))
 		return -1;
 
+	if (copy_import_ns(&e))
+		return -1;
+
 	if  (elf_write(e.out))
 		return -1;

On Mon, Jun 09, 2025 at 10:05:53PM -0400, Joe Lawrence wrote:
> +		# Copy symbol namespace
> +		readelf -p .modinfo "$ORIG_DIR/$rel_file" | \
> +			gawk -F= '/\<import_ns=/ {print $2}' | mapfile -t namespaces

Errr, that is $PATCHED_DIR/$rel_file if we want to pick up the updated
list of namespaces in case the .patch had amended it.

--
Joe

On 5/9/25 4:17 PM, Josh Poimboeuf wrote:
> +copy_patched_objects() {
> +	local found
> +	local files=()
> +	local opts=()
> +
> +	rm -rf "$PATCHED_DIR"
> +	mkdir -p "$PATCHED_DIR"
> +
> +	# Note this doesn't work with some configs, thus the 'cmp' below.
> +	opts=("-newer")
> +	opts+=("$TIMESTAMP")
> +
> +	find_objects "${opts[@]}" | mapfile -t files
> +
> +	xtrace_save "processing all objects"
> +	for _file in "${files[@]}"; do
> +		local rel_file="${_file/.ko/.o}"
> +		local file="$OBJ/$rel_file"
> +		local orig_file="$ORIG_DIR/$rel_file"
> +		local patched_file="$PATCHED_DIR/$rel_file"
> +
> +		[[ ! -f "$file" ]] && die "missing $(basename "$file") for $_file"
> +
> +		cmp -s "$orig_file" "$file" && continue
> +
> +		mkdir -p "$(dirname "$patched_file")"
> +		cp -f "$file" "$patched_file"
> +		found=1
> +	done
> +	xtrace_restore
> +
> +	[[ -n "$found" ]] || die "no changes detected"
> +

Minor nit here, I gave it a patch for files that didn't compile and
because because files() was presumably empty:

  ./scripts/livepatch/klp-build: line 564: found: unbound variable

since found was only declared local, but never set inside the loop.

-- 
Joe

On Fri, Jun 06, 2025 at 04:58:35PM -0400, Joe Lawrence wrote:
> On 5/9/25 4:17 PM, Josh Poimboeuf wrote:
> > +copy_patched_objects() {
> > +	local found
> > +	local files=()
> > +	local opts=()
> > +
> > +	rm -rf "$PATCHED_DIR"
> > +	mkdir -p "$PATCHED_DIR"
> > +
> > +	# Note this doesn't work with some configs, thus the 'cmp' below.
> > +	opts=("-newer")
> > +	opts+=("$TIMESTAMP")
> > +
> > +	find_objects "${opts[@]}" | mapfile -t files
> > +
> > +	xtrace_save "processing all objects"
> > +	for _file in "${files[@]}"; do
> > +		local rel_file="${_file/.ko/.o}"
> > +		local file="$OBJ/$rel_file"
> > +		local orig_file="$ORIG_DIR/$rel_file"
> > +		local patched_file="$PATCHED_DIR/$rel_file"
> > +
> > +		[[ ! -f "$file" ]] && die "missing $(basename "$file") for $_file"
> > +
> > +		cmp -s "$orig_file" "$file" && continue
> > +
> > +		mkdir -p "$(dirname "$patched_file")"
> > +		cp -f "$file" "$patched_file"
> > +		found=1
> > +	done
> > +	xtrace_restore
> > +
> > +	[[ -n "$found" ]] || die "no changes detected"
> > +
> 
> Minor nit here, I gave it a patch for files that didn't compile and
> because because files() was presumably empty:
> 
>   ./scripts/livepatch/klp-build: line 564: found: unbound variable
> 
> since found was only declared local, but never set inside the loop.

Thanks, I'm adding the following:

diff --git a/scripts/livepatch/klp-build b/scripts/livepatch/klp-build
index 9927d06dfdab..f689a4d143c6 100755
--- a/scripts/livepatch/klp-build
+++ b/scripts/livepatch/klp-build
@@ -563,7 +563,7 @@ copy_orig_objects() {
 copy_patched_objects() {
 	local files=()
 	local opts=()
-	local found
+	local found=0
 
 	rm -rf "$PATCHED_DIR"
 	mkdir -p "$PATCHED_DIR"
@@ -592,7 +592,7 @@ copy_patched_objects() {
 	done
 	xtrace_restore
 
-	[[ -n "$found" ]] || die "no changes detected"
+	(( found == 0 )) && die "no changes detected"
 
 	mv -f "$TMP_DIR/build.log" "$PATCHED_DIR"
 }

> +# Build and post-process livepatch module in $KMOD_DIR
> +build_patch_module() {
> +	local makefile="$KMOD_DIR/Kbuild"
> +	local log="$KMOD_DIR/build.log"
> +	local cflags=()
> +	local files=()
> +	local cmd=()
> +
> +	rm -rf "$KMOD_DIR"
> +	mkdir -p "$KMOD_DIR"
> +
> +	cp -f "$SRC/scripts/livepatch/init.c" "$KMOD_DIR"
> +
> +	echo "obj-m := $NAME.o" > "$makefile"
> +	echo -n "$NAME-y := init.o" >> "$makefile"
> +
> +	find "$DIFF_DIR" -type f -name "*.o" | mapfile -t files
> +	[[ ${#files[@]} -eq 0 ]] && die "no changes detected"
> +
> +	for file in "${files[@]}"; do
> +		local rel_file="${file#"$DIFF_DIR"/}"
> +		local kmod_file="$KMOD_DIR/$rel_file"
> +		local cmd_file
> +
> +		mkdir -p "$(dirname "$kmod_file")"
> +		cp -f "$file" "$kmod_file"
> +
> +		# Tell kbuild this is a prebuilt object
> +		cp -f "$file" "${kmod_file}_shipped"
> +
> +		echo -n " $rel_file" >> "$makefile"
> +
> +		cmd_file="$ORIG_DIR/$(dirname "$rel_file")/.$(basename "$rel_file").cmd"
> +		[[ -e "$cmd_file" ]] && cp -f "$cmd_file" "$(dirname "$kmod_file")"

Hi Josh,

Should the .cmd file copy come from the reference SRC and not original
ORIG directory?

  cmd_file="$SRC/$(dirname "$rel_file")/.$(basename "$rel_file").cmd"

because I don't see any .cmd files in klp-tmp/orig/

FWIW, I only noticed this after backporting the series to
centos-stream-10.  There, I got this build error:

  Building original kernel
  Copying original object files
  Fixing patches
  Building patched kernel
  Copying patched object files
  Diffing objects
  vmlinux.o: changed function: cmdline_proc_show
  Building patch module: livepatch-test.ko
  <...>/klp-tmp/kmod/.vmlinux.o.cmd: No such file or directory
  make[2]: *** [scripts/Makefile.modpost:145:
<...>/klp-tmp/kmod/Module.symvers] Error 1
 make[1]: *** [<...>/Makefile:1936: modpost] Error 2
 make: *** [Makefile:236: __sub-make] Error 2

The above edit worked for both your upstream branch and my downstream
backport.

-- 
Joe

On Fri, Jun 06, 2025 at 09:05:59AM -0400, Joe Lawrence wrote:
> Should the .cmd file copy come from the reference SRC and not original
> ORIG directory?
> 
>   cmd_file="$SRC/$(dirname "$rel_file")/.$(basename "$rel_file").cmd"
> 
> because I don't see any .cmd files in klp-tmp/orig/
> 
> FWIW, I only noticed this after backporting the series to
> centos-stream-10.  There, I got this build error:
> 
>   Building original kernel
>   Copying original object files
>   Fixing patches
>   Building patched kernel
>   Copying patched object files
>   Diffing objects
>   vmlinux.o: changed function: cmdline_proc_show
>   Building patch module: livepatch-test.ko
>   <...>/klp-tmp/kmod/.vmlinux.o.cmd: No such file or directory
>   make[2]: *** [scripts/Makefile.modpost:145:
> <...>/klp-tmp/kmod/Module.symvers] Error 1
>  make[1]: *** [<...>/Makefile:1936: modpost] Error 2
>  make: *** [Makefile:236: __sub-make] Error 2
> 
> The above edit worked for both your upstream branch and my downstream
> backport.

Hm, I broke this in one of my refactorings before posting.

Is this with CONFIG_MODVERSIONS?

If you get a chance to test, here's a fix (currently untested):

diff --git a/scripts/livepatch/klp-build b/scripts/livepatch/klp-build
index 277fbe948730..cd6e118da275 100755
--- a/scripts/livepatch/klp-build
+++ b/scripts/livepatch/klp-build
@@ -517,16 +517,29 @@ find_objects() {
 
 # Copy all objects (.o archives) to $ORIG_DIR
 copy_orig_objects() {
+	local files=()
 
 	rm -rf "$ORIG_DIR"
 	mkdir -p "$ORIG_DIR"
 
-	(
-		cd "$OBJ"
-		find_objects						\
-			| sed 's/\.ko$/.o/'				\
-			| xargs cp --parents --target-directory="$ORIG_DIR"
-	)
+	find_objects | mapfile -t files
+
+	xtrace_save "copying orig objects"
+	for _file in "${files[@]}"; do
+		local rel_file="${_file/.ko/.o}"
+		local file="$OBJ/$rel_file"
+		local file_dir="$(dirname "$file")"
+		local orig_file="$ORIG_DIR/$rel_file"
+		local orig_dir="$(dirname "$orig_file")"
+		local cmd_file="$file_dir/.$(basename "$file").cmd"
+
+		[[ ! -f "$file" ]] && die "missing $(basename "$file") for $_file"
+
+		mkdir -p "$orig_dir"
+		cp -f "$file" "$orig_dir"
+		[[ -e "$cmd_file" ]] && cp -f "$cmd_file" "$orig_dir"
+	done
+	xtrace_restore
 
 	mv -f "$TMP_DIR/build.log" "$ORIG_DIR"
 	touch "$TIMESTAMP"

On Fri, Jun 06, 2025 at 12:03:45PM -0700, Josh Poimboeuf wrote:
> On Fri, Jun 06, 2025 at 09:05:59AM -0400, Joe Lawrence wrote:
> > Should the .cmd file copy come from the reference SRC and not original
> > ORIG directory?
> > 
> >   cmd_file="$SRC/$(dirname "$rel_file")/.$(basename "$rel_file").cmd"
> > 
> > because I don't see any .cmd files in klp-tmp/orig/
> > 
> > FWIW, I only noticed this after backporting the series to
> > centos-stream-10.  There, I got this build error:
> > 
> >   Building original kernel
> >   Copying original object files
> >   Fixing patches
> >   Building patched kernel
> >   Copying patched object files
> >   Diffing objects
> >   vmlinux.o: changed function: cmdline_proc_show
> >   Building patch module: livepatch-test.ko
> >   <...>/klp-tmp/kmod/.vmlinux.o.cmd: No such file or directory
> >   make[2]: *** [scripts/Makefile.modpost:145:
> > <...>/klp-tmp/kmod/Module.symvers] Error 1
> >  make[1]: *** [<...>/Makefile:1936: modpost] Error 2
> >  make: *** [Makefile:236: __sub-make] Error 2
> > 
> > The above edit worked for both your upstream branch and my downstream
> > backport.
> 
> Hm, I broke this in one of my refactorings before posting.
> 
> Is this with CONFIG_MODVERSIONS?
> 
> If you get a chance to test, here's a fix (currently untested):

It was indeed CONFIG_MODVERSIONS.  I verified the fix works.

All the latest fixes are in my klp-build branch:

  git://git.kernel.org/pub/scm/linux/kernel/git/jpoimboe/linux.git klp-build

I hope to post v3 next week and then start looking at merging patches --
if not all of them, then at least the first ~40 dependency patches which
are mostly standalone improvements.

-- 
Josh

On 6/6/25 4:28 PM, Josh Poimboeuf wrote:
> On Fri, Jun 06, 2025 at 12:03:45PM -0700, Josh Poimboeuf wrote:
>> On Fri, Jun 06, 2025 at 09:05:59AM -0400, Joe Lawrence wrote:
>>> Should the .cmd file copy come from the reference SRC and not original
>>> ORIG directory?
>>>
>>>   cmd_file="$SRC/$(dirname "$rel_file")/.$(basename "$rel_file").cmd"
>>>
>>> because I don't see any .cmd files in klp-tmp/orig/
>>>
>>> FWIW, I only noticed this after backporting the series to
>>> centos-stream-10.  There, I got this build error:
>>>
>>>   Building original kernel
>>>   Copying original object files
>>>   Fixing patches
>>>   Building patched kernel
>>>   Copying patched object files
>>>   Diffing objects
>>>   vmlinux.o: changed function: cmdline_proc_show
>>>   Building patch module: livepatch-test.ko
>>>   <...>/klp-tmp/kmod/.vmlinux.o.cmd: No such file or directory
>>>   make[2]: *** [scripts/Makefile.modpost:145:
>>> <...>/klp-tmp/kmod/Module.symvers] Error 1
>>>  make[1]: *** [<...>/Makefile:1936: modpost] Error 2
>>>  make: *** [Makefile:236: __sub-make] Error 2
>>>
>>> The above edit worked for both your upstream branch and my downstream
>>> backport.
>>
>> Hm, I broke this in one of my refactorings before posting.
>>
>> Is this with CONFIG_MODVERSIONS?
>>
>> If you get a chance to test, here's a fix (currently untested):
> 
> It was indeed CONFIG_MODVERSIONS.  I verified the fix works.
> 
> All the latest fixes are in my klp-build branch:
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/jpoimboe/linux.git klp-build
> 
> I hope to post v3 next week and then start looking at merging patches --
> if not all of them, then at least the first ~40 dependency patches which
> are mostly standalone improvements.
> 

Ack, I tested this update with CONFIG_MODVERSIONS for both vmlinux and
module cases with success :)

-- 
Joe