From: Oleg Nesterov <oleg@redhat.com>
To: "Edgecombe, Rick P" <rick.p.edgecombe@intel.com>
Cc: "debug@rivosinc.com" <debug@rivosinc.com>,
"mingo@kernel.org" <mingo@kernel.org>,
"bp@alien8.de" <bp@alien8.de>,
"broonie@kernel.org" <broonie@kernel.org>,
"peterz@infradead.org" <peterz@infradead.org>,
"hpa@zytor.com" <hpa@zytor.com>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"tglx@linutronix.de" <tglx@linutronix.de>,
"dave.hansen@linux.intel.com" <dave.hansen@linux.intel.com>,
"Mehta, Sohil" <sohil.mehta@intel.com>,
"x86@kernel.org" <x86@kernel.org>
Subject: Re: [PATCH v2 0/5] x86/fpu: don't abuse x86_task_fpu(PF_USER_WORKER) in .regset_get() paths
Date: Wed, 3 Sep 2025 11:54:37 +0200 [thread overview]
Message-ID: <20250903095436.GA18799@redhat.com> (raw)
In-Reply-To: <e653fb9cab51ed2d0ea71f9d322c55420a83a4f5.camel@intel.com>
On 09/02, Edgecombe, Rick P wrote:
>
> On Fri, 2025-08-29 at 17:06 +0200, Oleg Nesterov wrote:
> > > *If* we want to worry about an extra shadow stack allocation (which Dave
> > > seems to doubt), we don't need to clear ARCH_SHSTK_SHSTK to avoid
> > > allocations.
> > > Other thread types already avoid it (vfork, etc). So just add to the
> > > existing logic that skips shadow stack allocation. Make it do that for user
> > > workers too, and leave ARCH_SHSTK_SHSTK alone.
> >
> > From 0/5:
> >
> > However, there is an annoying complication:
> > shstk_alloc_thread_stack()
> > can alloc the pointless shadow stack for PF_USER_WORKER thread and
> > set
> > the ARCH_SHSTK_SHSTK flag. This means that ssp_get()->ssp_active()
> > can
> > return true, and in this case it wouldn't be right to use the
> > "unrelated"
> > init_fpstate.
>
> Yea the ptrace code currently assumes there will be a non-init SHSTK FPU state.
> But if the init state is currently associated with the FPU, whether it's via a
> cleared copy, or some pointer redirection as you proposed, what is the
> difference?
Well. Lets forget about other changes for the moment. Lets only discuss 4/5.
> Hmm, I actually do see a potential concrete issue...
>
> fpu_clone() will wipe out the FPU state for PF_USER_WORKER, which means if
> xsaves decides to use the init optimization for CET, "get_xsave_addr(xsave,
> XFEATURE_CET_USER)" could return NULL and trigger a warning.
Even if get_xsave_addr() returns a valid pointer, what is the point to try to
report cetregs->user_ssp which doesn't match the reality?
Again, update_fpu_shstk() was not called, ->user_ssp can't be correct.
Why not simply clear ARCH_SHSTK_SHSTK as 4/5 does? With this change ssp_get()
will return -ENODEV right after the ssp_active() check.
Unless I am totally confused, ARCH_SHSTK_SHSTK has no meaning for PF_USER_WORKER
kernel threads, so I don't understand your objections.
Oleg.
next prev parent reply other threads:[~2025-09-03 9:56 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-08-22 15:36 [PATCH v2 0/5] x86/fpu: don't abuse x86_task_fpu(PF_USER_WORKER) in .regset_get() paths Oleg Nesterov
2025-08-22 15:36 ` [PATCH v2 1/5] x86/fpu: don't use x86_task_fpu() in copy_xstate_to_uabi_buf() Oleg Nesterov
2025-08-22 15:36 ` [PATCH v2 2/5] x86/fpu: regset: introduce get_fpstate() helper Oleg Nesterov
2025-08-22 15:36 ` [PATCH v2 3/5] x86/fpu: fold sync_fpstate() into get_fpstate() Oleg Nesterov
2025-08-22 15:37 ` [PATCH v2 4/5] x86/shstk: don't create the shadow stack for PF_USER_WORKERs Oleg Nesterov
2025-08-22 15:37 ` [PATCH v2 5/5] x86/fpu: change get_fpstate() to return &init_fpstate if PF_USER_WORKER Oleg Nesterov
2025-08-22 16:32 ` [PATCH v2 0/5] x86/fpu: don't abuse x86_task_fpu(PF_USER_WORKER) in .regset_get() paths Edgecombe, Rick P
2025-08-22 19:21 ` Oleg Nesterov
2025-08-22 20:01 ` Edgecombe, Rick P
2025-08-25 13:47 ` Oleg Nesterov
2025-08-27 14:12 ` Edgecombe, Rick P
2025-08-27 14:51 ` Oleg Nesterov
2025-08-28 21:48 ` Edgecombe, Rick P
2025-08-29 15:06 ` Oleg Nesterov
2025-09-02 20:37 ` Edgecombe, Rick P
2025-09-03 9:54 ` Oleg Nesterov [this message]
2025-09-03 15:46 ` Edgecombe, Rick P
2025-09-04 13:44 ` Oleg Nesterov
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=20250903095436.GA18799@redhat.com \
--to=oleg@redhat.com \
--cc=bp@alien8.de \
--cc=broonie@kernel.org \
--cc=dave.hansen@linux.intel.com \
--cc=debug@rivosinc.com \
--cc=hpa@zytor.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@kernel.org \
--cc=peterz@infradead.org \
--cc=rick.p.edgecombe@intel.com \
--cc=sohil.mehta@intel.com \
--cc=tglx@linutronix.de \
--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.