Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1483672 > unrolled thread

[RESEND][PATCH v2 1/3] syscalls,x86 Expose arch_prctl on x86-32.

Started byKyle Huey <me@kylehuey.com>
First post2016-09-14 23:10 +0200
Last post2016-09-15 03:10 +0200
Articles 5 — 2 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [RESEND][PATCH v2 1/3] syscalls,x86 Expose arch_prctl on x86-32. Kyle Huey <me@kylehuey.com> - 2016-09-14 23:10 +0200
    Re: [RESEND][PATCH v2 1/3] syscalls,x86 Expose arch_prctl on x86-32. Dmitry Safonov <0x7f454c46@gmail.com> - 2016-09-15 00:00 +0200
      Re: [RESEND][PATCH v2 1/3] syscalls,x86 Expose arch_prctl on x86-32. Kyle Huey <me@kylehuey.com> - 2016-09-15 00:10 +0200
        Re: [RESEND][PATCH v2 1/3] syscalls,x86 Expose arch_prctl on x86-32. Dmitry Safonov <0x7f454c46@gmail.com> - 2016-09-15 00:40 +0200
          Re: [RESEND][PATCH v2 1/3] syscalls,x86 Expose arch_prctl on x86-32. Kyle Huey <me@kylehuey.com> - 2016-09-15 03:10 +0200

#1483672 — [RESEND][PATCH v2 1/3] syscalls,x86 Expose arch_prctl on x86-32.

FromKyle Huey <me@kylehuey.com>
Date2016-09-14 23:10 +0200
Subject[RESEND][PATCH v2 1/3] syscalls,x86 Expose arch_prctl on x86-32.
Message-ID<shpBo-665-33@gated-at.bofh.it>
Signed-off-by: Kyle Huey <khuey@kylehuey.com>
---
 arch/x86/entry/syscalls/syscall_32.tbl |  1 +
 arch/x86/kernel/process.c              | 80 ++++++++++++++++++++++++++++++++++
 arch/x86/kernel/process_64.c           | 66 ----------------------------
 3 files changed, 81 insertions(+), 66 deletions(-)

diff --git a/arch/x86/entry/syscalls/syscall_32.tbl b/arch/x86/entry/syscalls/syscall_32.tbl
index f848572..3b6965b 100644
--- a/arch/x86/entry/syscalls/syscall_32.tbl
+++ b/arch/x86/entry/syscalls/syscall_32.tbl
@@ -386,3 +386,4 @@
 377	i386	copy_file_range		sys_copy_file_range
 378	i386	preadv2			sys_preadv2			compat_sys_preadv2
 379	i386	pwritev2		sys_pwritev2			compat_sys_pwritev2
+380	i386	arch_prctl		sys_arch_prctl
diff --git a/arch/x86/kernel/process.c b/arch/x86/kernel/process.c
index 62c0b0e..0f857c3 100644
--- a/arch/x86/kernel/process.c
+++ b/arch/x86/kernel/process.c
@@ -20,6 +20,7 @@
 #include <linux/cpuidle.h>
 #include <trace/events/power.h>
 #include <linux/hw_breakpoint.h>
+#include <linux/syscalls.h>
 #include <asm/cpu.h>
 #include <asm/apic.h>
 #include <asm/syscalls.h>
@@ -32,6 +33,7 @@
 #include <asm/tlbflush.h>
 #include <asm/mce.h>
 #include <asm/vm86.h>
+#include <asm/prctl.h>
 
 /*
  * per-CPU TSS segments. Threads are completely 'soft' on Linux,
@@ -567,3 +569,81 @@ unsigned long get_wchan(struct task_struct *p)
 	} while (count++ < 16 && p->state != TASK_RUNNING);
 	return 0;
 }
+
+long do_arch_prctl(struct task_struct *task, int code, unsigned long arg2)
+{
+	int ret = 0;
+	int doit = task == current;
+	int is_32 = IS_ENABLED(CONFIG_IA32_EMULATION) && test_thread_flag(TIF_IA32);
+	int cpu;
+
+	switch (code) {
+#ifdef CONFIG_X86_64
+	case ARCH_SET_GS:
+		if (is_32)
+			return -EINVAL;
+		if (arg2 >= TASK_SIZE_MAX)
+			return -EPERM;
+		cpu = get_cpu();
+		task->thread.gsindex = 0;
+		task->thread.gsbase = arg2;
+		if (doit) {
+			load_gs_index(0);
+			ret = wrmsrl_safe(MSR_KERNEL_GS_BASE, arg2);
+		}
+		put_cpu();
+		break;
+	case ARCH_SET_FS:
+		if (is_32)
+			return -EINVAL;
+		/* Not strictly needed for fs, but do it for symmetry
+		   with gs */
+		if (arg2 >= TASK_SIZE_MAX)
+			return -EPERM;
+		cpu = get_cpu();
+		task->thread.fsindex = 0;
+		task->thread.fsbase = arg2;
+		if (doit) {
+			/* set the selector to 0 to not confuse __switch_to */
+			loadsegment(fs, 0);
+			ret = wrmsrl_safe(MSR_FS_BASE, arg2);
+		}
+		put_cpu();
+		break;
+	case ARCH_GET_FS: {
+		unsigned long base;
+
+		if (is_32)
+			return -EINVAL;
+		if (doit)
+			rdmsrl(MSR_FS_BASE, base);
+		else
+			base = task->thread.fsbase;
+		ret = put_user(base, (unsigned long __user *)arg2);
+		break;
+	}
+	case ARCH_GET_GS: {
+		unsigned long base;
+
+		if (is_32)
+			return -EINVAL;
+		if (doit)
+			rdmsrl(MSR_KERNEL_GS_BASE, base);
+		else
+			base = task->thread.gsbase;
+		ret = put_user(base, (unsigned long __user *)arg2);
+		break;
+	}
+#endif
+	default:
+		ret = -EINVAL;
+		break;
+	}
+
+	return ret;
+}
+
+SYSCALL_DEFINE2(arch_prctl, int, code, unsigned long, arg2)
+{
+	return do_arch_prctl(current, code, arg2);
+}
diff --git a/arch/x86/kernel/process_64.c b/arch/x86/kernel/process_64.c
index 63236d8..e8c6302 100644
--- a/arch/x86/kernel/process_64.c
+++ b/arch/x86/kernel/process_64.c
@@ -524,72 +524,6 @@ void set_personality_ia32(bool x32)
 }
 EXPORT_SYMBOL_GPL(set_personality_ia32);
 
-long do_arch_prctl(struct task_struct *task, int code, unsigned long addr)
-{
-	int ret = 0;
-	int doit = task == current;
-	int cpu;
-
-	switch (code) {
-	case ARCH_SET_GS:
-		if (addr >= TASK_SIZE_MAX)
-			return -EPERM;
-		cpu = get_cpu();
-		task->thread.gsindex = 0;
-		task->thread.gsbase = addr;
-		if (doit) {
-			load_gs_index(0);
-			ret = wrmsrl_safe(MSR_KERNEL_GS_BASE, addr);
-		}
-		put_cpu();
-		break;
-	case ARCH_SET_FS:
-		/* Not strictly needed for fs, but do it for symmetry
-		   with gs */
-		if (addr >= TASK_SIZE_MAX)
-			return -EPERM;
-		cpu = get_cpu();
-		task->thread.fsindex = 0;
-		task->thread.fsbase = addr;
-		if (doit) {
-			/* set the selector to 0 to not confuse __switch_to */
-			loadsegment(fs, 0);
-			ret = wrmsrl_safe(MSR_FS_BASE, addr);
-		}
-		put_cpu();
-		break;
-	case ARCH_GET_FS: {
-		unsigned long base;
-		if (doit)
-			rdmsrl(MSR_FS_BASE, base);
-		else
-			base = task->thread.fsbase;
-		ret = put_user(base, (unsigned long __user *)addr);
-		break;
-	}
-	case ARCH_GET_GS: {
-		unsigned long base;
-		if (doit)
-			rdmsrl(MSR_KERNEL_GS_BASE, base);
-		else
-			base = task->thread.gsbase;
-		ret = put_user(base, (unsigned long __user *)addr);
-		break;
-	}
-
-	default:
-		ret = -EINVAL;
-		break;
-	}
-
-	return ret;
-}
-
-long sys_arch_prctl(int code, unsigned long addr)
-{
-	return do_arch_prctl(current, code, addr);
-}
-
 unsigned long KSTK_ESP(struct task_struct *task)
 {
 	return task_pt_regs(task)->sp;
-- 
2.7.4

[toc] | [next] | [standalone]

#1483713

FromDmitry Safonov <0x7f454c46@gmail.com>
Date2016-09-15 00:00 +0200
Message-ID<shqnM-6nQ-17@gated-at.bofh.it>
In reply to#1483672
2016-09-15 0:08 GMT+03:00 Kyle Huey <me@kylehuey.com>:
> Signed-off-by: Kyle Huey <khuey@kylehuey.com>
> ---
>  arch/x86/entry/syscalls/syscall_32.tbl |  1 +
>  arch/x86/kernel/process.c              | 80 ++++++++++++++++++++++++++++++++++
>  arch/x86/kernel/process_64.c           | 66 ----------------------------
>  3 files changed, 81 insertions(+), 66 deletions(-)
>
> diff --git a/arch/x86/entry/syscalls/syscall_32.tbl b/arch/x86/entry/syscalls/syscall_32.tbl
> index f848572..3b6965b 100644
> --- a/arch/x86/entry/syscalls/syscall_32.tbl
> +++ b/arch/x86/entry/syscalls/syscall_32.tbl
> @@ -386,3 +386,4 @@
>  377    i386    copy_file_range         sys_copy_file_range
>  378    i386    preadv2                 sys_preadv2                     compat_sys_preadv2
>  379    i386    pwritev2                sys_pwritev2                    compat_sys_pwritev2
> +380    i386    arch_prctl              sys_arch_prctl

Why not define it as other 32-bit syscalls with compat_sys_ prefix
with the help of COMPAT_SYSCALL_DEFINE() macro?
Then you could omit code moving, drop is_32 helper.
I miss something obvious?

-- 
             Dmitry

[toc] | [prev] | [next] | [standalone]

#1483716

FromKyle Huey <me@kylehuey.com>
Date2016-09-15 00:10 +0200
Message-ID<shqxr-6H5-3@gated-at.bofh.it>
In reply to#1483713
On Wed, Sep 14, 2016 at 2:59 PM, Dmitry Safonov <0x7f454c46@gmail.com> wrote:
> 2016-09-15 0:08 GMT+03:00 Kyle Huey <me@kylehuey.com>:
>> Signed-off-by: Kyle Huey <khuey@kylehuey.com>
>> ---
>>  arch/x86/entry/syscalls/syscall_32.tbl |  1 +
>>  arch/x86/kernel/process.c              | 80 ++++++++++++++++++++++++++++++++++
>>  arch/x86/kernel/process_64.c           | 66 ----------------------------
>>  3 files changed, 81 insertions(+), 66 deletions(-)
>>
>> diff --git a/arch/x86/entry/syscalls/syscall_32.tbl b/arch/x86/entry/syscalls/syscall_32.tbl
>> index f848572..3b6965b 100644
>> --- a/arch/x86/entry/syscalls/syscall_32.tbl
>> +++ b/arch/x86/entry/syscalls/syscall_32.tbl
>> @@ -386,3 +386,4 @@
>>  377    i386    copy_file_range         sys_copy_file_range
>>  378    i386    preadv2                 sys_preadv2                     compat_sys_preadv2
>>  379    i386    pwritev2                sys_pwritev2                    compat_sys_pwritev2
>> +380    i386    arch_prctl              sys_arch_prctl
>
> Why not define it as other 32-bit syscalls with compat_sys_ prefix
> with the help of COMPAT_SYSCALL_DEFINE() macro?
> Then you could omit code moving, drop is_32 helper.
> I miss something obvious?

The code will have to move regardless, because right now do_arch_prctl
is in process-64.c which is only compiled on a 64 bit kernel.

As I told Dave Hansen in the non-RESEND thread (not sure why
git-send-email didn't put him in this one ...) I considered doing a
compat_sys_arch_prctl that would reject the relevant arch_prctls that
don't apply on 32 bit but I didn't see any prior art for it (in my
admittedly non-exhaustive search).

- Kyle

[toc] | [prev] | [next] | [standalone]

#1483735

FromDmitry Safonov <0x7f454c46@gmail.com>
Date2016-09-15 00:40 +0200
Message-ID<shr0u-6Ra-5@gated-at.bofh.it>
In reply to#1483716
2016-09-15 1:08 GMT+03:00 Kyle Huey <me@kylehuey.com>:
> On Wed, Sep 14, 2016 at 2:59 PM, Dmitry Safonov <0x7f454c46@gmail.com> wrote:
>> 2016-09-15 0:08 GMT+03:00 Kyle Huey <me@kylehuey.com>:
>>> Signed-off-by: Kyle Huey <khuey@kylehuey.com>
>>> ---
>>>  arch/x86/entry/syscalls/syscall_32.tbl |  1 +
>>>  arch/x86/kernel/process.c              | 80 ++++++++++++++++++++++++++++++++++
>>>  arch/x86/kernel/process_64.c           | 66 ----------------------------
>>>  3 files changed, 81 insertions(+), 66 deletions(-)
>>>
>>> diff --git a/arch/x86/entry/syscalls/syscall_32.tbl b/arch/x86/entry/syscalls/syscall_32.tbl
>>> index f848572..3b6965b 100644
>>> --- a/arch/x86/entry/syscalls/syscall_32.tbl
>>> +++ b/arch/x86/entry/syscalls/syscall_32.tbl
>>> @@ -386,3 +386,4 @@
>>>  377    i386    copy_file_range         sys_copy_file_range
>>>  378    i386    preadv2                 sys_preadv2                     compat_sys_preadv2
>>>  379    i386    pwritev2                sys_pwritev2                    compat_sys_pwritev2
>>> +380    i386    arch_prctl              sys_arch_prctl
>>
>> Why not define it as other 32-bit syscalls with compat_sys_ prefix
>> with the help of COMPAT_SYSCALL_DEFINE() macro?
>> Then you could omit code moving, drop is_32 helper.
>> I miss something obvious?
>
> The code will have to move regardless, because right now do_arch_prctl
> is in process-64.c which is only compiled on a 64 bit kernel.

Why? This code will not work anyway for 32-bit in your patches
by obscuring it with is_32.

> As I told Dave Hansen in the non-RESEND thread (not sure why
> git-send-email didn't put him in this one ...) I considered doing a
> compat_sys_arch_prctl that would reject the relevant arch_prctls that
> don't apply on 32 bit but I didn't see any prior art for it (in my
> admittedly non-exhaustive search).

Well, you could just add to 64-bit do_arch_prctl() new cases for your
prctls - that would be just a two-lines for each new prctl.
Also add compat_sys_ and define *only* what's needed there for you,
do not add there ARCH_{SET,GET}_{FS,GS}.
Does this make sense?

-- 
             Dmitry

[toc] | [prev] | [next] | [standalone]

#1483804

FromKyle Huey <me@kylehuey.com>
Date2016-09-15 03:10 +0200
Message-ID<shtlD-as-7@gated-at.bofh.it>
In reply to#1483735
On Wed, Sep 14, 2016 at 3:29 PM, Dmitry Safonov <0x7f454c46@gmail.com> wrote:
> 2016-09-15 1:08 GMT+03:00 Kyle Huey <me@kylehuey.com>:
>> On Wed, Sep 14, 2016 at 2:59 PM, Dmitry Safonov <0x7f454c46@gmail.com> wrote:
>>> 2016-09-15 0:08 GMT+03:00 Kyle Huey <me@kylehuey.com>:
>>>> Signed-off-by: Kyle Huey <khuey@kylehuey.com>
>>>> ---
>>>>  arch/x86/entry/syscalls/syscall_32.tbl |  1 +
>>>>  arch/x86/kernel/process.c              | 80 ++++++++++++++++++++++++++++++++++
>>>>  arch/x86/kernel/process_64.c           | 66 ----------------------------
>>>>  3 files changed, 81 insertions(+), 66 deletions(-)
>>>>
>>>> diff --git a/arch/x86/entry/syscalls/syscall_32.tbl b/arch/x86/entry/syscalls/syscall_32.tbl
>>>> index f848572..3b6965b 100644
>>>> --- a/arch/x86/entry/syscalls/syscall_32.tbl
>>>> +++ b/arch/x86/entry/syscalls/syscall_32.tbl
>>>> @@ -386,3 +386,4 @@
>>>>  377    i386    copy_file_range         sys_copy_file_range
>>>>  378    i386    preadv2                 sys_preadv2                     compat_sys_preadv2
>>>>  379    i386    pwritev2                sys_pwritev2                    compat_sys_pwritev2
>>>> +380    i386    arch_prctl              sys_arch_prctl
>>>
>>> Why not define it as other 32-bit syscalls with compat_sys_ prefix
>>> with the help of COMPAT_SYSCALL_DEFINE() macro?
>>> Then you could omit code moving, drop is_32 helper.
>>> I miss something obvious?
>>
>> The code will have to move regardless, because right now do_arch_prctl
>> is in process-64.c which is only compiled on a 64 bit kernel.
>
> Why? This code will not work anyway for 32-bit in your patches
> by obscuring it with is_32.
>
>> As I told Dave Hansen in the non-RESEND thread (not sure why
>> git-send-email didn't put him in this one ...) I considered doing a
>> compat_sys_arch_prctl that would reject the relevant arch_prctls that
>> don't apply on 32 bit but I didn't see any prior art for it (in my
>> admittedly non-exhaustive search).
>
> Well, you could just add to 64-bit do_arch_prctl() new cases for your
> prctls - that would be just a two-lines for each new prctl.
> Also add compat_sys_ and define *only* what's needed there for you,
> do not add there ARCH_{SET,GET}_{FS,GS}.
> Does this make sense?

Yeah, I should have spoken more clearly.  We'll need some
implementation of the syscall outside of process_64.c.  But we could
leave the 64 bit specific stuff behind in it.   Dave Hansen suggested
something similar (though without the compat_sys_bit)

>FWIW, I don't think it would be horrible to leave the existing
> do_arch_prctl() code in process_64.h and call it
> do_64_bit_only_something_arch_prctl(), and only call in to it from the
> generic do_arch_prctl().  You really have one reason for all the "if
> (is_32)"'s and it would be nice to document why in one single place.

- Kyle

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web