linux-kernel - Re: [PATCH] Documentation: livepatch: Add kernel-doc link to klp_enable

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: <YbMc8YGIoyRU5nwJ@alley>
Date:   Fri, 10 Dec 2021 10:25:05 +0100
From:   Petr Mladek <pmladek@...e.com>
To:     David Vernet <void@...ifault.com>
Cc:     linux-doc@...r.kernel.org, live-patching@...r.kernel.org,
        linux-kernel@...r.kernel.org, jpoimboe@...hat.com,
        jikos@...nel.org, mbenes@...e.cz, joe.lawrence@...hat.com,
        corbet@....net, yhs@...com, songliubraving@...com
Subject: Re: [PATCH] Documentation: livepatch: Add kernel-doc link to
 klp_enable_patch

On Thu 2021-12-09 08:53:04, David Vernet wrote:
> The `klp_enable_patch()` function is the main entrypoint to the livepatch
> subsystem, and is invoked by a KLP module from the module_init callback
> when it is ready to be enabled.  The livepatch documentation specifies that
> `klp_enable_patch()` should be invoked from the `module_init()` callback,
> but does not actually link the user to the function's kerneldoc comment.
> 
> This simple change therefore adds a kernel-doc directive to link the
> `klp_enable_patch()` function's kerneldoc comment in the livepatch
> documentation page. With this, kernel/livepatch/core.c no longer comes up
> as a file containing an unused doc with
> `scripts/find-unused-docs.sh kernel/livepatch`
> 
> --- a/Documentation/livepatch/livepatch.rst
> +++ b/Documentation/livepatch/livepatch.rst
> @@ -312,8 +312,15 @@ the patch cannot get enabled.
>  -------------
>  
>  The livepatch gets enabled by calling klp_enable_patch() from
> -the module_init() callback. The system will start using the new
> -implementation of the patched functions at this stage.
> +the module_init() callback:
> +
> +.. kernel-doc:: kernel/livepatch/core.c
> +   :functions: klp_enable_patch
> +
> +----
> +
> +The system will start using the new implementation of the patched functions at
> +this stage.
>  
>  First, the addresses of the patched functions are found according to their
>  names. The special relocations, mentioned in the section "New functions",

Honestly, I do not like this. It might be acceptable when it converts
klp_enable_patch() into a link pointing to another page describing the API.

But this patch causes the entire documentation of klp_enable_patch()
inserted into livepatch.html. It does not fit there and breaks
the text flow.

Heh, I had hard times to build the documentation (sphinx crashed, ...).
So, I paste the html output for others here:

<cut&paste>
5.2. Enabling¶
The livepatch gets enabled by calling klp_enable_patch() from the module_init() callback:

int klp_enable_patch(struct klp_patch *patch)¶
enable the livepatch

Parameters

struct klp_patch *patch
patch to be enabled

Description

Initializes the data structure associated with the patch, creates the sysfs interface, performs the needed symbol lookups and code relocations, registers the patched functions with ftrace.

This function is supposed to be called from the livepatch module_init() callback.

Return

0 on success, otherwise error

The system will start using the new implementation of the patched functions at this stage.

First, the addresses of the patched functions are found according to their names. The special relocations, mentioned in the section “New functions”, are applied. The relevant entries are created under /sys/kernel/livepatch/<name>. The patch is rejected when any above operation fails.

Second, livepatch enters into a transition state where tasks are converging to the patched state. If an original function is patched for the first time, a function specific struct klp_ops is created and an universal ftrace handler is registered1. This stage is indicated by a value of ‘1’ in /sys/kernel/livepatch/<name>/transition. For more information about this process, see the “Consistency model” section.

Finally, once all tasks have been patched, the ‘transition’ value changes to ‘0’.

1
Note that functions might be patched multiple times. The ftrace handler is registered only once for a given function. Further patches just add an entry to the list (see field func_stack) of the struct klp_ops. The right implementation is selected by the ftrace handler, see the “Consistency model” section.

That said, it is highly recommended to use cumulative livepatches
because they help keeping the consistency of all changes. In this
case, functions might be patched two times only during the transition
period.
</cut&paste>

Best Regards,
Petr