Re: [RFC PATCH v2 5/7] livepatch/klp-build: add basic out-of-tree module support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The `apply_patch` function omits the `-d` directory option during the actual patch application, failing to apply patches to out-of-tree modules.
- [Medium] Unconditional requirement for `vmlinux` unnecessarily prevents out-of-tree module livepatching against standard distro kernel headers.
--

commit aa6b14041f6a905023bf4d110f2171d49e9e8c84
Author: Joe Lawrence <joe.lawrence@redhat.com>

livepatch/klp-build: add basic out-of-tree module support

klp-build is currently limited to patching in-tree kernel modules.
Introduce a --oot-dir option to enable livepatch generation for basic
out-of-tree (OOT) modules.  This requires the associated kernel tree to
be fully built (Module.symvers must exist).

> diff --git a/scripts/livepatch/klp-build b/scripts/livepatch/klp-build
> index b52a8489d9f67..b60f5a5da31e8 100755
> --- a/scripts/livepatch/klp-build
> +++ b/scripts/livepatch/klp-build

[ ... ]

> @@ -377,13 +393,14 @@ apply_patch() {
>  	local patch="$1"
>  	shift
>  	local extra_args=("$@")
> +	local patch_target="${OOT_DIR:-$PWD}"
>  	local drift_regex="with fuzz|offset [0-9]+ line"
>  	local output
>  	local status
>  
>  	[[ ! -f "$patch" ]] && die "$patch doesn't exist"
>  	status=0
> -	output=$(patch -p1 --dry-run --no-backup-if-mismatch -r /dev/null "${extra_args[@]}" < "$patch" 2>&1) || status=$?
> +	output=$(patch -d "$patch_target" -p1 --dry-run --no-backup-if-mismatch -r /dev/null "${extra_args[@]}" < "$patch" 2>&1) || status=$?
>  	if [[ "$status" -ne 0 ]]; then
>  		echo "$output" >&2
>  		die "$patch did not apply"

[Severity: High]
Does this actual patch application omit the -d directory argument?

It looks like the -d "$patch_target" argument was correctly added to the
dry-run here, but not to the real patch application lower down in
apply_patch():

    patch -p1 --no-backup-if-mismatch -r /dev/null "${extra_args[@]}" --silent < "$patch"

When running klp-build --oot-dir <dir> <patch>, won't this attempt to apply
the patch against the kernel root directory instead of the out-of-tree module?
This seems like it would cause the patch application to fail, or erroneously
modify identically-named files in the kernel tree.

[ ... ]

> @@ -597,10 +635,11 @@ copy_orig_objects() {
>  
>  	find_objects | mapfile -t files
>  
> +	local obj_root="${OOT_DIR:-$PWD}"
>  	xtrace_save "copying original objects"
>  	for _file in "${files[@]}"; do
>  		local rel_file="${_file/.ko/.o}"
> -		local file="$PWD/$rel_file"
> +		local file="$obj_root/$rel_file"
>  		local orig_file="$ORIG_DIR/$rel_file"
>  		local orig_dir="$(dirname "$orig_file")"
>  

[Severity: Medium]
Will this script crash for valid out-of-tree module builds against standard
distro kernel headers?

Lower down in copy_orig_objects(), there is an unconditional requirement for
vmlinux to be present:

    cp -f "$PWD/vmlinux" "$ORIG_DIR" || die "missing vmlinux"

A primary use case for out-of-tree modules is building against kernel headers
which provide Module.symvers but do not include vmlinux. Since vmlinux is
unused during the object diffing steps for out-of-tree modules, should this
dependency be skipped when running klp-build --oot-dir <dir> <patch> to avoid
unnecessarily preventing livepatching against headers?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260826195000.455905-1-joe.lawrence@redhat.com?part=5