lkml.org 
[lkml]   [2015]   [Jun]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [PATCH 07/12] x86/compat: get_gate_vma should check for both 32-bit compat and x32
On Mon, Jun 22, 2015 at 10:05 AM, Brian Gerst <brgerst@gmail.com> wrote:
> On Mon, Jun 22, 2015 at 12:19 PM, Andy Lutomirski <luto@amacapital.net> wrote:
>> On Mon, Jun 22, 2015 at 4:55 AM, Brian Gerst <brgerst@gmail.com> wrote:
>>> Change this to CONFIG_COMPAT so both 32-bit compat and x32 will do the
>>> check.
>>>
>>> Signed-off-by: Brian Gerst <brgerst@gmail.com>
>>> ---
>>> arch/x86/entry/vsyscall/vsyscall_64.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/arch/x86/entry/vsyscall/vsyscall_64.c b/arch/x86/entry/vsyscall/vsyscall_64.c
>>> index 2dcc6ff..26a46f4 100644
>>> --- a/arch/x86/entry/vsyscall/vsyscall_64.c
>>> +++ b/arch/x86/entry/vsyscall/vsyscall_64.c
>>> @@ -290,7 +290,7 @@ static struct vm_area_struct gate_vma = {
>>>
>>> struct vm_area_struct *get_gate_vma(struct mm_struct *mm)
>>> {
>>> -#ifdef CONFIG_IA32_EMULATION
>>> +#ifdef CONFIG_COMPAT
>>> if (!mm || mm->context.ia32_compat)
>>> return NULL;
>>> #endif
>>
>>
>> This makes little sense to me.
>>
>> First, why is the !mm check guarded by any ifdef at all? If this said
>> "if (mm && mm->...)", it would make no sense.
>>
>> Second, and more importantly, what does mm->context.ia32_compat mean
>> in the new less-nonsensical regime? The flag itself is defined in a
>> way that makes no sense (it's either 0, TIF_X32, or TIF_IA32 --
>> presumably it should be an enum). There aren't a whole lot of things
>> that care -- it's just this check and some uprobe thing. At some
>> point, mpx will start caring, too. There's also TIF_ADDR32, which is
>> similarly ridiculous (why isn't it part of mm->context?, and why does
>> it exist at all).
>>
>> I think that the questions we want to be able to answer are:
>>
>> 1. Is this mm intended to be addressable using 32 bits? If so, we
>> should probably not show the vsyscall area in /proc/self/maps.
>>
>> 2. Is this mm's mpx context intended to be used by 32-bit userspace?
>> (That's real 32 bit, not x32 -- x32 is certainly 64-bit as far as MPX
>> is concerned.)
>>
>> 3. Is the current mmap call intended to return something in the low 32
>> bits? Presumably that should depend only on the mmap call's bitness
>> (is_compat_task, etc, which we really need to rename to something like
>> in_compat_syscall).
>>
>> I find myself wondering whether there is any legitimate reason that
>> TASK_SIZE isn't the same thing as TASK_SIZE_MAX...
>>
>> Anyway, your patch is probably fine -- you're not actually making
>> anything worse.
>>
>> --Andy
>
> I agree there are more cleanups needed here. But that's probably a
> whole patch series in itself. This patch is intended to be no code
> change when X32 doesn't depend on 32-bit compat anymore.

Fair enough.

Acked-by: Andy Lutomirski <luto@kernel.org>

>
> --
> Brian Gerst



--
Andy Lutomirski
AMA Capital Management, LLC
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2015-06-22 19:41    [W:0.054 / U:0.120 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site