Re: [PATCH] x86/msr: Filter MSR writes

[Borislav Petkov <bp@alien8.de>]

On Thu, Nov 19, 2020 at 11:53:44AM +0100, Mathieu Chouquet-Stringer wrote:
> ---
> TAINT_CPU_OUT_OF_SPEC now means what a bit more than what it implies as

s/now means what a bit/now means a bit/

> the flag isn't set just because of a CPU misconfiguration or mismatch.
> Historically it was for SMP kernel oops on an officially SMP incapable
> processor but now it also covers CPUs whose MSRs have been incorrectly
> poked at from userspace, drivers being used on non supported
> architectures, broken firmware, mismatched CPUs, ...
> 
> Update documentation and script to reflect that.
> 
> Signed-off-by: Mathieu Chouquet-Stringer <me@mathieu.digital>
> ---
>  Documentation/admin-guide/sysctl/kernel.rst   |  2 +-
>  Documentation/admin-guide/tainted-kernels.rst | 22 +++++++++++++++++-----
>  tools/debugging/kernel-chktaint               |  2 +-
>  3 files changed, 19 insertions(+), 7 deletions(-)
> 
> 
> diff --git a/Documentation/admin-guide/sysctl/kernel.rst b/Documentation/admin-guide/sysctl/kernel.rst
> index d4b32cc32bb7..edd89e2d3af7 100644
> --- a/Documentation/admin-guide/sysctl/kernel.rst
> +++ b/Documentation/admin-guide/sysctl/kernel.rst
> @@ -1336,7 +1336,7 @@ ORed together. The letters are seen in "Tainted" line of Oops reports.
>  ======  =====  ==============================================================
>       1  `(P)`  proprietary module was loaded
>       2  `(F)`  module was force loaded
> -     4  `(S)`  SMP kernel oops on an officially SMP incapable processor
> +     4  `(S)`  kernel running on an out of specification system
>       8  `(R)`  module was force unloaded
>      16  `(M)`  processor reported a Machine Check Exception (MCE)
>      32  `(B)`  bad page referenced or some unexpected page flags
> diff --git a/Documentation/admin-guide/tainted-kernels.rst b/Documentation/admin-guide/tainted-kernels.rst
> index f718a2eaf1f6..5737dfa17cd1 100644
> --- a/Documentation/admin-guide/tainted-kernels.rst
> +++ b/Documentation/admin-guide/tainted-kernels.rst
> @@ -84,7 +84,7 @@ Bit  Log  Number  Reason that got the kernel tainted
>  ===  ===  ======  ========================================================
>    0  G/P       1  proprietary module was loaded
>    1  _/F       2  module was force loaded
> -  2  _/S       4  SMP kernel oops on an officially SMP incapable processor
> +  2  _/S       4  kernel running on an out of specification system
>    3  _/R       8  module was force unloaded
>    4  _/M      16  processor reported a Machine Check Exception (MCE)
>    5  _/B      32  bad page referenced or some unexpected page flags
> @@ -116,10 +116,22 @@ More detailed explanation for tainting
>   1)  ``F`` if any module was force loaded by ``insmod -f``, ``' '`` if all
>       modules were loaded normally.
>  
> - 2)  ``S`` if the oops occurred on an SMP kernel running on hardware that
> -     hasn't been certified as safe to run multiprocessor.
> -     Currently this occurs only on various Athlons that are not
> -     SMP capable.
> + 2)  ``S`` if the kernel is running on a processor or system that is out of
> +     specification: hardware has been put into an unsupported configuration,
> +     therefore proper execution cannot be guaranteed.
> +     Kernel will be tainted for example if:

"Kernel will be tainted if, for example:"

> +
> +     - on x86: you force PAE on intel CPUs, you run a SMP kernel on non

"user forces PAE on some Intel CPUs which do not advertize it by default"
- otherwise it doesn't make sense.

Also, please use passive voice: no "we" or "I", etc. Look at how the
rest of the text in that file is written and try to mimick it and get
rid of the "you" formulations.

> +       officially capable SMP Athlon CPUs, you poke at random MSRs from
> +       userspace.
> +     - on arm: you run a kernel on certain CPUs (such as Keystone 2) without
> +       having certain kernel features enabled.
> +     - on arm64: you have mismatched hardware features between CPUs, the
> +       bootloader has booted CPUs in different modes.
> +     - you use certain drivers on non supported architectures (such as
> +       scsi/snic on something else than x86_64, scsi/ips on non
> +       x86/x86_64/itanium, have broken firmware settings for the
> +       irqchip/irq-gic on arm64 ...).

Yes, the text should be giving the idea that the configuration the
system is running on, is not supported.

All in all, this is shaping up ok, you can send the next one as
a proper patch with the people on Cc who should take it - (use
./scripts/get_maintainer.pl for that) and not as an inline diff anymore.

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette