* RE: [PATCH 1/2] objtool: Set minimum xxhash version to 0.8
From: Michael Kelley @ 2025-11-13 2:25 UTC (permalink / raw)
To: Josh Poimboeuf, x86@kernel.org
Cc: linux-kernel@vger.kernel.org, Peter Zijlstra,
live-patching@vger.kernel.org
In-Reply-To: <7227c94692a3a51840278744c7af31b4797c6b96.1762990139.git.jpoimboe@kernel.org>
From: Josh Poimboeuf <jpoimboe@kernel.org> Sent: Wednesday, November 12, 2025 3:33 PM
>
> XXH3 is only supported starting with xxhash 0.8. Enforce that.
>
> Fixes: 0d83da43b1e1 ("objtool/klp: Add --checksum option to generate per-function checksums")
> Reported-by: Michael Kelley <mhklinux@outlook.com>
> Closes: https://lore.kernel.org/all/SN6PR02MB41579B83CD295C9FEE40EED6D4FCA@SN6PR02MB4157.namprd02.prod.outlook.com
> Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
> ---
> tools/objtool/Makefile | 2 +-
> tools/objtool/builtin-check.c | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/tools/objtool/Makefile b/tools/objtool/Makefile
> index 48928c9bebef..021f55b7bd87 100644
> --- a/tools/objtool/Makefile
> +++ b/tools/objtool/Makefile
> @@ -12,7 +12,7 @@ ifeq ($(SRCARCH),loongarch)
> endif
>
> ifeq ($(ARCH_HAS_KLP),y)
> - HAVE_XXHASH = $(shell echo "int main() {}" | \
> + HAVE_XXHASH = $(shell printf "$(pound)include <xxhash.h>\nXXH3_state_t *state;int main() {}" | \
> $(HOSTCC) -xc - -o /dev/null -lxxhash 2> /dev/null && echo y || echo n)
> ifeq ($(HAVE_XXHASH),y)
> BUILD_KLP := y
> diff --git a/tools/objtool/builtin-check.c b/tools/objtool/builtin-check.c
> index 1e1ea8396eb3..aab7fa9c7e00 100644
> --- a/tools/objtool/builtin-check.c
> +++ b/tools/objtool/builtin-check.c
> @@ -164,7 +164,7 @@ static bool opts_valid(void)
>
> #ifndef BUILD_KLP
> if (opts.checksum) {
> - ERROR("--checksum not supported; install xxhash-devel/libxxhash-dev and recompile");
> + ERROR("--checksum not supported; install xxhash-devel/libxxhash-dev (version >= 0.8) and recompile");
> return false;
> }
> #endif
> --
> 2.51.1
Reviewed-by: Michael Kelley <mhklinux@outlook.com>
Tested-by: Michael Kelley <mhklinux@outlook.com>
^ permalink raw reply
* [PATCH 4/4] objtool: Warn on functions with ambiguous -ffunction-sections section names
From: Josh Poimboeuf @ 2025-11-12 23:47 UTC (permalink / raw)
To: x86; +Cc: linux-kernel, Peter Zijlstra, live-patching
In-Reply-To: <cover.1762991150.git.jpoimboe@kernel.org>
When compiled with -ffunction-sections, a function named startup() will
be placed in .text.startup. However, .text.startup is also used by the
compiler for functions with __attribute__((constructor)).
That creates an ambiguity for the vmlinux linker script, which needs to
differentiate those two cases.
Similar naming conflicts exist for functions named exit(), split(),
unlikely(), hot() and unknown().
One potential solution would be to use '#ifdef CC_USING_FUNCTION_SECTIONS'
to create two distinct implementations of the TEXT_MAIN macro. However,
-ffunction-sections can be (and is) enabled or disabled on a per-object
basis (for example via ccflags-y or AUTOFDO_PROFILE).
So the recently unified TEXT_MAIN macro (commit 1ba9f8979426
("vmlinux.lds: Unify TEXT_MAIN, DATA_MAIN, and related macros")) is
necessary. This means there's no way for the linker script to
disambiguate things.
Instead, use objtool to warn on any function names whose resulting
section names might create ambiguity when the kernel is compiled (in
whole or in part) with -ffunction-sections.
Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
---
include/asm-generic/vmlinux.lds.h | 15 +++++++++++
tools/objtool/Documentation/objtool.txt | 7 ++++++
tools/objtool/check.c | 33 +++++++++++++++++++++++++
3 files changed, 55 insertions(+)
diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
index 20695bc8f174..57aa01d24087 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -97,6 +97,21 @@
* Other .text.* sections that are typically grouped separately, such as
* .text.unlikely or .text.hot, must be matched explicitly before using
* TEXT_MAIN.
+ *
+ * NOTE: builds *with* and *without* -ffunction-sections are both supported by
+ * this single macro. Even with -ffunction-sections, there may be some objects
+ * NOT compiled with the flag due to the use of a specific Makefile override
+ * like cflags-y or AUTOFDO_PROFILE_foo.o. So this single catchall rule is
+ * needed to support mixed object builds.
+ *
+ * One implication is that functions named startup(), exit(), split(),
+ * unlikely(), hot(), and unknown() are not allowed in the kernel due to the
+ * ambiguity of their section names with -ffunction-sections. For example,
+ * .text.startup could be __attribute__((constructor)) code in a *non*
+ * ffunction-sections object, which should be placed in .init.text; or it could
+ * be an actual function named startup() in an ffunction-sections object, which
+ * should be placed in .text. Objtool will detect and complain about any such
+ * ambiguously named functions.
*/
#define TEXT_MAIN \
.text \
diff --git a/tools/objtool/Documentation/objtool.txt b/tools/objtool/Documentation/objtool.txt
index 9e97fc25b2d8..f88f8d28513a 100644
--- a/tools/objtool/Documentation/objtool.txt
+++ b/tools/objtool/Documentation/objtool.txt
@@ -456,6 +456,13 @@ the objtool maintainers.
these special names and does not use module_init() / module_exit()
macros to create them.
+13. file.o: warning: func() function name creates ambiguity with -ffunctions-sections
+
+ Functions named startup(), exit(), split(), unlikely(), hot(), and
+ unknown() are not allowed due to the ambiguity of their section
+ names when compiled with -ffunction-sections. For more information,
+ see the comment above TEXT_MAIN in include/asm-generic/vmlinux.lds.h.
+
If the error doesn't seem to make sense, it could be a bug in objtool.
Feel free to ask objtool maintainers for help.
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index d071fbf73e4c..ac78f6ec9758 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -2662,6 +2662,37 @@ static int decode_sections(struct objtool_file *file)
return 0;
}
+/*
+ * Certain function names are disallowed due to section name ambiguities
+ * introduced by -ffunction-sections.
+ *
+ * See the comment above TEXT_MAIN in include/asm-generic/vmlinux.lds.h.
+ */
+static int validate_function_names(struct objtool_file *file)
+{
+ struct symbol *func;
+ int warnings = 0;
+
+ for_each_sym(file->elf, func) {
+ if (!is_func_sym(func))
+ continue;
+
+ if (!strcmp(func->name, "startup") || strstarts(func->name, "startup.") ||
+ !strcmp(func->name, "exit") || strstarts(func->name, "exit.") ||
+ !strcmp(func->name, "split") || strstarts(func->name, "split.") ||
+ !strcmp(func->name, "unlikely") || strstarts(func->name, "unlikely.") ||
+ !strcmp(func->name, "hot") || strstarts(func->name, "hot.") ||
+ !strcmp(func->name, "unknown") || strstarts(func->name, "unknown.")) {
+
+ WARN("%s() function name creates ambiguity with -ffunction-sections",
+ func->name);
+ warnings++;
+ }
+ }
+
+ return warnings;
+}
+
static bool is_special_call(struct instruction *insn)
{
if (insn->type == INSN_CALL) {
@@ -4928,6 +4959,8 @@ int check(struct objtool_file *file)
if (!nr_insns)
goto out;
+ warnings += validate_function_names(file);
+
if (opts.retpoline)
warnings += validate_retpoline(file);
--
2.51.1
^ permalink raw reply related
* [PATCH 3/4] drivers/xen/xenbus: Fix split() section placement with AutoFDO
From: Josh Poimboeuf @ 2025-11-12 23:47 UTC (permalink / raw)
To: x86; +Cc: linux-kernel, Peter Zijlstra, live-patching, Juergen Gross
In-Reply-To: <cover.1762991150.git.jpoimboe@kernel.org>
When compiling the kernel with -ffunction-sections enabled, the split()
function gets compiled into the .text.split section. In some cases it
can even be cloned into .text.split.constprop.0 or .text.split.isra.0.
However, .text.split.* is already reserved for use by the Clang
-fsplit-machine-functions flag, which is used by AutoFDO. That may
place part of a function's code in a .text.split.<func> section.
This naming conflict causes the vmlinux linker script to wrongly place
split() with other .text.split.* code, rather than where it belongs with
regular text.
Fix it by renaming split() to split_strings().
Cc: Juergen Gross <jgross@suse.com>
Fixes: 6568f14cb5ae ("vmlinux.lds: Exclude .text.startup and .text.exit from TEXT_MAIN")
Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
---
drivers/xen/xenbus/xenbus_xs.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/xen/xenbus/xenbus_xs.c b/drivers/xen/xenbus/xenbus_xs.c
index 528682bf0c7f..f794014814be 100644
--- a/drivers/xen/xenbus/xenbus_xs.c
+++ b/drivers/xen/xenbus/xenbus_xs.c
@@ -410,7 +410,7 @@ static char *join(const char *dir, const char *name)
return (!buffer) ? ERR_PTR(-ENOMEM) : buffer;
}
-static char **split(char *strings, unsigned int len, unsigned int *num)
+static char **split_strings(char *strings, unsigned int len, unsigned int *num)
{
char *p, **ret;
@@ -448,7 +448,7 @@ char **xenbus_directory(struct xenbus_transaction t,
if (IS_ERR(strings))
return ERR_CAST(strings);
- return split(strings, len, num);
+ return split_strings(strings, len, num);
}
EXPORT_SYMBOL_GPL(xenbus_directory);
--
2.51.1
^ permalink raw reply related
* [PATCH 2/4] media: atomisp: Fix startup() section placement with -ffunction-sections
From: Josh Poimboeuf @ 2025-11-12 23:47 UTC (permalink / raw)
To: x86
Cc: linux-kernel, Peter Zijlstra, live-patching, Hans de Goede,
Mauro Carvalho Chehab
In-Reply-To: <cover.1762991150.git.jpoimboe@kernel.org>
When compiling the kernel with -ffunction-sections (e.g., for LTO,
livepatch, dead code elimination, AutoFDO, or Propeller), the startup()
function gets compiled into the .text.startup section. In some cases it
can even be cloned into .text.startup.constprop.0 or
.text.startup.isra.0.
However, the .text.startup and .text.startup.* section names are already
reserved for use by the compiler for __attribute__((constructor)) code.
This naming conflict causes the vmlinux linker script to wrongly place
startup() function code in .init.text, which gets freed during boot.
Fix that by renaming startup() to ov2722_startup().
Cc: Hans de Goede <hansg@kernel.org>
Cc: Mauro Carvalho Chehab <mchehab@kernel.org>
Fixes: 6568f14cb5ae ("vmlinux.lds: Exclude .text.startup and .text.exit from TEXT_MAIN")
Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
---
drivers/staging/media/atomisp/i2c/atomisp-ov2722.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c b/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c
index c7de7800799a..a4519babf37d 100644
--- a/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c
+++ b/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c
@@ -600,7 +600,7 @@ static int ov2722_s_power(struct v4l2_subdev *sd, int on)
}
/* TODO: remove it. */
-static int startup(struct v4l2_subdev *sd)
+static int ov2722_startup(struct v4l2_subdev *sd)
{
struct ov2722_device *dev = to_ov2722_sensor(sd);
struct i2c_client *client = v4l2_get_subdevdata(sd);
@@ -662,7 +662,7 @@ static int ov2722_set_fmt(struct v4l2_subdev *sd,
dev->pixels_per_line = dev->res->pixels_per_line;
dev->lines_per_frame = dev->res->lines_per_frame;
- ret = startup(sd);
+ ret = ov2722_startup(sd);
if (ret) {
int i = 0;
@@ -677,7 +677,7 @@ static int ov2722_set_fmt(struct v4l2_subdev *sd,
dev_err(&client->dev, "power up failed, continue\n");
continue;
}
- ret = startup(sd);
+ ret = ov2722_startup(sd);
if (ret) {
dev_err(&client->dev, " startup FAILED!\n");
} else {
--
2.51.1
^ permalink raw reply related
* [PATCH 1/4] vmlinux.lds: Fix TEXT_MAIN to include .text.start and friends
From: Josh Poimboeuf @ 2025-11-12 23:47 UTC (permalink / raw)
To: x86; +Cc: linux-kernel, Peter Zijlstra, live-patching, kernel test robot
In-Reply-To: <cover.1762991150.git.jpoimboe@kernel.org>
Since commit 6568f14cb5ae ("vmlinux.lds: Exclude .text.startup and
.text.exit from TEXT_MAIN"), the TEXT_MAIN macro uses a series of
patterns to prevent the .text.startup[.*] and .text.exit[.*] sections
from getting linked into vmlinux runtime .text.
That commit is a tad too aggressive: it also inadvertently filters out
valid runtime text sections like .text.start and
.text.start.constprop.0, which can be generated for a function named
start() when -ffunction-sections is enabled.
As a result, those sections become orphans when building with
CONFIG_LD_DEAD_CODE_DATA_ELIMINATION for arm:
arm-linux-gnueabi-ld: warning: orphan section `.text.start.constprop.0' from `drivers/usb/host/sl811-hcd.o' being placed in section `.text.start.constprop.0'
arm-linux-gnueabi-ld: warning: orphan section `.text.start.constprop.0' from `drivers/media/dvb-frontends/drxk_hard.o' being placed in section `.text.start.constprop.0'
arm-linux-gnueabi-ld: warning: orphan section `.text.start' from `drivers/media/dvb-frontends/stv0910.o' being placed in section `.text.start'
arm-linux-gnueabi-ld: warning: orphan section `.text.start.constprop.0' from `drivers/media/pci/ddbridge/ddbridge-sx8.o' being placed in section `.text.start.constprop.0'
Fix that by explicitly adding the partial "substring" sections (.text.s,
.text.st, .text.sta, etc) and their cloned derivatives.
While this unfortunately means that TEXT_MAIN continues to grow, these
changes are ultimately necessary for proper support of
-ffunction-sections.
Fixes: 6568f14cb5ae ("vmlinux.lds: Exclude .text.startup and .text.exit from TEXT_MAIN")
Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202511040812.DFGedJiy-lkp@intel.com/
Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
---
include/asm-generic/vmlinux.lds.h | 23 ++++++++++++-----------
1 file changed, 12 insertions(+), 11 deletions(-)
diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
index 9de1d900fa15..20695bc8f174 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -90,8 +90,9 @@
* Support -ffunction-sections by matching .text and .text.*,
* but exclude '.text..*', .text.startup[.*], and .text.exit[.*].
*
- * .text.startup and .text.startup.* are matched later by INIT_TEXT.
- * .text.exit and .text.exit.* are matched later by EXIT_TEXT.
+ * .text.startup and .text.startup.* are matched later by INIT_TEXT, and
+ * .text.exit and .text.exit.* are matched later by EXIT_TEXT, so they must be
+ * explicitly excluded here.
*
* Other .text.* sections that are typically grouped separately, such as
* .text.unlikely or .text.hot, must be matched explicitly before using
@@ -100,16 +101,16 @@
#define TEXT_MAIN \
.text \
.text.[_0-9A-Za-df-rt-z]* \
- .text.s[_0-9A-Za-su-z]* \
- .text.st[_0-9A-Zb-z]* \
- .text.sta[_0-9A-Za-qs-z]* \
- .text.star[_0-9A-Za-su-z]* \
- .text.start[_0-9A-Za-tv-z]* \
- .text.startu[_0-9A-Za-oq-z]* \
+ .text.s[_0-9A-Za-su-z]* .text.s .text.s.* \
+ .text.st[_0-9A-Zb-z]* .text.st .text.st.* \
+ .text.sta[_0-9A-Za-qs-z]* .text.sta .text.sta.* \
+ .text.star[_0-9A-Za-su-z]* .text.star .text.star.* \
+ .text.start[_0-9A-Za-tv-z]* .text.start .text.start.* \
+ .text.startu[_0-9A-Za-oq-z]* .text.startu .text.startu.* \
.text.startup[_0-9A-Za-z]* \
- .text.e[_0-9A-Za-wy-z]* \
- .text.ex[_0-9A-Za-hj-z]* \
- .text.exi[_0-9A-Za-su-z]* \
+ .text.e[_0-9A-Za-wy-z]* .text.e .text.e.* \
+ .text.ex[_0-9A-Za-hj-z]* .text.ex .text.ex.* \
+ .text.exi[_0-9A-Za-su-z]* .text.exi .text.exi.* \
.text.exit[_0-9A-Za-z]*
/*
--
2.51.1
^ permalink raw reply related
* [PATCH 0/4] objtool: Fix some -ffunction-sections edge cases
From: Josh Poimboeuf @ 2025-11-12 23:47 UTC (permalink / raw)
To: x86; +Cc: linux-kernel, Peter Zijlstra, live-patching
Fix some more fallout from commit 1ba9f8979426 ("vmlinux.lds: Unify
TEXT_MAIN, DATA_MAIN, and related macros").
For tip/objtool/core.
Josh Poimboeuf (4):
vmlinux.lds: Fix TEXT_MAIN to include .text.start and friends
media: atomisp: Fix startup() section placement with
-ffunction-sections
drivers/xen/xenbus: Fix split() section placement with AutoFDO
objtool: Warn on functions with ambiguous -ffunction-sections section
names
.../media/atomisp/i2c/atomisp-ov2722.c | 6 +--
drivers/xen/xenbus/xenbus_xs.c | 4 +-
include/asm-generic/vmlinux.lds.h | 38 +++++++++++++------
tools/objtool/Documentation/objtool.txt | 7 ++++
tools/objtool/check.c | 33 ++++++++++++++++
5 files changed, 72 insertions(+), 16 deletions(-)
--
2.51.1
^ permalink raw reply
* [PATCH 2/2] objtool/klp: Only enable --checksum when needed
From: Josh Poimboeuf @ 2025-11-12 23:32 UTC (permalink / raw)
To: x86; +Cc: linux-kernel, Peter Zijlstra, Michael Kelley, live-patching
In-Reply-To: <cover.1762990139.git.jpoimboe@kernel.org>
With CONFIG_KLP_BUILD enabled, checksums are only needed during a
klp-build run. There's no need to enable them for normal kernel builds.
This also has the benefit of softening the xxhash dependency.
Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
---
arch/x86/boot/startup/Makefile | 2 +-
scripts/Makefile.lib | 1 -
scripts/livepatch/klp-build | 4 ++++
3 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/arch/x86/boot/startup/Makefile b/arch/x86/boot/startup/Makefile
index e8fdf020b422..5e499cfb29b5 100644
--- a/arch/x86/boot/startup/Makefile
+++ b/arch/x86/boot/startup/Makefile
@@ -36,7 +36,7 @@ $(patsubst %.o,$(obj)/%.o,$(lib-y)): OBJECT_FILES_NON_STANDARD := y
# relocations, even if other objtool actions are being deferred.
#
$(pi-objs): objtool-enabled = 1
-$(pi-objs): objtool-args = $(if $(delay-objtool),,$(objtool-args-y)) --noabs
+$(pi-objs): objtool-args = $(if $(delay-objtool),--dry-run,$(objtool-args-y)) --noabs
#
# Confine the startup code by prefixing all symbols with __pi_ (for position
diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
index f4b33919ec37..28a1c08e3b22 100644
--- a/scripts/Makefile.lib
+++ b/scripts/Makefile.lib
@@ -173,7 +173,6 @@ ifdef CONFIG_OBJTOOL
objtool := $(objtree)/tools/objtool/objtool
-objtool-args-$(CONFIG_KLP_BUILD) += --checksum
objtool-args-$(CONFIG_HAVE_JUMP_LABEL_HACK) += --hacks=jump_label
objtool-args-$(CONFIG_HAVE_NOINSTR_HACK) += --hacks=noinstr
objtool-args-$(CONFIG_MITIGATION_CALL_DEPTH_TRACKING) += --hacks=skylake
diff --git a/scripts/livepatch/klp-build b/scripts/livepatch/klp-build
index 881e052e7fae..882272120c9e 100755
--- a/scripts/livepatch/klp-build
+++ b/scripts/livepatch/klp-build
@@ -489,8 +489,11 @@ clean_kernel() {
build_kernel() {
local log="$TMP_DIR/build.log"
+ local objtool_args=()
local cmd=()
+ objtool_args=("--checksum")
+
cmd=("make")
# When a patch to a kernel module references a newly created unexported
@@ -513,6 +516,7 @@ build_kernel() {
cmd+=("$VERBOSE")
cmd+=("-j$JOBS")
cmd+=("KCFLAGS=-ffunction-sections -fdata-sections")
+ cmd+=("OBJTOOL_ARGS=${objtool_args[*]}")
cmd+=("vmlinux")
cmd+=("modules")
--
2.51.1
^ permalink raw reply related
* [PATCH 1/2] objtool: Set minimum xxhash version to 0.8
From: Josh Poimboeuf @ 2025-11-12 23:32 UTC (permalink / raw)
To: x86; +Cc: linux-kernel, Peter Zijlstra, Michael Kelley, live-patching
In-Reply-To: <cover.1762990139.git.jpoimboe@kernel.org>
XXH3 is only supported starting with xxhash 0.8. Enforce that.
Fixes: 0d83da43b1e1 ("objtool/klp: Add --checksum option to generate per-function checksums")
Reported-by: Michael Kelley <mhklinux@outlook.com>
Closes: https://lore.kernel.org/SN6PR02MB41579B83CD295C9FEE40EED6D4FCA@SN6PR02MB4157.namprd02.prod.outlook.com
Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
---
tools/objtool/Makefile | 2 +-
tools/objtool/builtin-check.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/objtool/Makefile b/tools/objtool/Makefile
index 48928c9bebef..021f55b7bd87 100644
--- a/tools/objtool/Makefile
+++ b/tools/objtool/Makefile
@@ -12,7 +12,7 @@ ifeq ($(SRCARCH),loongarch)
endif
ifeq ($(ARCH_HAS_KLP),y)
- HAVE_XXHASH = $(shell echo "int main() {}" | \
+ HAVE_XXHASH = $(shell printf "$(pound)include <xxhash.h>\nXXH3_state_t *state;int main() {}" | \
$(HOSTCC) -xc - -o /dev/null -lxxhash 2> /dev/null && echo y || echo n)
ifeq ($(HAVE_XXHASH),y)
BUILD_KLP := y
diff --git a/tools/objtool/builtin-check.c b/tools/objtool/builtin-check.c
index 1e1ea8396eb3..aab7fa9c7e00 100644
--- a/tools/objtool/builtin-check.c
+++ b/tools/objtool/builtin-check.c
@@ -164,7 +164,7 @@ static bool opts_valid(void)
#ifndef BUILD_KLP
if (opts.checksum) {
- ERROR("--checksum not supported; install xxhash-devel/libxxhash-dev and recompile");
+ ERROR("--checksum not supported; install xxhash-devel/libxxhash-dev (version >= 0.8) and recompile");
return false;
}
#endif
--
2.51.1
^ permalink raw reply related
* [PATCH 0/2] objtool: xxhash dependency improvements
From: Josh Poimboeuf @ 2025-11-12 23:32 UTC (permalink / raw)
To: x86; +Cc: linux-kernel, Peter Zijlstra, Michael Kelley, live-patching
Set the minimum xxhash version to 0.8, and soften the xxhash dependency
a bit so that it's only needed when running klp-build rather than doing
a normal kernel build.
For tip/objtool/core.
Josh Poimboeuf (2):
objtool: Set minimum xxhash version to 0.8
objtool/klp: Only enable --checksum when needed
arch/x86/boot/startup/Makefile | 2 +-
scripts/Makefile.lib | 1 -
scripts/livepatch/klp-build | 4 ++++
tools/objtool/Makefile | 2 +-
tools/objtool/builtin-check.c | 2 +-
5 files changed, 7 insertions(+), 4 deletions(-)
--
2.51.1
^ permalink raw reply
* Re: [PATCH v4 49/63] objtool/klp: Add --checksum option to generate per-function checksums
From: David Laight @ 2025-11-12 21:39 UTC (permalink / raw)
To: Josh Poimboeuf
Cc: Michael Kelley, x86@kernel.org, linux-kernel@vger.kernel.org,
Petr Mladek, Miroslav Benes, Joe Lawrence,
live-patching@vger.kernel.org, Song Liu, laokz, Jiri Kosina,
Marcos Paulo de Souza, Weinan Liu, Fazla Mehrab, Chen Zhongjin,
Puranjay Mohan, Dylan Hatch, Peter Zijlstra
In-Reply-To: <tujcypul6y3kmgwbrljozyce7lromotvgaoql26c6tdjnqk4r6@yycdxcvj2knz>
On Wed, 12 Nov 2025 08:16:59 -0800
Josh Poimboeuf <jpoimboe@kernel.org> wrote:
> On Wed, Nov 12, 2025 at 01:25:57PM +0000, David Laight wrote:
> > On Wed, 12 Nov 2025 04:32:02 +0000 Michael Kelley <mhklinux@outlook.com> wrote:
> > > From: Josh Poimboeuf <jpoimboe@kernel.org> Sent: Tuesday, November 11, 2025 8:04 PM
> > > > On Wed, Nov 12, 2025 at 02:26:18AM +0000, Michael Kelley wrote:
..
> > > > Does "$(pound)" work? This seems to work here:
> >
> > Please not 'pound' - that is the uk currency symbol (not what US greengrocers
> > scrawl for lb).
>
> While I do call it the "pound sign", I can't take the credit/blame for
> that name it as the variable already exists.
>
> It's better than "hashtag" which is what my kids call it :-/
'£' is a "pound sign", '#' is a "hash".
David
^ permalink raw reply
* Re: [PATCH v4 49/63] objtool/klp: Add --checksum option to generate per-function checksums
From: Josh Poimboeuf @ 2025-11-12 16:16 UTC (permalink / raw)
To: David Laight
Cc: Michael Kelley, x86@kernel.org, linux-kernel@vger.kernel.org,
Petr Mladek, Miroslav Benes, Joe Lawrence,
live-patching@vger.kernel.org, Song Liu, laokz, Jiri Kosina,
Marcos Paulo de Souza, Weinan Liu, Fazla Mehrab, Chen Zhongjin,
Puranjay Mohan, Dylan Hatch, Peter Zijlstra
In-Reply-To: <20251112132557.6928f799@pumpkin>
On Wed, Nov 12, 2025 at 01:25:57PM +0000, David Laight wrote:
> On Wed, 12 Nov 2025 04:32:02 +0000 Michael Kelley <mhklinux@outlook.com> wrote:
> > From: Josh Poimboeuf <jpoimboe@kernel.org> Sent: Tuesday, November 11, 2025 8:04 PM
> > > On Wed, Nov 12, 2025 at 02:26:18AM +0000, Michael Kelley wrote:
> > > > > 2) With make v4.2.1 on my Ubuntu 20.04 system, the "#" character in the
> > > > > "#include" added to the echo command is problematic. "make" seems to be
> > > > > treating it as a comment character, though I'm not 100% sure of that
> > > > > interpretation. Regardless, the "#" causes a syntax error in the "make" shell
> > > > > command. Adding a backslash before the "#" solves that problem. On an Ubuntu
> > > > > 24.04 system with make v4.3, the "#" does not cause any problems. (I tried to put
> > > > > make 4.3 on my Ubuntu 20.04 system, but ran into library compatibility problems
> > > > > so I wasn’t able to definitively confirm that it is the make version that changes the
> > > > > handling of the "#"). Unfortunately, adding the backslash before the # does *not*
> > > > > work with make v4.3. The backslash becomes part of the C source code sent to
> > > > > gcc, which barfs. I don't immediately have a suggestion on how to resolve this
> > > > > in a way that is compatible across make versions.
> > > >
> > > > Using "\043" instead of the "#" is a compatible solution that works in make
> > > > v4.2.1 and v4.3 and presumably all other versions as well.
> > >
> > > Hm... I've seen similar portability issues with "," for which we had to
> > > change it to "$(comma)" which magically worked for some reason that I am
> > > forgetting.
> > >
> > > Does "$(pound)" work? This seems to work here:
>
> Please not 'pound' - that is the uk currency symbol (not what US greengrocers
> scrawl for lb).
While I do call it the "pound sign", I can't take the credit/blame for
that name it as the variable already exists.
It's better than "hashtag" which is what my kids call it :-/
--
Josh
^ permalink raw reply
* Re: [PATCH v4 49/63] objtool/klp: Add --checksum option to generate per-function checksums
From: David Laight @ 2025-11-12 13:25 UTC (permalink / raw)
To: Michael Kelley
Cc: Josh Poimboeuf, x86@kernel.org, linux-kernel@vger.kernel.org,
Petr Mladek, Miroslav Benes, Joe Lawrence,
live-patching@vger.kernel.org, Song Liu, laokz, Jiri Kosina,
Marcos Paulo de Souza, Weinan Liu, Fazla Mehrab, Chen Zhongjin,
Puranjay Mohan, Dylan Hatch, Peter Zijlstra
In-Reply-To: <BN7PR02MB414887B3CA73281177406A5BD4CCA@BN7PR02MB4148.namprd02.prod.outlook.com>
On Wed, 12 Nov 2025 04:32:02 +0000
Michael Kelley <mhklinux@outlook.com> wrote:
> From: Josh Poimboeuf <jpoimboe@kernel.org> Sent: Tuesday, November 11, 2025 8:04 PM
> >
> > On Wed, Nov 12, 2025 at 02:26:18AM +0000, Michael Kelley wrote:
> > > > I've been able to debug this. Two problems:
> > > >
> > > > 1) On Ubuntu (both 20.04 and 24.04), /bin/sh and /usr/bin/sh are symlinks
> > > > to "dash" (not "bash"). So the "shell" command in "make" invokes dash. The
> > > > man page for dash shows that the built-in echo command accepts only -n as
> > > > an option. The -e behavior of processing "\n" and similar sequences is always
> > > > enabled. So on my Ubuntu systems, the "-e" is ignored by echo and becomes
> > > > part of the C source code sent to gcc, and of course it barfs. Dropping the -e
> > > > makes it work for me (and the \n is handled correctly), but that might not work
> > > > with other shells. Using "/bin/echo" with the -e solves the problem in a more
> > > > compatible way across different shells.
> >
> > Ah. I think we can use "printf" here.
Much better than echo - and a bultin on most shells.
> >
> > > > 2) With make v4.2.1 on my Ubuntu 20.04 system, the "#" character in the
> > > > "#include" added to the echo command is problematic. "make" seems to be
> > > > treating it as a comment character, though I'm not 100% sure of that
> > > > interpretation. Regardless, the "#" causes a syntax error in the "make" shell
> > > > command. Adding a backslash before the "#" solves that problem. On an Ubuntu
> > > > 24.04 system with make v4.3, the "#" does not cause any problems. (I tried to put
> > > > make 4.3 on my Ubuntu 20.04 system, but ran into library compatibility problems
> > > > so I wasn’t able to definitively confirm that it is the make version that changes the
> > > > handling of the "#"). Unfortunately, adding the backslash before the # does *not*
> > > > work with make v4.3. The backslash becomes part of the C source code sent to
> > > > gcc, which barfs. I don't immediately have a suggestion on how to resolve this
> > > > in a way that is compatible across make versions.
> > >
> > > Using "\043" instead of the "#" is a compatible solution that works in make
> > > v4.2.1 and v4.3 and presumably all other versions as well.
> >
> > Hm... I've seen similar portability issues with "," for which we had to
> > change it to "$(comma)" which magically worked for some reason that I am
> > forgetting.
> >
> > Does "$(pound)" work? This seems to work here:
Please not 'pound' - that is the uk currency symbol (not what US greengrocers
scrawl for lb).
David
> >
> > HAVE_XXHASH = $(shell printf "$(pound)include <xxhash.h>\nXXH3_state_t *state;int main() {}" | \
> >
>
> Yes, the above line works in my Ubuntu 20.04 and 24.04 environments.
> It properly detects the presence and absence of xxhash 0.8. Seems like a
> good solution to me.
>
> Michael
^ permalink raw reply
* RE: [PATCH v4 49/63] objtool/klp: Add --checksum option to generate per-function checksums
From: Michael Kelley @ 2025-11-12 4:32 UTC (permalink / raw)
To: Josh Poimboeuf
Cc: x86@kernel.org, linux-kernel@vger.kernel.org, Petr Mladek,
Miroslav Benes, Joe Lawrence, live-patching@vger.kernel.org,
Song Liu, laokz, Jiri Kosina, Marcos Paulo de Souza, Weinan Liu,
Fazla Mehrab, Chen Zhongjin, Puranjay Mohan, Dylan Hatch,
Peter Zijlstra
In-Reply-To: <yk3ku4ud35rsrfwzvuqnrcnwogwngqlmc3otxrnoqefb47ajm7@orl5gcxuwrme>
From: Josh Poimboeuf <jpoimboe@kernel.org> Sent: Tuesday, November 11, 2025 8:04 PM
>
> On Wed, Nov 12, 2025 at 02:26:18AM +0000, Michael Kelley wrote:
> > > I've been able to debug this. Two problems:
> > >
> > > 1) On Ubuntu (both 20.04 and 24.04), /bin/sh and /usr/bin/sh are symlinks
> > > to "dash" (not "bash"). So the "shell" command in "make" invokes dash. The
> > > man page for dash shows that the built-in echo command accepts only -n as
> > > an option. The -e behavior of processing "\n" and similar sequences is always
> > > enabled. So on my Ubuntu systems, the "-e" is ignored by echo and becomes
> > > part of the C source code sent to gcc, and of course it barfs. Dropping the -e
> > > makes it work for me (and the \n is handled correctly), but that might not work
> > > with other shells. Using "/bin/echo" with the -e solves the problem in a more
> > > compatible way across different shells.
>
> Ah. I think we can use "printf" here.
>
> > > 2) With make v4.2.1 on my Ubuntu 20.04 system, the "#" character in the
> > > "#include" added to the echo command is problematic. "make" seems to be
> > > treating it as a comment character, though I'm not 100% sure of that
> > > interpretation. Regardless, the "#" causes a syntax error in the "make" shell
> > > command. Adding a backslash before the "#" solves that problem. On an Ubuntu
> > > 24.04 system with make v4.3, the "#" does not cause any problems. (I tried to put
> > > make 4.3 on my Ubuntu 20.04 system, but ran into library compatibility problems
> > > so I wasn’t able to definitively confirm that it is the make version that changes the
> > > handling of the "#"). Unfortunately, adding the backslash before the # does *not*
> > > work with make v4.3. The backslash becomes part of the C source code sent to
> > > gcc, which barfs. I don't immediately have a suggestion on how to resolve this
> > > in a way that is compatible across make versions.
> >
> > Using "\043" instead of the "#" is a compatible solution that works in make
> > v4.2.1 and v4.3 and presumably all other versions as well.
>
> Hm... I've seen similar portability issues with "," for which we had to
> change it to "$(comma)" which magically worked for some reason that I am
> forgetting.
>
> Does "$(pound)" work? This seems to work here:
>
> HAVE_XXHASH = $(shell printf "$(pound)include <xxhash.h>\nXXH3_state_t *state;int main() {}" | \
>
Yes, the above line works in my Ubuntu 20.04 and 24.04 environments.
It properly detects the presence and absence of xxhash 0.8. Seems like a
good solution to me.
Michael
^ permalink raw reply
* Re: [PATCH v4 49/63] objtool/klp: Add --checksum option to generate per-function checksums
From: Josh Poimboeuf @ 2025-11-12 4:04 UTC (permalink / raw)
To: Michael Kelley
Cc: x86@kernel.org, linux-kernel@vger.kernel.org, Petr Mladek,
Miroslav Benes, Joe Lawrence, live-patching@vger.kernel.org,
Song Liu, laokz, Jiri Kosina, Marcos Paulo de Souza, Weinan Liu,
Fazla Mehrab, Chen Zhongjin, Puranjay Mohan, Dylan Hatch,
Peter Zijlstra
In-Reply-To: <SN6PR02MB4157F236604B6780327E6B43D4CCA@SN6PR02MB4157.namprd02.prod.outlook.com>
On Wed, Nov 12, 2025 at 02:26:18AM +0000, Michael Kelley wrote:
> > I've been able to debug this. Two problems:
> >
> > 1) On Ubuntu (both 20.04 and 24.04), /bin/sh and /usr/bin/sh are symlinks
> > to "dash" (not "bash"). So the "shell" command in "make" invokes dash. The
> > man page for dash shows that the built-in echo command accepts only -n as
> > an option. The -e behavior of processing "\n" and similar sequences is always
> > enabled. So on my Ubuntu systems, the "-e" is ignored by echo and becomes
> > part of the C source code sent to gcc, and of course it barfs. Dropping the -e
> > makes it work for me (and the \n is handled correctly), but that might not work
> > with other shells. Using "/bin/echo" with the -e solves the problem in a more
> > compatible way across different shells.
Ah. I think we can use "printf" here.
> > 2) With make v4.2.1 on my Ubuntu 20.04 system, the "#" character in the
> > "#include" added to the echo command is problematic. "make" seems to be
> > treating it as a comment character, though I'm not 100% sure of that
> > interpretation. Regardless, the "#" causes a syntax error in the "make" shell
> > command. Adding a backslash before the "#" solves that problem. On an Ubuntu
> > 24.04 system with make v4.3, the "#" does not cause any problems. (I tried to put
> > make 4.3 on my Ubuntu 20.04 system, but ran into library compatibility problems
> > so I wasn’t able to definitively confirm that it is the make version that changes the
> > handling of the "#"). Unfortunately, adding the backslash before the # does *not*
> > work with make v4.3. The backslash becomes part of the C source code sent to
> > gcc, which barfs. I don't immediately have a suggestion on how to resolve this
> > in a way that is compatible across make versions.
>
> Using "\043" instead of the "#" is a compatible solution that works in make
> v4.2.1 and v4.3 and presumably all other versions as well.
Hm... I've seen similar portability issues with "," for which we had to
change it to "$(comma)" which magically worked for some reason that I am
forgetting.
Does "$(pound)" work? This seems to work here:
HAVE_XXHASH = $(shell printf "$(pound)include <xxhash.h>\nXXH3_state_t *state;int main() {}" | \
--
Josh
^ permalink raw reply
* RE: [PATCH v4 49/63] objtool/klp: Add --checksum option to generate per-function checksums
From: Michael Kelley @ 2025-11-12 2:26 UTC (permalink / raw)
To: Michael Kelley, Josh Poimboeuf
Cc: x86@kernel.org, linux-kernel@vger.kernel.org, Petr Mladek,
Miroslav Benes, Joe Lawrence, live-patching@vger.kernel.org,
Song Liu, laokz, Jiri Kosina, Marcos Paulo de Souza, Weinan Liu,
Fazla Mehrab, Chen Zhongjin, Puranjay Mohan, Dylan Hatch,
Peter Zijlstra
In-Reply-To: <SN6PR02MB4157212C49D6A6E2AFE0CAA9D4CCA@SN6PR02MB4157.namprd02.prod.outlook.com>
From: Michael Kelley <mhklinux@outlook.com> Sent: Tuesday, November 11, 2025 5:39 PM
>
> From: Josh Poimboeuf <jpoimboe@kernel.org> Sent: Tuesday, November 11, 2025 12:09 PM
> >
> > On Wed, Nov 05, 2025 at 03:22:58PM +0000, Michael Kelley wrote:
> > > > Thanks for reporting that. I suppose something like the below would work?
> > > >
> > > > Though, maybe the missing xxhash shouldn't fail the build at all. It's
> > > > really only needed for people who are actually trying to run klp-build.
> > > > I may look at improving that.
> > >
> > > Yes, that would probably be better.
> > >
> > > >
> > > > diff --git a/tools/objtool/Makefile b/tools/objtool/Makefile
> > > > index 48928c9bebef1..8b95166b31602 100644
> > > > --- a/tools/objtool/Makefile
> > > > +++ b/tools/objtool/Makefile
> > > > @@ -12,7 +12,7 @@ ifeq ($(SRCARCH),loongarch)
> > > > endif
> > > >
> > > > ifeq ($(ARCH_HAS_KLP),y)
> > > > - HAVE_XXHASH = $(shell echo "int main() {}" | \
> > > > + HAVE_XXHASH = $(shell echo -e "#include <xxhash.h>\nXXH3_state_t *state;int main() {}" | \
> > > > $(HOSTCC) -xc - -o /dev/null -lxxhash 2> /dev/null && echo y || echo > n)
> > > > ifeq ($(HAVE_XXHASH),y)
> > > > BUILD_KLP := y
> > >
> > > Indeed this is what I had in mind for the enhanced check. But the above
> > > gets a syntax error:
> > >
> > > Makefile:15: *** unterminated call to function 'shell': missing ')'. Stop.
> > > make[4]: *** [Makefile:73: objtool] Error 2
> > >
> > > As a debugging experiment, adding only the -e option to the existing code
> > > like this shouldn't affect anything,
> > >
> > > HAVE_XXHASH = $(shell echo -e "int main() {}" | \
> > >
> > > but it causes HAVE_XXHASH to always be 'n' even if the xxhash library
> > > is present. So the -e option is somehow fouling things up.
> > >
> > > Running the equivalent interactively at a 'bash' prompt works as expected.
> > > And your proposed patch works correctly in an interactive bash. So
> > > something weird is happening in the context of make's shell function,
> > > and I haven't been able to figure out what it is.
> > >
> > > Do you get the same failures? Or is this some kind of problem with
> > > my environment? I've got GNU make version 4.2.1.
> >
> > That's weird, it builds fine for me. I have GNU make 4.4.1.
>
> I've been able to debug this. Two problems:
>
> 1) On Ubuntu (both 20.04 and 24.04), /bin/sh and /usr/bin/sh are symlinks
> to "dash" (not "bash"). So the "shell" command in "make" invokes dash. The
> man page for dash shows that the built-in echo command accepts only -n as
> an option. The -e behavior of processing "\n" and similar sequences is always
> enabled. So on my Ubuntu systems, the "-e" is ignored by echo and becomes
> part of the C source code sent to gcc, and of course it barfs. Dropping the -e
> makes it work for me (and the \n is handled correctly), but that might not work
> with other shells. Using "/bin/echo" with the -e solves the problem in a more
> compatible way across different shells.
>
> 2) With make v4.2.1 on my Ubuntu 20.04 system, the "#" character in the
> "#include" added to the echo command is problematic. "make" seems to be
> treating it as a comment character, though I'm not 100% sure of that
> interpretation. Regardless, the "#" causes a syntax error in the "make" shell
> command. Adding a backslash before the "#" solves that problem. On an Ubuntu
> 24.04 system with make v4.3, the "#" does not cause any problems. (I tried to put
> make 4.3 on my Ubuntu 20.04 system, but ran into library compatibility problems
> so I wasn’t able to definitively confirm that it is the make version that changes the
> handling of the "#"). Unfortunately, adding the backslash before the # does *not*
> work with make v4.3. The backslash becomes part of the C source code sent to
> gcc, which barfs. I don't immediately have a suggestion on how to resolve this
> in a way that is compatible across make versions.
Using "\043" instead of the "#" is a compatible solution that works in make
v4.2.1 and v4.3 and presumably all other versions as well.
Michael
^ permalink raw reply
* RE: [PATCH v4 49/63] objtool/klp: Add --checksum option to generate per-function checksums
From: Michael Kelley @ 2025-11-12 1:39 UTC (permalink / raw)
To: Josh Poimboeuf
Cc: x86@kernel.org, linux-kernel@vger.kernel.org, Petr Mladek,
Miroslav Benes, Joe Lawrence, live-patching@vger.kernel.org,
Song Liu, laokz, Jiri Kosina, Marcos Paulo de Souza, Weinan Liu,
Fazla Mehrab, Chen Zhongjin, Puranjay Mohan, Dylan Hatch,
Peter Zijlstra
In-Reply-To: <6msqczigbcypeclqlgzgtqew7pddmuu6xxrjli2rna22hul5j4@rc6tyxme34rc>
From: Josh Poimboeuf <jpoimboe@kernel.org> Sent: Tuesday, November 11, 2025 12:09 PM
>
> On Wed, Nov 05, 2025 at 03:22:58PM +0000, Michael Kelley wrote:
> > > Thanks for reporting that. I suppose something like the below would work?
> > >
> > > Though, maybe the missing xxhash shouldn't fail the build at all. It's
> > > really only needed for people who are actually trying to run klp-build.
> > > I may look at improving that.
> >
> > Yes, that would probably be better.
> >
> > >
> > > diff --git a/tools/objtool/Makefile b/tools/objtool/Makefile
> > > index 48928c9bebef1..8b95166b31602 100644
> > > --- a/tools/objtool/Makefile
> > > +++ b/tools/objtool/Makefile
> > > @@ -12,7 +12,7 @@ ifeq ($(SRCARCH),loongarch)
> > > endif
> > >
> > > ifeq ($(ARCH_HAS_KLP),y)
> > > - HAVE_XXHASH = $(shell echo "int main() {}" | \
> > > + HAVE_XXHASH = $(shell echo -e "#include <xxhash.h>\nXXH3_state_t *state;int main() {}" | \
> > > $(HOSTCC) -xc - -o /dev/null -lxxhash 2> /dev/null && echo y || echo n)
> > > ifeq ($(HAVE_XXHASH),y)
> > > BUILD_KLP := y
> >
> > Indeed this is what I had in mind for the enhanced check. But the above
> > gets a syntax error:
> >
> > Makefile:15: *** unterminated call to function 'shell': missing ')'. Stop.
> > make[4]: *** [Makefile:73: objtool] Error 2
> >
> > As a debugging experiment, adding only the -e option to the existing code
> > like this shouldn't affect anything,
> >
> > HAVE_XXHASH = $(shell echo -e "int main() {}" | \
> >
> > but it causes HAVE_XXHASH to always be 'n' even if the xxhash library
> > is present. So the -e option is somehow fouling things up.
> >
> > Running the equivalent interactively at a 'bash' prompt works as expected.
> > And your proposed patch works correctly in an interactive bash. So
> > something weird is happening in the context of make's shell function,
> > and I haven't been able to figure out what it is.
> >
> > Do you get the same failures? Or is this some kind of problem with
> > my environment? I've got GNU make version 4.2.1.
>
> That's weird, it builds fine for me. I have GNU make 4.4.1.
I've been able to debug this. Two problems:
1) On Ubuntu (both 20.04 and 24.04), /bin/sh and /usr/bin/sh are symlinks
to "dash" (not "bash"). So the "shell" command in "make" invokes dash. The
man page for dash shows that the built-in echo command accepts only -n as
an option. The -e behavior of processing "\n" and similar sequences is always
enabled. So on my Ubuntu systems, the "-e" is ignored by echo and becomes
part of the C source code sent to gcc, and of course it barfs. Dropping the -e
makes it work for me (and the \n is handled correctly), but that might not work
with other shells. Using "/bin/echo" with the -e solves the problem in a more
compatible way across different shells.
2) With make v4.2.1 on my Ubuntu 20.04 system, the "#" character in the
"#include" added to the echo command is problematic. "make" seems to be
treating it as a comment character, though I'm not 100% sure of that
interpretation. Regardless, the "#" causes a syntax error in the "make" shell
command. Adding a backslash before the "#" solves that problem. On an Ubuntu
24.04 system with make v4.3, the "#" does not cause any problems. (I tried to put
make 4.3 on my Ubuntu 20.04 system, but ran into library compatibility problems
so I wasn’t able to definitively confirm that it is the make version that changes the
handling of the "#"). Unfortunately, adding the backslash before the # does *not*
work with make v4.3. The backslash becomes part of the C source code sent to
gcc, which barfs. I don't immediately have a suggestion on how to resolve this
in a way that is compatible across make versions.
What a mess.
Michael
^ permalink raw reply
* Re: [PATCH v4 49/63] objtool/klp: Add --checksum option to generate per-function checksums
From: Josh Poimboeuf @ 2025-11-11 20:09 UTC (permalink / raw)
To: Michael Kelley
Cc: x86@kernel.org, linux-kernel@vger.kernel.org, Petr Mladek,
Miroslav Benes, Joe Lawrence, live-patching@vger.kernel.org,
Song Liu, laokz, Jiri Kosina, Marcos Paulo de Souza, Weinan Liu,
Fazla Mehrab, Chen Zhongjin, Puranjay Mohan, Dylan Hatch,
Peter Zijlstra
In-Reply-To: <SN6PR02MB41574AD398AD3DE26DB3D23BD4C5A@SN6PR02MB4157.namprd02.prod.outlook.com>
On Wed, Nov 05, 2025 at 03:22:58PM +0000, Michael Kelley wrote:
> > Thanks for reporting that. I suppose something like the below would work?
> >
> > Though, maybe the missing xxhash shouldn't fail the build at all. It's
> > really only needed for people who are actually trying to run klp-build.
> > I may look at improving that.
>
> Yes, that would probably be better.
>
> >
> > diff --git a/tools/objtool/Makefile b/tools/objtool/Makefile
> > index 48928c9bebef1..8b95166b31602 100644
> > --- a/tools/objtool/Makefile
> > +++ b/tools/objtool/Makefile
> > @@ -12,7 +12,7 @@ ifeq ($(SRCARCH),loongarch)
> > endif
> >
> > ifeq ($(ARCH_HAS_KLP),y)
> > - HAVE_XXHASH = $(shell echo "int main() {}" | \
> > + HAVE_XXHASH = $(shell echo -e "#include <xxhash.h>\nXXH3_state_t *state;int main() {}" | \
> > $(HOSTCC) -xc - -o /dev/null -lxxhash 2> /dev/null && echo y || echo n)
> > ifeq ($(HAVE_XXHASH),y)
> > BUILD_KLP := y
>
> Indeed this is what I had in mind for the enhanced check. But the above
> gets a syntax error:
>
> Makefile:15: *** unterminated call to function 'shell': missing ')'. Stop.
> make[4]: *** [Makefile:73: objtool] Error 2
>
> As a debugging experiment, adding only the -e option to the existing code
> like this shouldn't affect anything,
>
> HAVE_XXHASH = $(shell echo -e "int main() {}" | \
>
> but it causes HAVE_XXHASH to always be 'n' even if the xxhash library
> is present. So the -e option is somehow fouling things up.
>
> Running the equivalent interactively at a 'bash' prompt works as expected.
> And your proposed patch works correctly in an interactive bash. So
> something weird is happening in the context of make's shell function,
> and I haven't been able to figure out what it is.
>
> Do you get the same failures? Or is this some kind of problem with
> my environment? I've got GNU make version 4.2.1.
That's weird, it builds fine for me. I have GNU make 4.4.1.
--
Josh
^ permalink raw reply
* RE: [PATCH v4 49/63] objtool/klp: Add --checksum option to generate per-function checksums
From: Michael Kelley @ 2025-11-05 15:22 UTC (permalink / raw)
To: Josh Poimboeuf
Cc: x86@kernel.org, linux-kernel@vger.kernel.org, Petr Mladek,
Miroslav Benes, Joe Lawrence, live-patching@vger.kernel.org,
Song Liu, laokz, Jiri Kosina, Marcos Paulo de Souza, Weinan Liu,
Fazla Mehrab, Chen Zhongjin, Puranjay Mohan, Dylan Hatch,
Peter Zijlstra
In-Reply-To: <5an6r3jzuifkm2b7scmxv4u3suygr77apgue6zneelowbqyjzr@5g6mbczbyk5e>
From: Josh Poimboeuf <jpoimboe@kernel.org> Sent: Monday, October 27, 2025 3:22 PM
>
Sorry for the delay in my follow-up. I've been travelling the past 10 days.
> On Mon, Oct 27, 2025 at 01:19:10AM +0000, Michael Kelley wrote:
> > It turns out that Ubuntu 20.04 installed the 0.7.3-1 version of libxxhash. But from a
> > quick look at the README on the xxhash github site, XXH3 is first supported by the
> > 0.8.0 version, so the compile error probably makes sense. I found a PPA that offers
> > the 0.8.3 version of xxhash for Ubuntu 20.04, and that solved the problem.
> >
> > So the Makefile steps above that figure out if xxhash is present probably aren't
> > sufficient, as the version of xxhash matters. And the "--checksum not supported"
> > error message should be more specific about the required version.
> >
> > I reproduced the behavior on two different Ubuntu 20.04 systems, but
> > someone who knows this xxhash stuff better than I do should confirm
> > my conclusions. Maybe the way to fix the check for the presence of xxhash is
> > to augment the inline test program to include a reference to XXH3_state, but
> > I haven't tried to put together a patch to do that, pending any further discussion
> > or ideas.
>
> Thanks for reporting that. I suppose something like the below would work?
>
> Though, maybe the missing xxhash shouldn't fail the build at all. It's
> really only needed for people who are actually trying to run klp-build.
> I may look at improving that.
Yes, that would probably be better.
>
> diff --git a/tools/objtool/Makefile b/tools/objtool/Makefile
> index 48928c9bebef1..8b95166b31602 100644
> --- a/tools/objtool/Makefile
> +++ b/tools/objtool/Makefile
> @@ -12,7 +12,7 @@ ifeq ($(SRCARCH),loongarch)
> endif
>
> ifeq ($(ARCH_HAS_KLP),y)
> - HAVE_XXHASH = $(shell echo "int main() {}" | \
> + HAVE_XXHASH = $(shell echo -e "#include <xxhash.h>\nXXH3_state_t *state;int main() {}" | \
> $(HOSTCC) -xc - -o /dev/null -lxxhash 2> /dev/null && echo y || echo n)
> ifeq ($(HAVE_XXHASH),y)
> BUILD_KLP := y
Indeed this is what I had in mind for the enhanced check. But the above
gets a syntax error:
Makefile:15: *** unterminated call to function 'shell': missing ')'. Stop.
make[4]: *** [Makefile:73: objtool] Error 2
As a debugging experiment, adding only the -e option to the existing code
like this shouldn't affect anything,
HAVE_XXHASH = $(shell echo -e "int main() {}" | \
but it causes HAVE_XXHASH to always be 'n' even if the xxhash library
is present. So the -e option is somehow fouling things up.
Running the equivalent interactively at a 'bash' prompt works as expected.
And your proposed patch works correctly in an interactive bash. So
something weird is happening in the context of make's shell function,
and I haven't been able to figure out what it is.
Do you get the same failures? Or is this some kind of problem with
my environment? I've got GNU make version 4.2.1.
Michael
> diff --git a/tools/objtool/builtin-check.c b/tools/objtool/builtin-check.c
> index 1e1ea8396eb3a..aab7fa9c7e00a 100644
> --- a/tools/objtool/builtin-check.c
> +++ b/tools/objtool/builtin-check.c
> @@ -164,7 +164,7 @@ static bool opts_valid(void)
>
> #ifndef BUILD_KLP
> if (opts.checksum) {
> - ERROR("--checksum not supported; install xxhash-devel/libxxhash-dev and recompile");
> + ERROR("--checksum not supported; install xxhash-devel/libxxhash-dev (version >= 0.8) and recompile");
> return false;
> }
> #endif
^ permalink raw reply
* Re: [PATCH v4 bpf 0/3] Fix ftrace for livepatch + BPF fexit programs
From: patchwork-bot+netdevbpf @ 2025-11-04 1:40 UTC (permalink / raw)
To: Song Liu
Cc: bpf, linux-trace-kernel, live-patching, ast, daniel, andrii,
rostedt, andrey.grodzovsky, mhiramat, kernel-team, olsajiri
In-Reply-To: <20251027175023.1521602-1-song@kernel.org>
Hello:
This series was applied to bpf/bpf.git (master)
by Alexei Starovoitov <ast@kernel.org>:
On Mon, 27 Oct 2025 10:50:20 -0700 you wrote:
> livepatch and BPF trampoline are two special users of ftrace. livepatch
> uses ftrace with IPMODIFY flag and BPF trampoline uses ftrace direct
> functions. When livepatch and BPF trampoline with fexit programs attach to
> the same kernel function, BPF trampoline needs to call into the patched
> version of the kernel function.
>
> 1/3 and 2/3 of this patchset fix two issues with livepatch + fexit cases,
> one in the register_ftrace_direct path, the other in the
> modify_ftrace_direct path.
>
> [...]
Here is the summary with links:
- [v4,bpf,1/3] ftrace: Fix BPF fexit with livepatch
https://git.kernel.org/bpf/bpf/c/56b3c85e153b
- [v4,bpf,2/3] ftrace: bpf: Fix IPMODIFY + DIRECT in modify_ftrace_direct()
https://git.kernel.org/bpf/bpf/c/3e9a18e1c3e9
- [v4,bpf,3/3] selftests/bpf: Add tests for livepatch + bpf trampoline
https://git.kernel.org/bpf/bpf/c/62d2d0a33839
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply
* Re: [PATCH v4 bpf 0/3] Fix ftrace for livepatch + BPF fexit programs
From: Steven Rostedt @ 2025-11-02 23:39 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Song Liu, bpf, linux-trace-kernel, live-patching,
Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Andrey Grodzovsky, Masami Hiramatsu, Kernel Team, Jiri Olsa
In-Reply-To: <20251101091116.763638e5@batman.local.home>
On Sat, 1 Nov 2025 09:11:16 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:
> On Fri, 31 Oct 2025 17:19:54 -0700
> Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote:
>
> > can you apply the fixes or should I take them ?
> > If so, pls ack.
>
> Let me run them through my full test suite. It takes up to 13 hours to
> run. Then I'll give a ack for you to take them.
>
They passed my tests.
Acked-by: Steven Rostedt (Google) <rostedt@goodmis.org>
Feel free to send them through the BPF tree.
-- Steve
^ permalink raw reply
* Re: [PATCH v4 bpf 0/3] Fix ftrace for livepatch + BPF fexit programs
From: Steven Rostedt @ 2025-11-01 13:11 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Song Liu, bpf, linux-trace-kernel, live-patching,
Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Andrey Grodzovsky, Masami Hiramatsu, Kernel Team, Jiri Olsa
In-Reply-To: <CAADnVQ+azh4iUmq4_RHYatphAaZUGsW0Zo8=vGOT1_fv-UYOaA@mail.gmail.com>
On Fri, 31 Oct 2025 17:19:54 -0700
Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote:
> can you apply the fixes or should I take them ?
> If so, pls ack.
Let me run them through my full test suite. It takes up to 13 hours to
run. Then I'll give a ack for you to take them.
-- Steve
^ permalink raw reply
* Re: [PATCH v4 bpf 0/3] Fix ftrace for livepatch + BPF fexit programs
From: Alexei Starovoitov @ 2025-11-01 0:19 UTC (permalink / raw)
To: Song Liu
Cc: bpf, linux-trace-kernel, live-patching, Alexei Starovoitov,
Daniel Borkmann, Andrii Nakryiko, Steven Rostedt,
Andrey Grodzovsky, Masami Hiramatsu, Kernel Team, Jiri Olsa
In-Reply-To: <20251027175023.1521602-1-song@kernel.org>
On Mon, Oct 27, 2025 at 10:50 AM Song Liu <song@kernel.org> wrote:
>
> livepatch and BPF trampoline are two special users of ftrace. livepatch
> uses ftrace with IPMODIFY flag and BPF trampoline uses ftrace direct
> functions. When livepatch and BPF trampoline with fexit programs attach to
> the same kernel function, BPF trampoline needs to call into the patched
> version of the kernel function.
>
> 1/3 and 2/3 of this patchset fix two issues with livepatch + fexit cases,
> one in the register_ftrace_direct path, the other in the
> modify_ftrace_direct path.
>
> 3/3 adds selftests for both cases.
>
> ---
>
> Changes v3 => v4:
> 1. Add helper reset_direct. (Steven)
> 2. Add Reviewed-by from Jiri.
> 3. Fix minor typo in comments.
Steven,
can you apply the fixes or should I take them ?
If so, pls ack.
^ permalink raw reply
* Re: [PATCH v4 49/63] objtool/klp: Add --checksum option to generate per-function checksums
From: Josh Poimboeuf @ 2025-10-27 22:22 UTC (permalink / raw)
To: Michael Kelley
Cc: x86@kernel.org, linux-kernel@vger.kernel.org, Petr Mladek,
Miroslav Benes, Joe Lawrence, live-patching@vger.kernel.org,
Song Liu, laokz, Jiri Kosina, Marcos Paulo de Souza, Weinan Liu,
Fazla Mehrab, Chen Zhongjin, Puranjay Mohan, Dylan Hatch,
Peter Zijlstra
In-Reply-To: <SN6PR02MB41579B83CD295C9FEE40EED6D4FCA@SN6PR02MB4157.namprd02.prod.outlook.com>
On Mon, Oct 27, 2025 at 01:19:10AM +0000, Michael Kelley wrote:
> It turns out that Ubuntu 20.04 installed the 0.7.3-1 version of libxxhash. But from a
> quick look at the README on the xxhash github site, XXH3 is first supported by the
> 0.8.0 version, so the compile error probably makes sense. I found a PPA that offers
> the 0.8.3 version of xxhash for Ubuntu 20.04, and that solved the problem.
>
> So the Makefile steps above that figure out if xxhash is present probably aren't
> sufficient, as the version of xxhash matters. And the "--checksum not supported"
> error message should be more specific about the required version.
>
> I reproduced the behavior on two different Ubuntu 20.04 systems, but
> someone who knows this xxhash stuff better than I do should confirm
> my conclusions. Maybe the way to fix the check for the presence of xxhash is
> to augment the inline test program to include a reference to XXH3_state, but
> I haven't tried to put together a patch to do that, pending any further discussion
> or ideas.
Thanks for reporting that. I suppose something like the below would work?
Though, maybe the missing xxhash shouldn't fail the build at all. It's
really only needed for people who are actually trying to run klp-build.
I may look at improving that.
diff --git a/tools/objtool/Makefile b/tools/objtool/Makefile
index 48928c9bebef1..8b95166b31602 100644
--- a/tools/objtool/Makefile
+++ b/tools/objtool/Makefile
@@ -12,7 +12,7 @@ ifeq ($(SRCARCH),loongarch)
endif
ifeq ($(ARCH_HAS_KLP),y)
- HAVE_XXHASH = $(shell echo "int main() {}" | \
+ HAVE_XXHASH = $(shell echo -e "#include <xxhash.h>\nXXH3_state_t *state;int main() {}" | \
$(HOSTCC) -xc - -o /dev/null -lxxhash 2> /dev/null && echo y || echo n)
ifeq ($(HAVE_XXHASH),y)
BUILD_KLP := y
diff --git a/tools/objtool/builtin-check.c b/tools/objtool/builtin-check.c
index 1e1ea8396eb3a..aab7fa9c7e00a 100644
--- a/tools/objtool/builtin-check.c
+++ b/tools/objtool/builtin-check.c
@@ -164,7 +164,7 @@ static bool opts_valid(void)
#ifndef BUILD_KLP
if (opts.checksum) {
- ERROR("--checksum not supported; install xxhash-devel/libxxhash-dev and recompile");
+ ERROR("--checksum not supported; install xxhash-devel/libxxhash-dev (version >= 0.8) and recompile");
return false;
}
#endif
^ permalink raw reply related
* [PATCH v4 bpf 3/3] selftests/bpf: Add tests for livepatch + bpf trampoline
From: Song Liu @ 2025-10-27 17:50 UTC (permalink / raw)
To: bpf, linux-trace-kernel, live-patching
Cc: ast, daniel, andrii, rostedt, andrey.grodzovsky, mhiramat,
kernel-team, olsajiri, Song Liu, Jiri Olsa
In-Reply-To: <20251027175023.1521602-1-song@kernel.org>
Both livepatch and BPF trampoline use ftrace. Special attention is needed
when livepatch and fexit program touch the same function at the same
time, because livepatch updates a kernel function and the BPF trampoline
need to call into the right version of the kernel function.
Use samples/livepatch/livepatch-sample.ko for the test.
The test covers two cases:
1) When a fentry program is loaded first. This exercises the
modify_ftrace_direct code path.
2) When a fentry program is loaded first. This exercises the
register_ftrace_direct code path.
Signed-off-by: Song Liu <song@kernel.org>
Reviewed-by: Jiri Olsa <jolsa@kernel.org>
---
tools/testing/selftests/bpf/config | 3 +
.../bpf/prog_tests/livepatch_trampoline.c | 107 ++++++++++++++++++
.../bpf/progs/livepatch_trampoline.c | 30 +++++
3 files changed, 140 insertions(+)
create mode 100644 tools/testing/selftests/bpf/prog_tests/livepatch_trampoline.c
create mode 100644 tools/testing/selftests/bpf/progs/livepatch_trampoline.c
diff --git a/tools/testing/selftests/bpf/config b/tools/testing/selftests/bpf/config
index 70b28c1e653e..f2a2fd236ca8 100644
--- a/tools/testing/selftests/bpf/config
+++ b/tools/testing/selftests/bpf/config
@@ -50,6 +50,7 @@ CONFIG_IPV6_SIT=y
CONFIG_IPV6_TUNNEL=y
CONFIG_KEYS=y
CONFIG_LIRC=y
+CONFIG_LIVEPATCH=y
CONFIG_LWTUNNEL=y
CONFIG_MODULE_SIG=y
CONFIG_MODULE_SRCVERSION_ALL=y
@@ -111,6 +112,8 @@ CONFIG_IP6_NF_FILTER=y
CONFIG_NF_NAT=y
CONFIG_PACKET=y
CONFIG_RC_CORE=y
+CONFIG_SAMPLES=y
+CONFIG_SAMPLE_LIVEPATCH=m
CONFIG_SECURITY=y
CONFIG_SECURITYFS=y
CONFIG_SYN_COOKIES=y
diff --git a/tools/testing/selftests/bpf/prog_tests/livepatch_trampoline.c b/tools/testing/selftests/bpf/prog_tests/livepatch_trampoline.c
new file mode 100644
index 000000000000..72aa5376c30e
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/livepatch_trampoline.c
@@ -0,0 +1,107 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2025 Meta Platforms, Inc. and affiliates. */
+
+#include <test_progs.h>
+#include "testing_helpers.h"
+#include "livepatch_trampoline.skel.h"
+
+static int load_livepatch(void)
+{
+ char path[4096];
+
+ /* CI will set KBUILD_OUTPUT */
+ snprintf(path, sizeof(path), "%s/samples/livepatch/livepatch-sample.ko",
+ getenv("KBUILD_OUTPUT") ? : "../../../..");
+
+ return load_module(path, env_verbosity > VERBOSE_NONE);
+}
+
+static void unload_livepatch(void)
+{
+ /* Disable the livepatch before unloading the module */
+ system("echo 0 > /sys/kernel/livepatch/livepatch_sample/enabled");
+
+ unload_module("livepatch_sample", env_verbosity > VERBOSE_NONE);
+}
+
+static void read_proc_cmdline(void)
+{
+ char buf[4096];
+ int fd, ret;
+
+ fd = open("/proc/cmdline", O_RDONLY);
+ if (!ASSERT_OK_FD(fd, "open /proc/cmdline"))
+ return;
+
+ ret = read(fd, buf, sizeof(buf));
+ if (!ASSERT_GT(ret, 0, "read /proc/cmdline"))
+ goto out;
+
+ ASSERT_OK(strncmp(buf, "this has been live patched", 26), "strncmp");
+
+out:
+ close(fd);
+}
+
+static void __test_livepatch_trampoline(bool fexit_first)
+{
+ struct livepatch_trampoline *skel = NULL;
+ int err;
+
+ skel = livepatch_trampoline__open_and_load();
+ if (!ASSERT_OK_PTR(skel, "skel_open_and_load"))
+ goto out;
+
+ skel->bss->my_pid = getpid();
+
+ if (!fexit_first) {
+ /* fentry program is loaded first by default */
+ err = livepatch_trampoline__attach(skel);
+ if (!ASSERT_OK(err, "skel_attach"))
+ goto out;
+ } else {
+ /* Manually load fexit program first. */
+ skel->links.fexit_cmdline = bpf_program__attach(skel->progs.fexit_cmdline);
+ if (!ASSERT_OK_PTR(skel->links.fexit_cmdline, "attach_fexit"))
+ goto out;
+
+ skel->links.fentry_cmdline = bpf_program__attach(skel->progs.fentry_cmdline);
+ if (!ASSERT_OK_PTR(skel->links.fentry_cmdline, "attach_fentry"))
+ goto out;
+ }
+
+ read_proc_cmdline();
+
+ ASSERT_EQ(skel->bss->fentry_hit, 1, "fentry_hit");
+ ASSERT_EQ(skel->bss->fexit_hit, 1, "fexit_hit");
+out:
+ livepatch_trampoline__destroy(skel);
+}
+
+void test_livepatch_trampoline(void)
+{
+ int retry_cnt = 0;
+
+retry:
+ if (load_livepatch()) {
+ if (retry_cnt) {
+ ASSERT_OK(1, "load_livepatch");
+ goto out;
+ }
+ /*
+ * Something else (previous run of the same test?) loaded
+ * the KLP module. Unload the KLP module and retry.
+ */
+ unload_livepatch();
+ retry_cnt++;
+ goto retry;
+ }
+
+ if (test__start_subtest("fentry_first"))
+ __test_livepatch_trampoline(false);
+
+ if (test__start_subtest("fexit_first"))
+ __test_livepatch_trampoline(true);
+out:
+ unload_livepatch();
+}
diff --git a/tools/testing/selftests/bpf/progs/livepatch_trampoline.c b/tools/testing/selftests/bpf/progs/livepatch_trampoline.c
new file mode 100644
index 000000000000..15579d5bcd91
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/livepatch_trampoline.c
@@ -0,0 +1,30 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2025 Meta Platforms, Inc. and affiliates. */
+
+#include <linux/bpf.h>
+#include <bpf/bpf_helpers.h>
+#include <bpf/bpf_tracing.h>
+
+int fentry_hit;
+int fexit_hit;
+int my_pid;
+
+SEC("fentry/cmdline_proc_show")
+int BPF_PROG(fentry_cmdline)
+{
+ if (my_pid != (bpf_get_current_pid_tgid() >> 32))
+ return 0;
+
+ fentry_hit = 1;
+ return 0;
+}
+
+SEC("fexit/cmdline_proc_show")
+int BPF_PROG(fexit_cmdline)
+{
+ if (my_pid != (bpf_get_current_pid_tgid() >> 32))
+ return 0;
+
+ fexit_hit = 1;
+ return 0;
+}
--
2.47.3
^ permalink raw reply related
* [PATCH v4 bpf 2/3] ftrace: bpf: Fix IPMODIFY + DIRECT in modify_ftrace_direct()
From: Song Liu @ 2025-10-27 17:50 UTC (permalink / raw)
To: bpf, linux-trace-kernel, live-patching
Cc: ast, daniel, andrii, rostedt, andrey.grodzovsky, mhiramat,
kernel-team, olsajiri, Song Liu, Jiri Olsa
In-Reply-To: <20251027175023.1521602-1-song@kernel.org>
ftrace_hash_ipmodify_enable() checks IPMODIFY and DIRECT ftrace_ops on
the same kernel function. When needed, ftrace_hash_ipmodify_enable()
calls ops->ops_func() to prepare the direct ftrace (BPF trampoline) to
share the same function as the IPMODIFY ftrace (livepatch).
ftrace_hash_ipmodify_enable() is called in register_ftrace_direct() path,
but not called in modify_ftrace_direct() path. As a result, the following
operations will break livepatch:
1. Load livepatch to a kernel function;
2. Attach fentry program to the kernel function;
3. Attach fexit program to the kernel function.
After 3, the kernel function being used will not be the livepatched
version, but the original version.
Fix this by adding __ftrace_hash_update_ipmodify() to
__modify_ftrace_direct() and adjust some logic around the call.
Signed-off-by: Song Liu <song@kernel.org>
Reviewed-by: Jiri Olsa <jolsa@kernel.org>
---
kernel/trace/ftrace.c | 40 +++++++++++++++++++++++++++++++---------
1 file changed, 31 insertions(+), 9 deletions(-)
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index cbeb7e833131..59cfacb8a5bb 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -1971,7 +1971,8 @@ static void ftrace_hash_rec_enable_modify(struct ftrace_ops *ops)
*/
static int __ftrace_hash_update_ipmodify(struct ftrace_ops *ops,
struct ftrace_hash *old_hash,
- struct ftrace_hash *new_hash)
+ struct ftrace_hash *new_hash,
+ bool update_target)
{
struct ftrace_page *pg;
struct dyn_ftrace *rec, *end = NULL;
@@ -2006,10 +2007,13 @@ static int __ftrace_hash_update_ipmodify(struct ftrace_ops *ops,
if (rec->flags & FTRACE_FL_DISABLED)
continue;
- /* We need to update only differences of filter_hash */
+ /*
+ * Unless we are updating the target of a direct function,
+ * we only need to update differences of filter_hash
+ */
in_old = !!ftrace_lookup_ip(old_hash, rec->ip);
in_new = !!ftrace_lookup_ip(new_hash, rec->ip);
- if (in_old == in_new)
+ if (!update_target && (in_old == in_new))
continue;
if (in_new) {
@@ -2020,7 +2024,16 @@ static int __ftrace_hash_update_ipmodify(struct ftrace_ops *ops,
if (is_ipmodify)
goto rollback;
- FTRACE_WARN_ON(rec->flags & FTRACE_FL_DIRECT);
+ /*
+ * If this is called by __modify_ftrace_direct()
+ * then it is only changing where the direct
+ * pointer is jumping to, and the record already
+ * points to a direct trampoline. If it isn't,
+ * then it is a bug to update ipmodify on a direct
+ * caller.
+ */
+ FTRACE_WARN_ON(!update_target &&
+ (rec->flags & FTRACE_FL_DIRECT));
/*
* Another ops with IPMODIFY is already
@@ -2076,7 +2089,7 @@ static int ftrace_hash_ipmodify_enable(struct ftrace_ops *ops)
if (ftrace_hash_empty(hash))
hash = NULL;
- return __ftrace_hash_update_ipmodify(ops, EMPTY_HASH, hash);
+ return __ftrace_hash_update_ipmodify(ops, EMPTY_HASH, hash, false);
}
/* Disabling always succeeds */
@@ -2087,7 +2100,7 @@ static void ftrace_hash_ipmodify_disable(struct ftrace_ops *ops)
if (ftrace_hash_empty(hash))
hash = NULL;
- __ftrace_hash_update_ipmodify(ops, hash, EMPTY_HASH);
+ __ftrace_hash_update_ipmodify(ops, hash, EMPTY_HASH, false);
}
static int ftrace_hash_ipmodify_update(struct ftrace_ops *ops,
@@ -2101,7 +2114,7 @@ static int ftrace_hash_ipmodify_update(struct ftrace_ops *ops,
if (ftrace_hash_empty(new_hash))
new_hash = NULL;
- return __ftrace_hash_update_ipmodify(ops, old_hash, new_hash);
+ return __ftrace_hash_update_ipmodify(ops, old_hash, new_hash, false);
}
static void print_ip_ins(const char *fmt, const unsigned char *p)
@@ -6114,7 +6127,7 @@ EXPORT_SYMBOL_GPL(unregister_ftrace_direct);
static int
__modify_ftrace_direct(struct ftrace_ops *ops, unsigned long addr)
{
- struct ftrace_hash *hash;
+ struct ftrace_hash *hash = ops->func_hash->filter_hash;
struct ftrace_func_entry *entry, *iter;
static struct ftrace_ops tmp_ops = {
.func = ftrace_stub,
@@ -6134,13 +6147,21 @@ __modify_ftrace_direct(struct ftrace_ops *ops, unsigned long addr)
if (err)
return err;
+ /*
+ * Call __ftrace_hash_update_ipmodify() here, so that we can call
+ * ops->ops_func for the ops. This is needed because the above
+ * register_ftrace_function_nolock() worked on tmp_ops.
+ */
+ err = __ftrace_hash_update_ipmodify(ops, hash, hash, true);
+ if (err)
+ goto out;
+
/*
* Now the ftrace_ops_list_func() is called to do the direct callers.
* We can safely change the direct functions attached to each entry.
*/
mutex_lock(&ftrace_lock);
- hash = ops->func_hash->filter_hash;
size = 1 << hash->size_bits;
for (i = 0; i < size; i++) {
hlist_for_each_entry(iter, &hash->buckets[i], hlist) {
@@ -6155,6 +6176,7 @@ __modify_ftrace_direct(struct ftrace_ops *ops, unsigned long addr)
mutex_unlock(&ftrace_lock);
+out:
/* Removing the tmp_ops will add the updated direct callers to the functions */
unregister_ftrace_function(&tmp_ops);
--
2.47.3
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox