Re: [PATCH] x86/alternative: Consistently patch SMP locks in vmlinux and modules

[Julian Pidancet]

On Wed Oct 26, 2022 at 20:08, Borislav Petkov wrote:
> On Tue, Aug 30, 2022 at 09:42:37AM +0200, Julian Pidancet wrote:
> > The alternatives_smp_module_add() function restricts patching of SMP
> > lock prefixes to the text address range passed as an argument.
> >
> > For vmlinux, patching all the instructions located between the _text and
> > _etext symbols is allowed. That includes the .text section but also
> > other sections such as .text.hot and .text.unlikely.
> >
> > As per the comment inside the 'struct smp_alt_module' definition, the
> > original purpose of this restriction is to avoid patching the init code
> > which may have been deallocated when the alternatives code run.
> >
> > For modules, the current code only allows patching instructions located
> > inside the .text segment, excluding other sections such as .text.hot or
> > .text.unlikely, which may need patching.
>
> Is this something you noticed by inspection or is there a real failure
> behind it?
>

We do live patching of the kernel code at Ksplice. Before applying a
binary patch we have a safety step that compares the running code with
the code that the binary patch is expected to modify and abort if they
differ.
This problem was picked up by the tools that we developped for this
purpose. We have a workaround for this internally, yet the inconsistency
lies in the kernel, so we thought it best fixed there directly.

> > This change aims to make patching of the kernel core and modules more
>
> Avoid having "This patch" or "This commit" and so on, in the commit
> message. It is tautologically useless.
>
> Also, do
>
> $ git grep 'This patch' Documentation/process
>
> for more details.
>
> > consistent, by allowing all text sections of modules except .init.text
> > to be patched in module_finalize().
> >
> > For that we use mod->core_layout.base/mod->core_layout.text_size as the
>
> Please use passive voice in your commit message: no "we" or "I", etc,
> and describe your changes in imperative mood.
>
> Bottom line is: personal pronouns are ambiguous in text, especially with
> so many parties/companies/etc developing the kernel so let's avoid them
> please.
>

Thanks for your time looking at this. I'll reword the commit description
and submit a v2 shortly.

Regards,

--
Julian

Attachment: signature.asc
Description: PGP signature