From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 02757C5472D for ; Tue, 27 Aug 2024 00:53:35 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 40B016B0098; Mon, 26 Aug 2024 20:53:35 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 3BA166B0099; Mon, 26 Aug 2024 20:53:35 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 2817C6B009A; Mon, 26 Aug 2024 20:53:35 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0012.hostedemail.com [216.40.44.12]) by kanga.kvack.org (Postfix) with ESMTP id 0A9386B0098 for ; Mon, 26 Aug 2024 20:53:35 -0400 (EDT) Received: from smtpin05.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay08.hostedemail.com (Postfix) with ESMTP id A03271414E2 for ; Tue, 27 Aug 2024 00:53:34 +0000 (UTC) X-FDA: 82496202348.05.F0D7581 Received: from mail-oa1-f54.google.com (mail-oa1-f54.google.com [209.85.160.54]) by imf04.hostedemail.com (Postfix) with ESMTP id D172E4001B for ; Tue, 27 Aug 2024 00:53:32 +0000 (UTC) Authentication-Results: imf04.hostedemail.com; dkim=pass header.d=chromium.org header.s=google header.b="i/dU+czN"; dmarc=pass (policy=none) header.from=chromium.org; spf=pass (imf04.hostedemail.com: domain of jeffxu@chromium.org designates 209.85.160.54 as permitted sender) smtp.mailfrom=jeffxu@chromium.org ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1724719993; a=rsa-sha256; cv=none; b=yw95UPLAOeoh9x0NHEzewc/IfjJbnQ7qlGgHya2OoAXCwr3ypk3RljP3zqlqtSfimYwQUX be25oJXJBmTL/JteW3PRyf32+wFKSN2yDUnFtVZ5o2HwvI9dL3J6dyGpVzs+I1hzAzGRqA m7YZZHXs0vS6YiiPgT01zhfrvhNgZ54= ARC-Authentication-Results: i=1; imf04.hostedemail.com; dkim=pass header.d=chromium.org header.s=google header.b="i/dU+czN"; dmarc=pass (policy=none) header.from=chromium.org; spf=pass (imf04.hostedemail.com: domain of jeffxu@chromium.org designates 209.85.160.54 as permitted sender) smtp.mailfrom=jeffxu@chromium.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1724719993; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=JtuOQCHwx5HUQnS2vsYtjI8UWrxeqwGLR2irtHbOiFE=; b=AseOBJMdIypFuxBPFjmTR/51mrqmlloH2lIcR4TzWhGMwjF4tt0fikLV0T+G4nxGAdIPvA ds2zChnThchvZiL2RP6ZuENyeMH6SmZzmlZ4LXe/wv5+j4swZ7zHvm7qFOwNoLCsQhGsPX wWv5RNU5h4sfkjHBjj3s8+DxyYw28P0= Received: by mail-oa1-f54.google.com with SMTP id 586e51a60fabf-268e0b13471so689737fac.2 for ; Mon, 26 Aug 2024 17:53:32 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; t=1724720012; x=1725324812; darn=kvack.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=JtuOQCHwx5HUQnS2vsYtjI8UWrxeqwGLR2irtHbOiFE=; b=i/dU+czNOFQn1aPC12hdmxavulxGLnrv5ATm3f/JXKObTptQ95f0oT2gCSrpTX9aAN eHoIBVviZvI6WqYhPK4UNAuV6RvaXK/FpLyWPqyJGOuALNQVMnvHlqeeDEDN10pJREds wvX0CiaM/TLqDQj0D0W3cPm8dX+ZwXG+JA/jQ= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1724720012; x=1725324812; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=JtuOQCHwx5HUQnS2vsYtjI8UWrxeqwGLR2irtHbOiFE=; b=S2wzARDn6wiYVPYQsUfwKl3gFAz7t0s6vp/yT8hwYwvik8vldtjSPEuClVzoz4zAnJ dAvFCp52Two0CSjNiHJ5eWhLlVnJGNOBGvj57RJnFC8ZIlz82K50HR5Im1XXZ3uh6QrO bdHKNgrKs89Bu2bGXPs+f30c3/GwSELzTIF16Rsg2Ry8tEJSCt2fIp1wkOYv7YW5PWTD PkdxHBN+2CgFzqRUh4o2qW/I/cAJouYrV9UX1HRVp7O0M4e4yQggwH6DuxDHytb1IJWg D5O+cG+uHVImTr5YN6u4ikdGyTlO7je+fatPSZk5p10buGPzmV5tjvUiUCj8nJQk6mKG DcPQ== X-Forwarded-Encrypted: i=1; AJvYcCXIR/FTEiN70uY0AxfuZQHKaCfX8LM1TIkUI/9Z3tO+rBxZn1s3rHTViXT+Z+zJ+sp7vLYOk2Arvg==@kvack.org X-Gm-Message-State: AOJu0Yyxj0d0Q7HJKpaoAv1EsDnq1bHb+BwKlnU/36Kn4hlNlth7NgWZ o8Ht02oEH/Pom8VaR5szMlDI6Ta+RpWAue+A5B+pv8TrRkUj9mPztQNL0A5jqK87tOFaTgp5ECR yRF1kCX5JdxzZPwEmVwx6mo79oNlHQiU2sB1Z X-Google-Smtp-Source: AGHT+IEjCRa2q7PaKLyDpYx4ZA2zAuxQsjSc/D1uBOV536GrWwHdxkCRk9mOnCaKJd26BZOB4gIxB8tNLpiPp5eibo0= X-Received: by 2002:a05:6871:691:b0:260:f1c4:2fe3 with SMTP id 586e51a60fabf-273e6792bf4mr6456010fac.10.1724720011644; Mon, 26 Aug 2024 17:53:31 -0700 (PDT) MIME-Version: 1.0 References: <20240802061318.2140081-1-aruna.ramakrishna@oracle.com> <20240802061318.2140081-4-aruna.ramakrishna@oracle.com> In-Reply-To: From: Jeff Xu Date: Mon, 26 Aug 2024 17:53:19 -0700 Message-ID: Subject: Re: [PATCH v8 3/5] x86/pkeys: Update PKRU to enable all pkeys before XSAVE To: Aruna Ramakrishna Cc: linux-kernel@vger.kernel.org, x86@kernel.org, dave.hansen@linux.intel.com, tglx@linutronix.de, mingo@kernel.org, linux-mm@kvack.org, keith.lucas@oracle.com, rick.p.edgecombe@intel.com, jorgelo@chromium.org, keescook@chromium.org, sroettger@google.com, jannh@google.com Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspam-User: X-Rspamd-Queue-Id: D172E4001B X-Rspamd-Server: rspam01 X-Stat-Signature: cx6gctsb7tbu5np45icpbz9nktqof84c X-HE-Tag: 1724720012-862498 X-HE-Meta: U2FsdGVkX19dcpng5Azf1tm2HZ7MWAS5Yr/0ERGv1W9I8oNlgu4J/3WU2wrEWEfo1yYumEGMOvnitDMsB/hZWqGnEB3OALe1SNvXjy9XDq4BsE22L8ML8pOA2XVj8pjjzwP/jg41rdsmAMPTB6PwpY2C3q9uZepKUgLCnSt1CYJiA3L6s+JR1sEf+140KUFAKkAKxO7MwiGjbGCknSpOU8IlY/sdSoCSO49WNlfwzdDHhFz7vZ2Lbqd0KXhu4htuvCND0zNdQwopxyXqlkOciU4dW467B7gv/xPyN8lm6CBBdE+jOTCmdZ4s5xC0V7i+t/gLij68RAVvoXOkVv1V5m8H5kaqQxhAUuW9XmWy1Z7YxTvOjpjkfZdRVe70Utjyi8xHKBcKZ/J0995qLoQDeqn8osNWfdk8tZ49gKgpbrQrkXx1NdCIN6YLggQ1i9aCice6u4vW42cYSkrsWfnwgYRAKAxbw1JREbzWbfQex/RDbWafIfs1iialQZxzBuZst/oGu0DA6C4rnjow8cKhZrYzYE1zfV//k7FOkm0l3HoyDunN7oPAFXSPau/TQB8h8iGA+XQSfjtgZ9oiKN/UVpQt8Ak0IAH7GB/+E3qao7aks8dFxizuZcSJoduZWQDfNu30Failk7ChItZ5GuegGowNfbZV7yU0CVIP8Lx1t39Piaj7pdGY+g2NEIr3KiYpW6ubYCrYc726nl98BzRVzDHxgJ08XzE1P5+TZO3pCq3K2U/7TNlbhhXP0LQLMTRJQOCCvagiWvZZut11XIMhAhuFoZ6BA+B0gpiC36f8JBHcY8V27+xeg8yupb5Rr1PxRIPvK7lh4ULAjInNrBgxavk0nGC9cnNSXNg8xqQW/DYubMjAcuYBoWJNDl9zAz+n4H7XQEbBdaWc3Zt/j/PA1HE0PNFo2NZtTW1J35atKQIyUNAWYgo7WRWeor0aBnhj42w9DpPln5tlOL9vrbo +6yb1mQA UCDOMmOTFUyRcuauITuquFz3hE10bqE+Refx92VgM88r7fDw/jGHTysh3caoG6usJotm0Zyi8FENVGblPRGOW5a+eMZVaV5nvorbNJcpTq08+XuxmL8ns4sRrzwhssjgqs/rHc+OcgOhXtH+D0SUkykiCnlic5UOiLmKQQosguaR1HmneWXVaIHBPCPbz41zHVuYDnBj9nankXrATeNGIAINSXkuEMPoWgszZNJ68qJbf0Uhwm4cnS0L8J0LIXWUch2Hxzo8LKomMAXrogRV4isXSaw== X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: Hi Aruna On Fri, Aug 9, 2024 at 10:30=E2=80=AFAM Jeff Xu wrote= : > > On Thu, Aug 1, 2024 at 11:13=E2=80=AFPM Aruna Ramakrishna > wrote: > > > > If the alternate signal stack is protected by a different pkey than the > > current execution stack, copying xsave data to the sigaltstack will fai= l > > if its pkey is not enabled in the PKRU register. > > > > We do not know which pkey was used by the application for the altstack, > > so enable all pkeys before xsave. > > > > But this updated PKRU value is also pushed onto the sigframe, which > > means the register value restored from sigcontext will be different fro= m > > the user-defined one, which is unexpected. Fix that by overwriting the > > PKRU value on the sigframe with the original, user-defined PKRU. > > > > Signed-off-by: Aruna Ramakrishna > > --- > > arch/x86/kernel/fpu/signal.c | 11 +++++++++-- > > arch/x86/kernel/signal.c | 12 ++++++++++-- > > 2 files changed, 19 insertions(+), 4 deletions(-) > > > > diff --git a/arch/x86/kernel/fpu/signal.c b/arch/x86/kernel/fpu/signal.= c > > index 931c5469d7f3..1065ab995305 100644 > > --- a/arch/x86/kernel/fpu/signal.c > > +++ b/arch/x86/kernel/fpu/signal.c > > @@ -168,8 +168,15 @@ static inline bool save_xstate_epilog(void __user = *buf, int ia32_frame, > > > > static inline int copy_fpregs_to_sigframe(struct xregs_state __user *b= uf, u32 pkru) > > { > > - if (use_xsave()) > > - return xsave_to_user_sigframe(buf); > > + int err =3D 0; > > + > > + if (use_xsave()) { > > + err =3D xsave_to_user_sigframe(buf); > > + if (!err) > > + err =3D update_pkru_in_sigframe(buf, pkru); > > + return err; > > + } > > + > > if (use_fxsr()) > > return fxsave_to_user_sigframe((struct fxregs_state __u= ser *) buf); > > else > > diff --git a/arch/x86/kernel/signal.c b/arch/x86/kernel/signal.c > > index 9dc77ad03a0e..5f441039b572 100644 > > --- a/arch/x86/kernel/signal.c > > +++ b/arch/x86/kernel/signal.c > > @@ -102,7 +102,7 @@ get_sigframe(struct ksignal *ksig, struct pt_regs *= regs, size_t frame_size, > > unsigned long math_size =3D 0; > > unsigned long sp =3D regs->sp; > > unsigned long buf_fx =3D 0; > > - u32 pkru =3D read_pkru(); > > + u32 pkru; > > > > /* redzone */ > > if (!ia32_frame) > > @@ -157,9 +157,17 @@ get_sigframe(struct ksignal *ksig, struct pt_regs = *regs, size_t frame_size, > > return (void __user *)-1L; > > } > > > > + /* Update PKRU to enable access to the alternate signal stack. = */ > > + pkru =3D sig_prepare_pkru(); > I think, the better place to call sig_prepare_pkru() at begging of the > get_sigframe: > > get_sigframe() > { > /* ... */ > if (ka->sa.sa_flags & SA_ONSTACK) { > if (sas_ss_flags(sp) =3D=3D 0) { > // set pkru =3D 0 <--- set pkru =3D 0 here. > entering_altstack =3D true; > } > } > For two reasons: > - We probably don't want all signaling handling going through "PKRU=3D0" > , this includes the regular stack and nested signaling handling. The > best place is when "entering the altstack". IIUC, this feature only > enabled when sigaltstack() is used. > - The thread might not have read-access to the altstack, so we will > want to make sure that pkru=3D0 is set before any read to the altstack. > And IIRC, fpu__alloc_mathframe needs read-access to the altstack. > To help with testing, I will send a test case to demo the usage. Apologies, I remembered it wrong, the fpu__alloc_mathframe doesn't need read-access to altstack. But I think the best place to set pkru=3D0 is still when the stack entering altstack, as shown above. the reason is: The signal stack can be nested, i.e. trigger another signaling inside signal handler and we don't need to set pkru=3D0 multiple times (only the first time enter sigaltstack) > (please give me sometime to organize the test code, I'm hoping to send > out before the end of next week) > the test code is placed at [1] [1] https://github.com/peaktocreek/pkeytest > Also, could you please consider adding a new flag SS_PKEYALTSTACK (see > SS_AUTODISARM for example). Most existing apps that use sigaltstack() > don't use PKEY and don't care about setting PKRU=3D0, and don't need to > overwrite the sig frame after XSAVE. The flag will limit the impact > of this patch. > Thanks -Jeff > Thanks > Best regards, > -Jeff > > > > > > > /* save i387 and extended state */ > > - if (!copy_fpstate_to_sigframe(*fpstate, (void __user *)buf_fx, = math_size, pkru)) > > + if (!copy_fpstate_to_sigframe(*fpstate, (void __user *)buf_fx, = math_size, pkru)) { > > + /* > > + * Restore PKRU to the original, user-defined value; di= sable > > + * extra pkeys enabled for the alternate signal stack, = if any. > > + */ > > + write_pkru(pkru); > > return (void __user *)-1L; > > + } > > > > return (void __user *)sp; > > } > > -- > > 2.39.3 > >