From: "Bae, Chang Seok" <chang.seok.bae@intel.com>
To: Borislav Petkov <bp@alien8.de>
Cc: "Lutomirski, Andy" <luto@kernel.org>,
"tglx@linutronix.de" <tglx@linutronix.de>,
"mingo@kernel.org" <mingo@kernel.org>,
"x86@kernel.org" <x86@kernel.org>,
"Brown, Len" <len.brown@intel.com>,
"Hansen, Dave" <dave.hansen@intel.com>,
"Macieira, Thiago" <thiago.macieira@intel.com>,
"Liu, Jing2" <jing2.liu@intel.com>,
"Shankar, Ravi V" <ravi.v.shankar@intel.com>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v9 08/26] x86/fpu/xstate: Introduce helpers to manage the XSTATE buffer dynamically
Date: Fri, 13 Aug 2021 19:43:53 +0000 [thread overview]
Message-ID: <2B279B5F-ACF2-46F2-9259-684419A57BDF@intel.com> (raw)
In-Reply-To: <YRZDu2Rk+KdRhh1U@zn.tnic>
On Aug 13, 2021, at 03:04, Borislav Petkov <bp@alien8.de> wrote:
> On Fri, Aug 13, 2021 at 08:04:54AM +0000, Bae, Chang Seok wrote:
>> Yes, each state offset in the non-compacted format is fixed in a machine
>> regardless of RFBM. So, simply take the size like that.
>
> Comment above it please.
>
> Also, why is this special case needed at all?
Without the “compacted” notion in the function name, one might call this even
with !XSAVES. But chances are very low in practice.
>> The query is intended to check whether the xstate buffer is fully expanded or
>> not -- no need to enlarge.
>>
>> If the buffer is already the maximum, the code to retrieve XSTATE_BV, this
>> call, etc should be skipped there.
>>
>> If the query is moved here, I guess this call site code becomes a bit ugly.
>
> Why does it become ugly?
>
> You simply return early without touching the buffer at all.
Perhaps, the call site in the ptrace path becomes like this, I think:
+ if (xfeatures_mask_user_dynamic) {
+ u64 state_mask;
+
+ /* Retrieve XSTATE_BV. */
+ memcpy(&state_mask, (kbuf ?: tmpbuf) + offsetof(struct xregs_state, header),
+ sizeof(u64));
+
+ /* Expand the xstate buffer based on the XSTATE_BV. */
+ ret = realloc_xstate_buffer(fpu, state_mask & xfeatures_mask_user_dynamic);
+ if (ret)
+ goto out;
+ }
Maybe retrieve XSTATE_BV is inevitable here. Then, it is not that ugly.
>> No, it is still pointed by fpu->state and will be freed in the exit path.
>
> Exit path of the task?
>
> All I see is "return -ENOMEM" and no callers of alloc_xstate_buffer()
> are calling free_xstate_buffer()...
>
> And looking further into the patchset:
>
> exc_device_not_available does not call free_xstate_buffer() I'm assuming
>
> force_sig_fault(SIGILL, ILL_ILLOPC,..
>
> later will cause arch_release_task_struct() to happen which will call
> free_xstate_buffer(). Yes, no?
Yes.
> I don't see any freeing in xstateregs_set() either, so what's happening
> there when it returns -ENOMEM?
>
> I guess there we remain with the old buffer, i.e., the ptrace operation
> fails.
>
> Am I close?
In this case, the ptracer just failed to inject some context. But the
ptracee’s context in the (old) buffer is intact. It will resume and eventually
exit. I think arch_release_task_struct()->free_xstate_buffer() will take care
of the old buffer.
Thanks,
Chang
next prev parent reply other threads:[~2021-08-13 19:44 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 [this message]
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
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=2B279B5F-ACF2-46F2-9259-684419A57BDF@intel.com \
--to=chang.seok.bae@intel.com \
--cc=bp@alien8.de \
--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.