Re: [RFC 4/4] livepatch/klp-build: add basic out-of-tree module patching support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] The script hardcodes the temporary directory (klp-tmp) and default output file to the kernel root directory ($PWD), which breaks building out-of-tree modules against read-only system headers.
- [Medium] The commit message incorrectly states that 'make modules_prepare' is sufficient to prepare the kernel tree for out-of-tree patching.
--

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

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

This commit introduces a module-dir option to klp-build to support
generating livepatches for out-of-tree kernel modules. It performs two
builds of the out-of-tree module to generate the original and patched
objects.

>     klp-build is currently limited to patching in-tree kernel modules.
>     Introduce a -M/--module-dir option to enable livepatch generation for
>     basic out-of-tree (OOT) modules.  This requires the associated kernel
>     tree to be pre-configured (e.g., 'make modules_prepare').

The commit message states that running 'make modules_prepare' is
sufficient to prepare the tree. Is this description accurate?

Looking at do_init(), the script explicitly checks for Module.symvers:

[[ -f "$PWD/Module.symvers" ]] ||
    die "kernel must be built first (no Module.symvers in $PWD)"

Running 'make modules_prepare' on a clean kernel source tree prepares
configuration and headers but does not generate Module.symvers.

Could the commit message be updated to clarify that a full build is
required?

> diff --git a/scripts/livepatch/klp-build b/scripts/livepatch/klp-build
> index c4a7acf8edc3f..aef3b47db4f24 100755
> --- a/scripts/livepatch/klp-build
> +++ b/scripts/livepatch/klp-build

[ ... ]

> @@ -433,8 +451,6 @@ validate_patches() {
>  }
>  
>  do_init() {
> -	# We're not yet smart enough to handle anything other than in-tree
> -	# builds in pwd.
>  	[[ ! "$PWD" -ef "$SCRIPT_DIR/../.." ]] && die "please run from the kernel root directory"

[ ... ]

> +	if [[ -n "$MODULE_DIR" ]]; then
> +		[[ -d "$MODULE_DIR" ]] || die "module directory not found: $MODULE_DIR"
> +		MODULE_DIR="$(realpath "$MODULE_DIR")"
> +		[[ -f "$MODULE_DIR/Kbuild" || -f "$MODULE_DIR/Makefile" ]] ||
> +			die "no Kbuild or Makefile in $MODULE_DIR"
> +		[[ -f "$PWD/Module.symvers" ]] ||
> +			die "kernel must be built first (no Module.symvers in $PWD)"
> +	fi
> +
>  	(( SHORT_CIRCUIT <= 1 )) && rm -rf "$TMP_DIR"
>  	mkdir -p "$TMP_DIR"

Does creating the temporary directory in the kernel root directory cause
permission regressions when building out-of-tree modules?

Out-of-tree modules are often built against system-installed kernel headers,
which are typically read-only for standard users. Since the script mandates
running from the kernel root directory, creating the temporary working
directory and default output file inside the present working directory will
cause permission denied errors.

Could the script use the module directory or a user-specified path for
temporary files and the default output instead?

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