linux-kernel - Re: [RFC PATCH 2/2] livepatch: Implement livepatch hybrid mode

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Z6YRhjQA4wkBxP0v@pathway.suse.cz>
Date: Fri, 7 Feb 2025 14:58:30 +0100
From: Petr Mladek <pmladek@...e.com>
To: Yafang Shao <laoar.shao@...il.com>
Cc: jpoimboe@...nel.org, jikos@...nel.org, mbenes@...e.cz,
	joe.lawrence@...hat.com, live-patching@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [RFC PATCH 2/2] livepatch: Implement livepatch hybrid mode

On Thu 2025-02-06 10:35:11, Yafang Shao wrote:
> On Thu, Feb 6, 2025 at 12:03 AM Petr Mladek <pmladek@...e.com> wrote:
> >
> > On Wed 2025-02-05 10:54:47, Yafang Shao wrote:
> > > On Tue, Feb 4, 2025 at 9:21 PM Petr Mladek <pmladek@...e.com> wrote:
> > > >
> > > > On Mon 2025-01-27 23:34:50, Yafang Shao wrote:

I am not sure if you still would want the hybrid model.
It is possible that the timeout in klp_try_complete_transition()
would remove the hardlockup watchdog warnings, see
https://lore.kernel.org/r/Z6Tmqro6CSm0h-E3@pathway.suse.cz

I reply to this mail just for record because there were some
unanswered questions...

> > > To clarify the hybrid model, I'll illustrate it with the following Fixes:
> > >
> > > Fix1 livepatches: funcA
> > > Fix2  livepatches: funcC
> > > Fix3 livepatches: funcA, funcB
> >
> > How does look the livepatched FuncA here?
> > Does it contain changes only for Fix3?
> > Or is it cummulative livepatches_funA includes both Fix1 + Fix3?
> 
> It is cumulative livepatches_funA includes both Fix1 + Fix3.

It makes sense.

I have missed this in the previous mail. I see it there now (after
re-reading). But trick was somehow encrypted in long sentences.

> > > Fix4  livepatches: funcD
> > > Fix5 livepatches: funcB
> > >
> > > Each FixN represents a single /sys/kernel/livepatch/FixN.
> > > By default, all Fixes are set to non-replaceable.
> > >
> > > Step-by-step process:
> > > 1. Loading Fix1
> > >    Loaded Fixes: Fix1
> > > 2. Loading Fix2
> > >    Loaded Fixes: Fix1 Fix2
> > > 3. Loading Fix3
> > >     Before loading, set Fix1 to replaceable and replace it with Fix3,
> > > which is a cumulative patch of Fix1 and Fix3.
> >
> > Who will modify the "replaceable" flag of "Fix1"? Userspace or kernel?
> 
> Userspace scripts. Modify it before loading a new one.

This is one mine concern. Such a usespace script would be
more complex for the the hybrid model then for cumulative livepatches.
Any user of the hybrid model would have their own scripts
and eventual bugs.

Anyway, the more possibilities there more ways to break things
and the more complicated is to debug eventual problems.

If anyone would still like the hybrid model then I would like
to enforce some safe behavior from the kernel. I mean to do
as much as possible in the common (kernel) code.

I have the following ideas:

  1. Allow to load a livepatch with .replace enabled only when
     all conflicting[*] livepatches are allowed to be replaced.

  2. Automatically replace all conflicting[*] livepatches.

  3. Allow to define a list of to-be-replaced livepatches
     into struct patch.

The 1st or 2nd idea would make the hybrid model more safe.

The 2nd idea would even work without the .replaceable flag.

The 3rd idea would allow to replace even non-conflicting[*]
patches.

[*] I would define that two livepatches are "conflicting" when
    at least one function is modified by both of them. e.g.

	+ Patch1: funcA, funcB   \
	+ Patch2: funcC, funcD   - non-conflicting 

	+ Patch1: funcA, funcB          \
	+ Patch2:        funcB, funcC   - conflicting 

    Or a bit weaker definition. Two patches are "conflicting"
    when the new livepatch provides only partial replacement
    for already livepatch functions, .e.g.

	+ Patch1: funcA, funcB                \
	+ Patch2:               funcC, funcD  - non-conflicting anyway

	+ Patch1: funcA, funcB          - Patch1 can't replace Patch2 (conflict)
	+ Patch2: funcA, funcB, funcC   - Patch2 can replace Patch1 (no conflict)

	+ Patch1: funcA, funcB          \
	+ Patch2:        funcB, funcC   - conflicting anyway

Especially, the automatic replacement of conflicting patches might
make the hybrid model safe and easier to use. And it would resolve
most of my fears with this approach.

Best Regards,
Petr