On Thu, Oct 31, 2024 at 1:45 AM Kevin Brodsky kevin.brodsky@arm.com wrote:
On 30/10/2024 23:01, Jeff Xu wrote:
-static int restore_poe_context(struct user_ctxs *user) +static int restore_poe_context(struct user_ctxs *user,
struct user_access_state *ua_state){ u64 por_el0; int err = 0; @@ -282,7 +338,7 @@ static int restore_poe_context(struct user_ctxs *user)
__get_user_error(por_el0, &(user->poe->por_el0), err); if (!err)
write_sysreg_s(por_el0, SYS_POR_EL0);
ua_state->por_el0 = por_el0; return err;} @@ -850,7 +906,8 @@ static int parse_user_sigframe(struct user_ctxs *user, }
static int restore_sigframe(struct pt_regs *regs,
struct rt_sigframe __user *sf)
struct rt_sigframe __user *sf,struct user_access_state *ua_state){ sigset_t set; int i, err; @@ -899,7 +956,7 @@ static int restore_sigframe(struct pt_regs *regs, err = restore_zt_context(&user);
if (err == 0 && system_supports_poe() && user.poe)
err = restore_poe_context(&user);
err = restore_poe_context(&user, ua_state); return err;} @@ -908,6 +965,7 @@ SYSCALL_DEFINE0(rt_sigreturn) { struct pt_regs *regs = current_pt_regs(); struct rt_sigframe __user *frame;
struct user_access_state ua_state; /* Always make any pending restarted system calls return -EINTR */ current->restart_block.fn = do_no_restart_syscall;@@ -924,12 +982,14 @@ SYSCALL_DEFINE0(rt_sigreturn) if (!access_ok(frame, sizeof (*frame))) goto badframe;
if (restore_sigframe(regs, frame))
if (restore_sigframe(regs, frame, &ua_state)) goto badframe; if (restore_altstack(&frame->uc.uc_stack)) goto badframe;Do you need to move restore_altstack ahead of restore_sigframe?
This is not necessary because restore_sigframe() no longer writes to POR_EL0. restore_poe_context() (above) now saves the original POR_EL0 value into ua_state, and it is restore_user_access_state() (called below just before returning to userspace) that actually writes to POR_EL0, after all uaccess is completed.
Got it, thanks for the explanation.
-Jeff
Having said that, I somehow missed the call to restore_altstack() when writing the commit message, so these changes in sys_rt_sigreturn are in fact necessary. Good catch! At least the patch itself should be doing the right thing.
- Kevin
similar as x86 change [1], the discussion for this happened in [2] [3]
[1] https://lore.kernel.org/lkml/20240802061318.2140081-5-aruna.ramakrishna@orac... [2] https://lore.kernel.org/lkml/20240425210540.3265342-1-jeffxu@chromium.org/ [3] https://lore.kernel.org/lkml/d0162c76c25bc8e1c876aebe8e243ff2e6862359.camel@...
Thanks -Jeff
restore_user_access_state(&ua_state);return regs->regs[0];