Re: [PATCH 3/8] selftests: livepatch: test-kprobe: Check if kprobes can work with livepatches

[Marcos Paulo de Souza <mpdesouza@suse.com>]

On Fri, 2026-03-20 at 12:33 +0100, Petr Mladek wrote:
> On Thu 2026-03-19 11:35:16, Marcos Paulo de Souza wrote:
> > On Mon, 2026-03-16 at 16:38 -0400, Joe Lawrence wrote:
> > > On Fri, Mar 13, 2026 at 05:58:34PM -0300, Marcos Paulo de Souza
> > > wrote:
> > > > Running the upstream selftests on older kernels can presente
> > > > some
> > > > issues
> > > > regarding features being not present. One of such issues if the
> > > > missing
> > > > capability of having both kprobes and livepatches on the same
> > > > function.
> > > > 
> > > 
> > > nit picking, but slightly reworded for clarity and spelling:
> > > 
> > > Running upstream selftests on older kernels can be problematic
> > > when
> > > features or fixes from newer versions are not present. For
> > > example,
> > > older kernels may lack the capability to support kprobes and
> > > livepatches
> > > on the same function simultaneously.
> > 
> > Much better, I'll pick your description for v2.
> > 
> > > 
> > > > The support was introduced in commit 0bc11ed5ab60c
> > > > ("kprobes: Allow kprobes coexist with livepatch"), which means
> > > > that
> > > > older
> > > > kernels may lack this change.
> > > > 
> > > > The lack of this feature can be checked when a kprobe without a
> > > > post_handler is loaded and checking that the enabled_function's
> > > > file
> > > > shows the flag "I". A kernel with the proper support for
> > > > kprobes
> > > > and
> > > > livepatches would presente the flag only when a post_handler is
> > > 
> > > nit: s/presente/present
> > 
> > > > --- a/tools/testing/selftests/livepatch/test-kprobe.sh
> > > > +++ b/tools/testing/selftests/livepatch/test-kprobe.sh
> > > > @@ -16,30 +16,19 @@ setup_config
> > > >  # when it uses a post_handler since only one IPMODIFY maybe be
> > > > registered
> > > >  # to any given function at a time.
> > > >  
> > > > -start_test "livepatch interaction with kprobed function with
> > > > post_handler"
> > > > -
> > > > -echo 1 > "$SYSFS_KPROBES_DIR/enabled"
> > > > -
> > > > -load_mod $MOD_KPROBE has_post_handler=1
> > > > -load_failing_mod $MOD_LIVEPATCH
> > > > -unload_mod $MOD_KPROBE
> > > > -
> > > > -check_result "% insmod test_modules/test_klp_kprobe.ko
> > > > has_post_handler=1
> > > > -% insmod test_modules/$MOD_LIVEPATCH.ko
> > > > -livepatch: enabling patch '$MOD_LIVEPATCH'
> > > > -livepatch: '$MOD_LIVEPATCH': initializing patching transition
> > > > -livepatch: failed to register ftrace handler for function
> > > > 'cmdline_proc_show' (-16)
> > > > -livepatch: failed to patch object 'vmlinux'
> > > > -livepatch: failed to enable patch '$MOD_LIVEPATCH'
> > > > -livepatch: '$MOD_LIVEPATCH': canceling patching transition,
> > > > going
> > > > to unpatch
> > > > -livepatch: '$MOD_LIVEPATCH': completing unpatching transition
> > > > -livepatch: '$MOD_LIVEPATCH': unpatching complete
> > > > -insmod: ERROR: could not insert module
> > > > test_modules/$MOD_LIVEPATCH.ko: Device or resource busy
> > > > -% rmmod test_klp_kprobe"
> > > > -
> > > >  start_test "livepatch interaction with kprobed function
> > > > without
> > > > post_handler"
> > > >  
> > > >  load_mod $MOD_KPROBE has_post_handler=0
> > > > +
> > > > +# Check if commit 0bc11ed5ab60c ("kprobes: Allow kprobes
> > > > coexist
> > > > with livepatch")
> > > > +# is missing, meaning that livepatches and kprobes can't be
> > > > used
> > > > together.
> > > > +# When the commit is missing, kprobes always set IPMODIFY (the
> > > > I
> > > > flag), even
> > > > +# when the post handler is missing.
> > > > +if grep --quiet ") R I"
> > > > "$SYSFS_DEBUG_DIR/tracing/enabled_functions"; then
> > > 
> > > Will flags R I always be in this order?
> > 
> > 		seq_printf(m, " (%ld)%s%s%s%s%s",
> > 			   ftrace_rec_count(rec),
> > 			   rec->flags & FTRACE_FL_REGS ? " R" : " 
> > ",
> > 			   rec->flags & FTRACE_FL_IPMODIFY ? " I"
> > : " 
> > ",
> > 
> > So this is safe. I'll add a comment in the patch to explain why
> > this is
> > safe too. Thanks for the comment!
> 
> I would personally check also "cmdline_proc_show" to make sure that
> the line is about this function. Something like:
> 
>      grep --quiet ") "cmdline_proc_show.*([0-9]\+) R"
> 
> 
> But I am afraid that this approach is not good. It breaks the test.
> It won't longer be able to catch regressions when the kprobe
> sets "FTRACE_FL_IPMODIFY" by mistake again.
> 
> We could add a version check. But it would break users who backport
> the fix into older kernels.
> 
> IMHO, the best solution would be to keep the test as is.
> Whoever is running the test with older kernels should mark it
> as "failure-expected". The test is pointing out an existing problem
> in the old kernel. IMHO, it should not hide it.

Makes sense, thanks for your review Petr. I'll drop this patch from v2.

> 
> Best Regards,
> Petr