arch/x86/kernel/signal.o: warning: objtool: restore_sigcontext()+0x3cc: call to force_valid_ss.isra.5() with UACCESS enabled
XXX: move the callsite out of te AC=1 region instead?
Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
---
arch/x86/kernel/signal.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
--- a/arch/x86/kernel/signal.c
+++ b/arch/x86/kernel/signal.c
@@ -71,7 +71,7 @@
* alone. Using this generally makes no sense unless
* user_64bit_mode(regs) would return true.
*/
-static void force_valid_ss(struct pt_regs *regs)
+static __always_inline void force_valid_ss(struct pt_regs *regs)
{
u32 ar;
asm volatile ("lar %[old_ss], %[ar]\n\t"
On Thu, Mar 07, 2019 at 12:45:18PM +0100, Peter Zijlstra wrote:
> arch/x86/kernel/signal.o: warning: objtool: restore_sigcontext()+0x3cc: call to force_valid_ss.isra.5() with UACCESS enabled
>
> XXX: move the callsite out of te AC=1 region instead?
>
> Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
> ---
> arch/x86/kernel/signal.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> --- a/arch/x86/kernel/signal.c
> +++ b/arch/x86/kernel/signal.c
> @@ -71,7 +71,7 @@
> * alone. Using this generally makes no sense unless
> * user_64bit_mode(regs) would return true.
> */
> -static void force_valid_ss(struct pt_regs *regs)
> +static __always_inline void force_valid_ss(struct pt_regs *regs)
> {
> u32 ar;
> asm volatile ("lar %[old_ss], %[ar]\n\t"
arch/x86/kernel/signal.o: warning: objtool: do_signal()+0x384: call to frame_uc_flags.isra.0() with UACCESS enabled
@@ -441,7 +441,7 @@ static int __setup_rt_frame(int sig, str
return 0;
}
#else /* !CONFIG_X86_32 */
-static unsigned long frame_uc_flags(struct pt_regs *regs)
+static __always_inline unsigned long frame_uc_flags(struct pt_regs *regs)
{
unsigned long flags;
On Thu, Mar 7, 2019 at 3:52 AM Peter Zijlstra <[email protected]> wrote:
>
> arch/x86/kernel/signal.o: warning: objtool: restore_sigcontext()+0x3cc: call to force_valid_ss.isra.5() with UACCESS enabled
>
> XXX: move the callsite out of te AC=1 region instead?
Let's move it instead. I looked at the code and it should be fine.
--Andy
On Thu, Mar 07, 2019 at 10:05:43AM -0800, Andy Lutomirski wrote:
> On Thu, Mar 7, 2019 at 3:52 AM Peter Zijlstra <[email protected]> wrote:
> >
> > arch/x86/kernel/signal.o: warning: objtool: restore_sigcontext()+0x3cc: call to force_valid_ss.isra.5() with UACCESS enabled
> >
> > XXX: move the callsite out of te AC=1 region instead?
>
> Let's move it instead. I looked at the code and it should be fine.
Something like so; or did I miss something subtle?
--- a/arch/x86/kernel/signal.c
+++ b/arch/x86/kernel/signal.c
@@ -132,16 +132,6 @@ static int restore_sigcontext(struct pt_
COPY_SEG_CPL3(cs);
COPY_SEG_CPL3(ss);
-#ifdef CONFIG_X86_64
- /*
- * Fix up SS if needed for the benefit of old DOSEMU and
- * CRIU.
- */
- if (unlikely(!(uc_flags & UC_STRICT_RESTORE_SS) &&
- user_64bit_mode(regs)))
- force_valid_ss(regs);
-#endif
-
get_user_ex(tmpflags, &sc->flags);
regs->flags = (regs->flags & ~FIX_EFLAGS) | (tmpflags & FIX_EFLAGS);
regs->orig_ax = -1; /* disable syscall checks */
@@ -150,6 +140,15 @@ static int restore_sigcontext(struct pt_
buf = (void __user *)buf_val;
} get_user_catch(err);
+#ifdef CONFIG_X86_64
+ /*
+ * Fix up SS if needed for the benefit of old DOSEMU and
+ * CRIU.
+ */
+ if (unlikely(!(uc_flags & UC_STRICT_RESTORE_SS) && user_64bit_mode(regs)))
+ force_valid_ss(regs);
+#endif
+
err |= fpu__restore_sig(buf, IS_ENABLED(CONFIG_X86_32));
force_iret();
@@ -441,7 +440,7 @@ static int __setup_rt_frame(int sig, str
return 0;
}
#else /* !CONFIG_X86_32 */
-static unsigned long frame_uc_flags(struct pt_regs *regs)
+static __always_inline unsigned long frame_uc_flags(struct pt_regs *regs)
{
unsigned long flags;
@@ -461,6 +460,7 @@ static int __setup_rt_frame(int sig, str
{
struct rt_sigframe __user *frame;
void __user *fp = NULL;
+ unsigned long uc_flags;
int err = 0;
frame = get_sigframe(&ksig->ka, regs, sizeof(struct rt_sigframe), &fp);
@@ -473,9 +473,11 @@ static int __setup_rt_frame(int sig, str
return -EFAULT;
}
+ uc_flags = frame_uc_flags(regs);
+
put_user_try {
/* Create the ucontext. */
- put_user_ex(frame_uc_flags(regs), &frame->uc.uc_flags);
+ put_user_ex(uc_flags, &frame->uc.uc_flags);
put_user_ex(0, &frame->uc.uc_link);
save_altstack_ex(&frame->uc.uc_stack, regs->sp);
On Thu, Mar 7, 2019 at 10:59 AM Peter Zijlstra <[email protected]> wrote:
>
> On Thu, Mar 07, 2019 at 10:05:43AM -0800, Andy Lutomirski wrote:
> > On Thu, Mar 7, 2019 at 3:52 AM Peter Zijlstra <[email protected]> wrote:
> > >
> > > arch/x86/kernel/signal.o: warning: objtool: restore_sigcontext()+0x3cc: call to force_valid_ss.isra.5() with UACCESS enabled
> > >
> > > XXX: move the callsite out of te AC=1 region instead?
> >
> > Let's move it instead. I looked at the code and it should be fine.
>
> Something like so; or did I miss something subtle?
Reviewed-by: Andy Lutomirski <[email protected]>
Also, this stuff is pretty well covered by the x86 selftests, mostly
because getting it right in the first place was way too subtle for
comfort.
--Andy