From: Borislav Petkov <bp@alien8.de>
To: "Chang S. Bae" <chang.seok.bae@intel.com>
Cc: luto@kernel.org, tglx@linutronix.de, mingo@kernel.org,
x86@kernel.org, len.brown@intel.com, dave.hansen@intel.com,
thiago.macieira@intel.com, jing2.liu@intel.com,
ravi.v.shankar@intel.com, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v9 11/26] x86/fpu/xstate: Update the XSTATE context copy function to support dynamic states
Date: Wed, 18 Aug 2021 14:03:29 +0200 [thread overview]
Message-ID: <YRz3EWQl7pHEahdF@zn.tnic> (raw)
In-Reply-To: <20210730145957.7927-12-chang.seok.bae@intel.com>
On Fri, Jul 30, 2021 at 07:59:42AM -0700, Chang S. Bae wrote:
> ptrace() and signal return paths use XSTATE context copy functions. They
> allow callers to read (or write) XSTATE values in the target's buffer. With
> dynamic user states, a component's position in the buffer may vary and the
> init fpstate is not always large enough to cover all the states.
>
> Adjust the helpers to find a component's offset correctly. Also, update the
> copy loop in the ptrace read path to support dynamic states.
>
> Signed-off-by: Chang S. Bae <chang.seok.bae@intel.com>
> Reviewed-by: Len Brown <len.brown@intel.com>
> Cc: x86@kernel.org
> Cc: linux-kernel@vger.kernel.org
> ---
> Changes from v5:
> * Updated to ensure xstate_bv aligned with the target.
> * Rewrote the xstate copy loop, for the ptrace() read path, in an open
> code.
> * Adjusted the changelog.
>
> Changes from v3:
> * Cleaned up the code change with more comments.
> * Removed 'no functional change' in the changelog. (Borislav Petkov)
>
> Changes from v2:
> * Updated the changelog with task->fpu removed. (Borislav Petkov)
> ---
> arch/x86/kernel/fpu/xstate.c | 30 +++++++++++++++++++++++++-----
> 1 file changed, 25 insertions(+), 5 deletions(-)
>
> diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c
> index 98ab10e4da3b..3b56e7612c45 100644
> --- a/arch/x86/kernel/fpu/xstate.c
> +++ b/arch/x86/kernel/fpu/xstate.c
> @@ -1273,6 +1273,7 @@ void copy_xstate_to_uabi_buf(struct membuf to, struct task_struct *tsk,
> zerofrom = offsetof(struct xregs_state, extended_state_area);
>
> for (i = FIRST_EXTENDED_XFEATURE; i < XFEATURE_MAX; i++) {
> + u64 mask = BIT_ULL(i);
> /*
> * The ptrace buffer is in non-compacted XSAVE format.
> * In non-compacted format disabled features still occupy
> @@ -1280,7 +1281,7 @@ void copy_xstate_to_uabi_buf(struct membuf to, struct task_struct *tsk,
> * compacted init_fpstate. The gap tracking will zero this
> * later.
> */
> - if (!(xfeatures_mask_uabi() & BIT_ULL(i)))
> + if (!(xfeatures_mask_uabi() & mask))
> continue;
>
> /*
> @@ -1300,10 +1301,24 @@ void copy_xstate_to_uabi_buf(struct membuf to, struct task_struct *tsk,
> pkru.pkru = tsk->thread.pkru;
> membuf_write(&to, &pkru, sizeof(pkru));
> } else {
So this chunk, the else branch starting here is begging to be a separate
function.
> - copy_feature(header.xfeatures & BIT_ULL(i), &to,
> - __raw_xsave_addr(&tsk->thread.fpu, i),
> - __raw_xsave_addr(NULL, i),
> - xstate_sizes[i]);
> + unsigned int size = xstate_sizes[i];
> + void *from = NULL;
> +
> + /*
> + * Copy the xstate if available. Otherwise, copy the
> + * non-zero init states for legacy states (FP and
> + * SSE) or fill zeros.
> + */
> +
> + if (header.xfeatures & mask)
> + from = __raw_xsave_addr(&tsk->thread.fpu, i);
> + else if (XFEATURE_MASK_FPSSE & mask)
The i loop variable above starts from FIRST_EXTENDED_XFEATURE - why is
this XFEATURE_MASK_FPSSE check even here?
> + from = __raw_xsave_addr(NULL, i);
> +
> + if (from)
> + membuf_write(&to, from, size);
> + else
> + membuf_zero(&to, size);
> }
> /*
> * Keep track of the last copied state in the non-compacted
> @@ -1345,6 +1360,8 @@ static int copy_uabi_to_xstate(struct fpu *fpu, const void *kbuf,
> if (validate_user_xstate_header(&hdr))
> return -EINVAL;
>
> + hdr.xfeatures &= fpu->state_mask;
> +
This hunk looks arbitrary here and wants to be together with the patch
which adds ->state_mask.
> /* Validate MXCSR when any of the related features is in use */
> mask = XFEATURE_MASK_FP | XFEATURE_MASK_SSE | XFEATURE_MASK_YMM;
> if (hdr.xfeatures & mask) {
> @@ -1371,6 +1388,9 @@ static int copy_uabi_to_xstate(struct fpu *fpu, const void *kbuf,
> if (hdr.xfeatures & mask) {
> void *dst = __raw_xsave_addr(fpu, i);
>
> + if (!dst)
> + continue;
> +
> offset = xstate_offsets[i];
> size = xstate_sizes[i];
I don't know where this hunk belongs to...
Maybe as a completely separate patch which fixes the case where
__raw_xsave_addr() can in the very unlikely event return NULL...
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
next prev parent reply other threads:[~2021-08-18 12:02 UTC|newest]
Thread overview: 91+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-07-30 14:59 [PATCH v9 00/26] x86: Support Intel Advanced Matrix Extensions Chang S. Bae
2021-07-30 14:59 ` [PATCH v9 01/26] x86/fpu/xstate: Modify the initialization helper to handle both static and dynamic buffers Chang S. Bae
2021-07-30 14:59 ` [PATCH v9 02/26] x86/fpu/xstate: Modify state copy helpers " Chang S. Bae
2021-07-30 14:59 ` [PATCH v9 03/26] x86/fpu/xstate: Modify address finders " Chang S. Bae
2021-07-30 14:59 ` [PATCH v9 04/26] x86/fpu/xstate: Add a new variable to indicate dynamic user states Chang S. Bae
2021-07-30 14:59 ` [PATCH v9 05/26] x86/fpu/xstate: Add new variables to indicate dynamic XSTATE buffer size Chang S. Bae
2021-08-12 15:03 ` Borislav Petkov
2021-07-30 14:59 ` [PATCH v9 06/26] x86/fpu/xstate: Calculate and remember dynamic XSTATE buffer sizes Chang S. Bae
2021-08-12 16:36 ` Borislav Petkov
2021-07-30 14:59 ` [PATCH v9 07/26] x86/fpu/xstate: Convert the struct fpu 'state' field to a pointer Chang S. Bae
2021-08-12 17:09 ` Borislav Petkov
2021-07-30 14:59 ` [PATCH v9 08/26] x86/fpu/xstate: Introduce helpers to manage the XSTATE buffer dynamically Chang S. Bae
2021-08-12 19:44 ` Borislav Petkov
2021-08-13 8:04 ` Bae, Chang Seok
2021-08-13 10:04 ` Borislav Petkov
2021-08-13 19:43 ` Bae, Chang Seok
2021-08-18 9:28 ` Borislav Petkov
2021-08-18 19:46 ` Bae, Chang Seok
2021-08-25 16:01 ` Bae, Chang Seok
2021-08-30 17:07 ` Borislav Petkov
2021-08-30 23:39 ` Bae, Chang Seok
2021-08-16 18:33 ` Bae, Chang Seok
2021-08-16 18:53 ` Borislav Petkov
2021-08-30 17:45 ` Dave Hansen
2021-08-30 23:39 ` Bae, Chang Seok
2021-07-30 14:59 ` [PATCH v9 09/26] x86/fpu/xstate: Update the XSTATE save function to support dynamic states Chang S. Bae
2021-07-30 14:59 ` [PATCH v9 10/26] x86/fpu/xstate: Update the XSTATE buffer address finder " Chang S. Bae
2021-08-18 11:33 ` Borislav Petkov
2021-08-18 19:47 ` Bae, Chang Seok
2021-08-30 17:18 ` Borislav Petkov
2021-08-30 23:38 ` Bae, Chang Seok
2021-07-30 14:59 ` [PATCH v9 11/26] x86/fpu/xstate: Update the XSTATE context copy function " Chang S. Bae
2021-08-18 12:03 ` Borislav Petkov [this message]
2021-08-18 19:47 ` Bae, Chang Seok
2021-07-30 14:59 ` [PATCH v9 12/26] x86/fpu/xstate: Use feature disable (XFD) to protect dynamic user state Chang S. Bae
2021-08-18 16:24 ` Borislav Petkov
2021-08-18 17:20 ` Thiago Macieira
2021-08-18 17:46 ` Borislav Petkov
2021-08-18 17:58 ` Thiago Macieira
2021-08-18 18:10 ` Borislav Petkov
2021-08-24 22:51 ` Len Brown
2021-08-18 20:43 ` Bae, Chang Seok
2021-08-18 21:04 ` Thiago Macieira
2021-08-18 21:12 ` Bae, Chang Seok
2021-08-18 22:27 ` Thiago Macieira
2021-08-19 1:21 ` Andy Lutomirski
2021-08-19 16:06 ` Thiago Macieira
2021-08-18 21:17 ` Borislav Petkov
2021-08-18 21:37 ` Bae, Chang Seok
2021-08-19 8:00 ` Borislav Petkov
2021-08-19 15:24 ` Bae, Chang Seok
2021-08-24 23:22 ` Len Brown
2021-08-30 17:31 ` Borislav Petkov
2021-09-17 3:48 ` Len Brown
2021-08-18 19:47 ` Bae, Chang Seok
2021-08-24 22:21 ` Len Brown
2021-08-30 17:41 ` Borislav Petkov
2021-08-31 21:44 ` Len Brown
2021-08-24 23:17 ` Len Brown
2021-08-30 17:53 ` Borislav Petkov
2021-08-31 22:07 ` Len Brown
2021-08-31 22:11 ` Dave Hansen
2021-08-30 18:04 ` Dave Hansen
2021-08-31 22:15 ` Len Brown
2021-08-31 22:16 ` Len Brown
2021-08-31 22:39 ` Thiago Macieira
2021-08-31 22:44 ` Len Brown
2021-07-30 14:59 ` [PATCH v9 13/26] x86/fpu/xstate: Support ptracer-induced XSTATE buffer expansion Chang S. Bae
2021-07-30 14:59 ` [PATCH v9 14/26] x86/arch_prctl: Create ARCH_SET_STATE_ENABLE/ARCH_GET_STATE_ENABLE Chang S. Bae
2021-08-06 16:46 ` Thiago Macieira
2021-08-09 22:08 ` Bae, Chang Seok
2021-08-09 23:42 ` Thiago Macieira
2021-08-10 0:57 ` Bae, Chang Seok
2021-08-13 19:44 ` Bae, Chang Seok
2021-07-30 14:59 ` [PATCH v9 15/26] x86/fpu/xstate: Support both legacy and expanded signal XSTATE size Chang S. Bae
2021-07-30 14:59 ` [PATCH v9 16/26] x86/fpu/xstate: Adjust the XSAVE feature table to address gaps in state component numbers Chang S. Bae
2021-07-30 14:59 ` [PATCH v9 17/26] x86/fpu/xstate: Disable XSTATE support if an inconsistent state is detected Chang S. Bae
2021-07-30 14:59 ` [PATCH v9 18/26] x86/cpufeatures/amx: Enumerate Advanced Matrix Extension (AMX) feature bits Chang S. Bae
2021-07-30 14:59 ` [PATCH v9 19/26] x86/fpu/amx: Define AMX state components and have it used for boot-time checks Chang S. Bae
2021-07-30 14:59 ` [PATCH v9 20/26] x86/fpu/amx: Initialize child's AMX state Chang S. Bae
2021-07-30 14:59 ` [PATCH v9 21/26] x86/fpu/amx: Enable the AMX feature in 64-bit mode Chang S. Bae
2021-07-30 14:59 ` [PATCH v9 22/26] x86/fpu/xstate: Skip writing zeros to signal frame for dynamic user states if in INIT-state Chang S. Bae
2021-07-30 14:59 ` [PATCH v9 23/26] selftest/x86/amx: Test cases for the AMX state management Chang S. Bae
2021-07-30 14:59 ` [PATCH v9 24/26] x86/insn/amx: Add TILERELEASE instruction to the opcode map Chang S. Bae
2021-07-30 14:59 ` [PATCH v9 25/26] intel_idle/amx: Add SPR support with XTILEDATA capability Chang S. Bae
2021-07-30 18:41 ` Dave Hansen
2021-08-03 21:32 ` Bae, Chang Seok
2021-08-03 21:38 ` Dave Hansen
2021-08-03 21:43 ` Brown, Len
2021-07-30 20:15 ` Dave Hansen
2021-07-30 14:59 ` [PATCH v9 26/26] x86/fpu/xstate: Add a sanity check for XFD state when saving XSTATE Chang S. Bae
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=YRz3EWQl7pHEahdF@zn.tnic \
--to=bp@alien8.de \
--cc=chang.seok.bae@intel.com \
--cc=dave.hansen@intel.com \
--cc=jing2.liu@intel.com \
--cc=len.brown@intel.com \
--cc=linux-kernel@vger.kernel.org \
--cc=luto@kernel.org \
--cc=mingo@kernel.org \
--cc=ravi.v.shankar@intel.com \
--cc=tglx@linutronix.de \
--cc=thiago.macieira@intel.com \
--cc=x86@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.