linux-kernel - Re: [PATCH v4 2/2] livepatch: Add using attribute to klp

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: <alpine.LSU.2.21.2409060851580.1385@pobox.suse.cz>
Date: Fri, 6 Sep 2024 08:55:52 +0200 (CEST)
From: Miroslav Benes <mbenes@...e.cz>
To: Josh Poimboeuf <jpoimboe@...nel.org>
cc: Wardenjohn <zhangwarden@...il.com>, jikos@...nel.org, pmladek@...e.com, 
    joe.lawrence@...hat.com, live-patching@...r.kernel.org, 
    linux-kernel@...r.kernel.org
Subject: Re: [PATCH v4 2/2] livepatch: Add using attribute to klp_func for
 using function show

On Thu, 5 Sep 2024, Josh Poimboeuf wrote:

> On Thu, Sep 05, 2024 at 12:23:20PM +0200, Miroslav Benes wrote:
> > I am not a fan. Josh wrote most of my objections already so I will not 
> > repeat them. I understand that the attribute might be useful but the 
> > amount of code it adds to sensitive functions like 
> > klp_complete_transition() is no fun.
> > 
> > Would it be possible to just use klp_transition_patch and implement the 
> > logic just in using_show()?
> 
> Yes, containing the logic to the sysfs file sounds a lot better.
> 
> > I have not thought through it completely but 
> > klp_transition_patch is also an indicator that there is a transition going 
> > on. It is set to NULL only after all func->transition are false. So if you 
> > check that, you can assign -1 in using_show() immediately and then just 
> > look at the top of func_stack.
> 
> sysfs already has per-patch 'transition' and 'enabled' files so I don't
> like duplicating that information.
> 
> The only thing missing is the patch stack order.  How about a simple
> per-patch file which indicates that?
> 
>   /sys/kernel/livepatch/<patchA>/order => 1
>   /sys/kernel/livepatch/<patchB>/order => 2
> 
> The implementation should be trivial with the use of
> klp_for_each_patch() to count the patches.

Yes, it should work as well.

Wardenjohn, you should then get all the information that you need. Also, 
please test your patches with livepatch kselftests before a submission 
next time. New sysfs attributes need to be documented in 
Documentation/ABI/testing/sysfs-kernel-livepatch and there should be a new 
kselftest for them.

Thank you,
Miroslav