[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <2BA1926A-8720-4B85-8D15-557CB9A8464E@oracle.com>
Date: Tue, 11 Jun 2024 13:56:14 +0000
From: Aruna Ramakrishna <aruna.ramakrishna@...cle.com>
To: "jeffxu@...omium.org" <jeffxu@...omium.org>
CC: "dave.hansen@...ux.intel.com" <dave.hansen@...ux.intel.com>,
Keith Lucas
<keith.lucas@...cle.com>,
"linux-kernel@...r.kernel.org"
<linux-kernel@...r.kernel.org>,
Ingo Molnar <mingo@...nel.org>, Thomas
Gleixner <tglx@...utronix.de>,
"x86@...nel.org" <x86@...nel.org>,
Andrew
Brownsword <andrew.brownsword@...cle.com>,
Matthias Neugschwandtner
<matthias.neugschwandtner@...cle.com>,
"jeffxu@...gle.com"
<jeffxu@...gle.com>,
"jannh@...gle.com" <jannh@...gle.com>,
"keescook@...omium.org" <keescook@...omium.org>,
"sroettger@...gle.com"
<sroettger@...gle.com>,
"jorgelo@...omium.org" <jorgelo@...omium.org>,
"rick.p.edgecombe@...el.com" <rick.p.edgecombe@...el.com>
Subject: Re: Re [PATCH v5 1/5] x86/pkeys: Add PKRU as a parameter in signal
handling functions
> On Jun 10, 2024, at 2:31 PM, jeffxu@...omium.org wrote:
>
> On Thu, Jun 06, 2024 at 10:40:31PM +0000, Aruna Ramakrishna wrote:
>
>> diff --git a/arch/x86/include/asm/sighandling.h b/arch/x86/include/asm/sighandling.h
>> index e770c4fc47f4..de458354a3ea 100644
>> --- a/arch/x86/include/asm/sighandling.h
>> +++ b/arch/x86/include/asm/sighandling.h
>> @@ -17,11 +17,11 @@ void signal_fault(struct pt_regs *regs, void __user *frame, char *where);
>>
>> void __user *
>> get_sigframe(struct ksignal *ksig, struct pt_regs *regs, size_t frame_size,
>> - void __user **fpstate);
>> + void __user **fpstate, u32 pkru);
>>
>> -int ia32_setup_frame(struct ksignal *ksig, struct pt_regs *regs);
>> -int ia32_setup_rt_frame(struct ksignal *ksig, struct pt_regs *regs);
>> -int x64_setup_rt_frame(struct ksignal *ksig, struct pt_regs *regs);
>> -int x32_setup_rt_frame(struct ksignal *ksig, struct pt_regs *regs);
>> +int ia32_setup_frame(struct ksignal *ksig, struct pt_regs *regs, u32 pkru);
>> +int ia32_setup_rt_frame(struct ksignal *ksig, struct pt_regs *regs, u32 pkru);
>> +int x64_setup_rt_frame(struct ksignal *ksig, struct pt_regs *regs, u32 pkru);
>> +int x32_setup_rt_frame(struct ksignal *ksig, struct pt_regs *regs, u32 pkru);
>
> Does ia32 support PKEY ? I thought it is X64 only.
> It might be possible to not to change any of ia32 code.
From the Intel manual, it did not seem that pkey support was x64 only.
>
>
>>
>> #endif /* _ASM_X86_SIGHANDLING_H */
>> diff --git a/arch/x86/kernel/fpu/signal.c b/arch/x86/kernel/fpu/signal.c
>> index 247f2225aa9f..2b3b9e140dd4 100644
>> --- a/arch/x86/kernel/fpu/signal.c
>> +++ b/arch/x86/kernel/fpu/signal.c
>> @@ -156,7 +156,7 @@ static inline bool save_xstate_epilog(void __user *buf, int ia32_frame,
>> return !err;
>> }
>>
>> -static inline int copy_fpregs_to_sigframe(struct xregs_state __user *buf)
>> +static inline int copy_fpregs_to_sigframe(struct xregs_state __user *buf, u32 pkru)
>> {
>> if (use_xsave())
>> return xsave_to_user_sigframe(buf);
>> @@ -185,7 +185,7 @@ static inline int copy_fpregs_to_sigframe(struct xregs_state __user *buf)
>> * For [f]xsave state, update the SW reserved fields in the [f]xsave frame
>> * indicating the absence/presence of the extended state to the user.
>> */
>> -bool copy_fpstate_to_sigframe(void __user *buf, void __user *buf_fx, int size)
>> +bool copy_fpstate_to_sigframe(void __user *buf, void __user *buf_fx, int size, u32 pkru)
>> {
>> struct task_struct *tsk = current;
>> struct fpstate *fpstate = tsk->thread.fpu.fpstate;
>> @@ -228,7 +228,7 @@ bool copy_fpstate_to_sigframe(void __user *buf, void __user *buf_fx, int size)
>> fpregs_restore_userregs();
>>
>> pagefault_disable();
>> - ret = copy_fpregs_to_sigframe(buf_fx);
>> + ret = copy_fpregs_to_sigframe(buf_fx, pkru);
>> pagefault_enable();
>> fpregs_unlock();
>>
>> diff --git a/arch/x86/kernel/signal.c b/arch/x86/kernel/signal.c
>> index 31b6f5dddfc2..94b894437327 100644
>> --- a/arch/x86/kernel/signal.c
>> +++ b/arch/x86/kernel/signal.c
>> @@ -74,7 +74,7 @@ static inline int is_x32_frame(struct ksignal *ksig)
>> */
>> void __user *
>> get_sigframe(struct ksignal *ksig, struct pt_regs *regs, size_t frame_size,
>> - void __user **fpstate)
>> + void __user **fpstate, u32 pkru)
>> {
>> struct k_sigaction *ka = &ksig->ka;
>> int ia32_frame = is_ia32_frame(ksig);
>> @@ -139,7 +139,7 @@ get_sigframe(struct ksignal *ksig, struct pt_regs *regs, size_t frame_size,
>> }
>>
>> /* save i387 and extended state */
>> - if (!copy_fpstate_to_sigframe(*fpstate, (void __user *)buf_fx, math_size))
>> + if (!copy_fpstate_to_sigframe(*fpstate, (void __user *)buf_fx, math_size, pkru))
>
> You might find that we only need to update PKRU right before copy_fpstate_to_sigframe,
> so no need to pass pkru all the way from top.
>
The PKRU is updated in copy_fpregs_to_sigframe(), right after xsave_to_user_sigframe().
But the code that decides or cares about what pkru is written to the sigframe is handle_signal(), so
it seemed cleaner to do those checks there and pass in pkru as a parameter down the stack.
Thanks,
Aruna
Powered by blists - more mailing lists