linux-kernel - Re: [RFC PATCH v1 0/2] Introduce XSAVE/XRSTOR self-test

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: <af8e6de6-c428-d3f9-9abf-f8bd5cc95838@intel.com>
Date:   Tue, 27 Jul 2021 10:46:45 -0700
From:   Dave Hansen <dave.hansen@...el.com>
To:     Pengfei Xu <pengfei.xu@...el.com>,
        Shuah Khan <skhan@...uxfoundation.org>,
        linux-kselftest <linux-kselftest@...r.kernel.org>,
        linux-kernel <linux-kernel@...r.kernel.org>
Cc:     Heng Su <heng.su@...el.com>, Yu Yu-cheng <yu-cheng.yu@...el.com>,
        Yu Fenghua <fenghua.yu@...el.com>,
        Luck Tony <tony.luck@...el.com>,
        Mehta Sohil <sohil.mehta@...el.com>,
        Chen Yu C <yu.c.chen@...el.com>
Subject: Re: [RFC PATCH v1 0/2] Introduce XSAVE/XRSTOR self-test

On 7/26/21 8:34 PM, Pengfei Xu wrote:
> The XSAVE feature set supports the saving and restoring of state components
> such as FPU, which is used for process context switching.

This sentence is really awkward.  It reads at first as saying that the
FPU is used for context switching.  Can you rephrase.

> In order to ensure that XSAVE works correctly, add XSAVE basic test for
> XSAVE architecture functionality.

This sentence needs to be start on the same line as the previous one,
*or* be in a new paragraph.  Please rewrap it.

> This patch set tests XSAVE/XRSTOR instructions on x86 platforms and verify if
> the XSAVE/XRSTOR works correctly during signal handling.

This reads to me like you are going to test the XSAVE/XRSTOR
instructions *in* a signal handler, instead of testing the XSAVE/XRSTOR
instructions that the kernel uses at signal entry/exit.

Also, the kernel does *NOT* *USE* XSAVE/XRSTOR in many cases to
save/restore signal state.  The changelog could be read as implying that
it does.

> Cases such as signal handling, process creation, other xstate(except FPU)
> tests for XSAVE check, etc. will be added to the Linux kernel self-test.
> If appropriate, it is even planned to add the [1] mentioned XSAVE issues
> reproduce and some XSAVE anomaly tests to the kernel self-test.

This is not clear whether it is talking about *this* series int he
future tense (will be added) or whether it is talking about future *work*.

Maybe something like this:

	This series introduces only the most basic XSAVE tests.  In the
	future, the intention is to continue expanding the scope of
	these selftests to include more kernel XSAVE-related
	functionality and XSAVE-managed features like AMX and shadow
	stacks.