Re: [PATCH v2 59/62] livepatch/klp-build: Introduce klp-build script for generating livepatch modules
On Wed, Jun 18, 2025 at 05:38:07PM -0500, Dylan Hatch wrote:
> On Fri, May 9, 2025 at 1:30 PM Josh Poimboeuf 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: ", 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
Re: [PATCH v2 59/62] livepatch/klp-build: Introduce klp-build script for generating livepatch modules
On Fri, May 9, 2025 at 1:30 PM Josh Poimboeuf 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: ", 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
Re: [PATCH v2 59/62] livepatch/klp-build: Introduce klp-build script for generating livepatch modules
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
Re: [PATCH v2 59/62] livepatch/klp-build: Introduce klp-build script for generating livepatch modules
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() {
Re: [PATCH v2 59/62] livepatch/klp-build: Introduce klp-build script for generating livepatch modules
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
Re: [PATCH v2 59/62] livepatch/klp-build: Introduce klp-build script for generating livepatch modules
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 ("--- \t").
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
Re: [PATCH v2 59/62] livepatch/klp-build: Introduce klp-build script for generating livepatch modules
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 ("--- \t").
> 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
Re: [PATCH v2 59/62] livepatch/klp-build: Introduce klp-build script for generating livepatch modules
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
> ---
> 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 ("--- \t").
> +# 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
Re: [PATCH v2 59/62] livepatch/klp-build: Introduce klp-build script for generating livepatch modules
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
Re: [PATCH v2 59/62] livepatch/klp-build: Introduce klp-build script for generating livepatch modules
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;
Re: [PATCH v2 59/62] livepatch/klp-build: Introduce klp-build script for generating livepatch modules
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
Re: [PATCH v2 59/62] livepatch/klp-build: Introduce klp-build script for generating livepatch modules
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= '/\ 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
Re: [PATCH v2 59/62] livepatch/klp-build: Introduce klp-build script for generating livepatch modules
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 " >> "$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= '/\>
"$KMOD_DIR/namespaces.c"
+ done
+
mkdir -p "$(dirname "$kmod_file")"
cp -f "$file" "$kmod_file"
Re: [PATCH v2 59/62] livepatch/klp-build: Introduce klp-build script for generating livepatch modules
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
Re: [PATCH v2 59/62] livepatch/klp-build: Introduce klp-build script for generating livepatch modules
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 " >> "$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
Re: [PATCH v2 59/62] livepatch/klp-build: Introduce klp-build script for generating livepatch modules
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"
}
Re: [PATCH v2 59/62] livepatch/klp-build: Introduce klp-build script for generating livepatch modules
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
Re: [PATCH v2 59/62] livepatch/klp-build: Introduce klp-build script for generating livepatch modules
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
Re: [PATCH v2 59/62] livepatch/klp-build: Introduce klp-build script for generating livepatch modules
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
Re: [PATCH v2 59/62] livepatch/klp-build: Introduce klp-build script for generating livepatch modules
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"
Re: [PATCH v2 59/62] livepatch/klp-build: Introduce klp-build script for generating livepatch modules
> +# 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
