lkml.org 
[lkml]   [2020]   [Jul]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 2/6] arch: x86: Wrap TIF_IA32 checks
On Tue, Jul 28, 2020 at 08:43:27PM -0700, Andy Lutomirski wrote:
> > index d4edf281fff4..d39f9b3ae683 100644
> > --- a/arch/x86/include/asm/compat.h
> > +++ b/arch/x86/include/asm/compat.h
> > @@ -181,7 +181,7 @@ static inline void __user *arch_compat_alloc_user_space(long len)
> > {
> > compat_uptr_t sp;
> >
> > - if (test_thread_flag(TIF_IA32)) {
> > + if (TASK_IA32(current)) {
> > sp = task_pt_regs(current)->sp;
>
> Christoph, you spend a *lot* more time looking at this stuff lately
> than I do, but this looks totally wrong. Shouldn't this be either:
>
> sp = task_pt_regs(current)->sp;
>
> /* This might be a compat syscall issued via int $0x80 from 64-bit-ABI code. */
> if (user_64bit_mode(task_pt_regs(current))
> sp -= 128;
>
> Or perhaps the same thing without the user_64bit_mode() check at all?
> There shouldn't be much if any harm done by respecting the redzone
> unnecessarily.

compat_alloc_user_space is only used when either called from compat
calls or if in_compat_syscall() is true (and there are very few callers
left, and we plan to kill it off entirely..).

Which means we are either called from an i386 or x32 syscall, but then
again IIRC user_64bit_mode would also return true for x32. So your
above version looks correct, and I'd also be tempted to just always
respect the redzone.

> Surely this should be:
>
> if (user_64bit_mode(task_pt_regs(regs))

s/regs/current/

Btw, I wonder if want a shorthand for

user_64bit_mode(task_pt_regs(thread))

instead of always open coding it.

\
 
 \ /
  Last update: 2020-07-29 09:01    [W:0.141 / U:0.108 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site