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

Groups > linux.kernel > #1290543 > unrolled thread

Re: [PATCH] x86/signal: fix restart_syscall number for x32 tasks

Started by"Dmitry V. Levin" <ldv@altlinux.org>
First post2015-12-13 04:50 +0100
Last post2015-12-19 21:50 +0100
Articles 6 — 3 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

  Re: [PATCH] x86/signal: fix restart_syscall number for x32 tasks "Dmitry V. Levin" <ldv@altlinux.org> - 2015-12-13 04:50 +0100
    Re: [PATCH] x86/signal: fix restart_syscall number for x32 tasks Andy Lutomirski <luto@amacapital.net> - 2015-12-17 21:30 +0100
      [PATCH] x86/signal: Cleanup get_nr_restart_syscall "Dmitry V. Levin" <ldv@altlinux.org> - 2015-12-19 00:40 +0100
        Re: [PATCH] x86/signal: Cleanup get_nr_restart_syscall "H. Peter Anvin" <hpa@zytor.com> - 2015-12-19 07:40 +0100
          [PATCH v2] x86/signal: Cleanup get_nr_restart_syscall "Dmitry V. Levin" <ldv@altlinux.org> - 2015-12-19 15:50 +0100
            Re: [PATCH v2] x86/signal: Cleanup get_nr_restart_syscall Andy Lutomirski <luto@amacapital.net> - 2015-12-19 21:50 +0100

#1290543 — Re: [PATCH] x86/signal: fix restart_syscall number for x32 tasks

From"Dmitry V. Levin" <ldv@altlinux.org>
Date2015-12-13 04:50 +0100
SubjectRe: [PATCH] x86/signal: fix restart_syscall number for x32 tasks
Message-ID<qF5Pz-30P-1@gated-at.bofh.it>
On Mon, Dec 07, 2015 at 03:22:06PM -0800, Andy Lutomirski wrote:
> [not real reply because I'm using a bad internet connection right now
> and I'm not set up with my usual Gmane reply hack right now]
> 
> The new code is (whitespace-damaged):
> 
> static inline unsigned long get_nr_restart_syscall(const struct pt_regs *regs)
> {
> #if defined(CONFIG_X86_32) || !defined(CONFIG_X86_64)
>  return __NR_restart_syscall;
> #else /* !CONFIG_X86_32 && CONFIG_X86_64 */
>  return test_thread_flag(TIF_IA32) ? __NR_ia32_restart_syscall :
>  __NR_restart_syscall | (regs->orig_ax & __X32_SYSCALL_BIT);
> #endif /* CONFIG_X86_32 || !CONFIG_X86_64 */
> }
> 
> This is IMO awful.  This use of TIF_IA32 is wrong, and this is
> otherwise gross.  Can we do it for real:
> 
> if (is_ia32_task())
>   return __NR_ia32_restart_syscall;
> else
>   return __NR_restart_syscall | (regs->orig_ax & __X32_SYSCALL_BIT);
> /* preserve x32 bit */
> 
> I'd send the patch myself, but you apparently have a good test case
> for this, and I don't.

Unfortunately, this won't compile on CONFIG_X86_32 because
__NR_ia32_restart_syscall is defined for CONFIG_X86_64 only.

Something like this should work:

static inline unsigned long get_nr_restart_syscall(const struct pt_regs *regs)
{
#ifdef CONFIG_X86_64
	if (is_ia32_task())
		return __NR_ia32_restart_syscall;
# ifdef CONFIG_X86_X32_ABI
	if (regs->orig_ax & __X32_SYSCALL_BIT)
		return __NR_restart_syscall | __X32_SYSCALL_BIT;
# endif
#endif
	return __NR_restart_syscall;
}

I don't see any way to avoid ifdefs here, sorry.


-- 
ldv
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1294246

FromAndy Lutomirski <luto@amacapital.net>
Date2015-12-17 21:30 +0100
Message-ID<qGNlw-4hK-13@gated-at.bofh.it>
In reply to#1290543
On Sat, Dec 12, 2015 at 7:44 PM, Dmitry V. Levin <ldv@altlinux.org> wrote:
> On Mon, Dec 07, 2015 at 03:22:06PM -0800, Andy Lutomirski wrote:
>> [not real reply because I'm using a bad internet connection right now
>> and I'm not set up with my usual Gmane reply hack right now]
>>
>> The new code is (whitespace-damaged):
>>
>> static inline unsigned long get_nr_restart_syscall(const struct pt_regs *regs)
>> {
>> #if defined(CONFIG_X86_32) || !defined(CONFIG_X86_64)
>>  return __NR_restart_syscall;
>> #else /* !CONFIG_X86_32 && CONFIG_X86_64 */
>>  return test_thread_flag(TIF_IA32) ? __NR_ia32_restart_syscall :
>>  __NR_restart_syscall | (regs->orig_ax & __X32_SYSCALL_BIT);
>> #endif /* CONFIG_X86_32 || !CONFIG_X86_64 */
>> }
>>
>> This is IMO awful.  This use of TIF_IA32 is wrong, and this is
>> otherwise gross.  Can we do it for real:
>>
>> if (is_ia32_task())
>>   return __NR_ia32_restart_syscall;
>> else
>>   return __NR_restart_syscall | (regs->orig_ax & __X32_SYSCALL_BIT);
>> /* preserve x32 bit */
>>
>> I'd send the patch myself, but you apparently have a good test case
>> for this, and I don't.
>
> Unfortunately, this won't compile on CONFIG_X86_32 because
> __NR_ia32_restart_syscall is defined for CONFIG_X86_64 only.
>
> Something like this should work:
>
> static inline unsigned long get_nr_restart_syscall(const struct pt_regs *regs)
> {
> #ifdef CONFIG_X86_64
>         if (is_ia32_task())
>                 return __NR_ia32_restart_syscall;
> # ifdef CONFIG_X86_X32_ABI
>         if (regs->orig_ax & __X32_SYSCALL_BIT)
>                 return __NR_restart_syscall | __X32_SYSCALL_BIT;
> # endif
> #endif
>         return __NR_restart_syscall;
> }

Looks good to me.  Want to send a patch?

--Andy
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

#1295242 — [PATCH] x86/signal: Cleanup get_nr_restart_syscall

From"Dmitry V. Levin" <ldv@altlinux.org>
Date2015-12-19 00:40 +0100
Subject[PATCH] x86/signal: Cleanup get_nr_restart_syscall
Message-ID<qHcMV-3PJ-3@gated-at.bofh.it>
In reply to#1294246
Check for TS_COMPAT instead of TIF_IA32 to distinguish ia32 tasks
from 64-bit tasks.
Check for __X32_SYSCALL_BIT only if CONFIG_X86_X32_ABI is defined.

Signed-off-by: Dmitry V. Levin <ldv@altlinux.org>
Cc: Elvira Khabirova <lineprinter0@gmail.com>
Cc: Andy Lutomirski <luto@amacapital.net>
---
 arch/x86/kernel/signal.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/arch/x86/kernel/signal.c b/arch/x86/kernel/signal.c
index cb6282c..ff7dedc 100644
--- a/arch/x86/kernel/signal.c
+++ b/arch/x86/kernel/signal.c
@@ -692,12 +692,15 @@ handle_signal(struct ksignal *ksig, struct pt_regs *regs)
 
 static inline unsigned long get_nr_restart_syscall(const struct pt_regs *regs)
 {
-#if defined(CONFIG_X86_32) || !defined(CONFIG_X86_64)
+#ifdef CONFIG_X86_64
+	if (is_ia32_task())
+		return __NR_ia32_restart_syscall;
+# ifdef CONFIG_X86_X32_ABI
+	if (regs->orig_ax & __X32_SYSCALL_BIT)
+		return __NR_restart_syscall | __X32_SYSCALL_BIT;
+# endif
+#endif
 	return __NR_restart_syscall;
-#else /* !CONFIG_X86_32 && CONFIG_X86_64 */
-	return test_thread_flag(TIF_IA32) ? __NR_ia32_restart_syscall :
-		__NR_restart_syscall | (regs->orig_ax & __X32_SYSCALL_BIT);
-#endif /* CONFIG_X86_32 || !CONFIG_X86_64 */
 }
 
 /*
-- 
ldv
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

#1295317 — Re: [PATCH] x86/signal: Cleanup get_nr_restart_syscall

From"H. Peter Anvin" <hpa@zytor.com>
Date2015-12-19 07:40 +0100
SubjectRe: [PATCH] x86/signal: Cleanup get_nr_restart_syscall
Message-ID<qHjln-85O-7@gated-at.bofh.it>
In reply to#1295242
On 12/18/15 15:37, Dmitry V. Levin wrote:
> Check for TS_COMPAT instead of TIF_IA32 to distinguish ia32 tasks
> from 64-bit tasks.
> Check for __X32_SYSCALL_BIT only if CONFIG_X86_X32_ABI is defined.
>
> Signed-off-by: Dmitry V. Levin <ldv@altlinux.org>
> Cc: Elvira Khabirova <lineprinter0@gmail.com>
> Cc: Andy Lutomirski <luto@amacapital.net>
> ---
>   arch/x86/kernel/signal.c | 13 ++++++++-----
>   1 file changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/arch/x86/kernel/signal.c b/arch/x86/kernel/signal.c
> index cb6282c..ff7dedc 100644
> --- a/arch/x86/kernel/signal.c
> +++ b/arch/x86/kernel/signal.c
> @@ -692,12 +692,15 @@ handle_signal(struct ksignal *ksig, struct pt_regs *regs)
>
>   static inline unsigned long get_nr_restart_syscall(const struct pt_regs *regs)
>   {
> -#if defined(CONFIG_X86_32) || !defined(CONFIG_X86_64)
> +#ifdef CONFIG_X86_64
> +	if (is_ia32_task())
> +		return __NR_ia32_restart_syscall;
> +# ifdef CONFIG_X86_X32_ABI
> +	if (regs->orig_ax & __X32_SYSCALL_BIT)
> +		return __NR_restart_syscall | __X32_SYSCALL_BIT;
> +# endif
> +#endif
>   	return __NR_restart_syscall;
> -#else /* !CONFIG_X86_32 && CONFIG_X86_64 */
> -	return test_thread_flag(TIF_IA32) ? __NR_ia32_restart_syscall :
> -		__NR_restart_syscall | (regs->orig_ax & __X32_SYSCALL_BIT);
> -#endif /* CONFIG_X86_32 || !CONFIG_X86_64 */
>   }
>
>   /*
>

I bet you actually made the code slower.

	-hpa

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

#1295382 — [PATCH v2] x86/signal: Cleanup get_nr_restart_syscall

From"Dmitry V. Levin" <ldv@altlinux.org>
Date2015-12-19 15:50 +0100
Subject[PATCH v2] x86/signal: Cleanup get_nr_restart_syscall
Message-ID<qHqZA-4rt-11@gated-at.bofh.it>
In reply to#1295317
Check for TS_COMPAT instead of TIF_IA32 to distinguish ia32 tasks
from 64-bit tasks.
Check for __X32_SYSCALL_BIT iff CONFIG_X86_X32_ABI is defined.

Signed-off-by: Dmitry V. Levin <ldv@altlinux.org>
Cc: Elvira Khabirova <lineprinter0@gmail.com>
Cc: Andy Lutomirski <luto@amacapital.net>
---
 v2: reintroduced __NR_restart_syscall | (regs->orig_ax & __X32_SYSCALL_BIT)

 arch/x86/kernel/signal.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/arch/x86/kernel/signal.c b/arch/x86/kernel/signal.c
index cb6282c..c07ff5d 100644
--- a/arch/x86/kernel/signal.c
+++ b/arch/x86/kernel/signal.c
@@ -692,12 +692,15 @@ handle_signal(struct ksignal *ksig, struct pt_regs *regs)
 
 static inline unsigned long get_nr_restart_syscall(const struct pt_regs *regs)
 {
-#if defined(CONFIG_X86_32) || !defined(CONFIG_X86_64)
+#ifdef CONFIG_X86_64
+	if (is_ia32_task())
+		return __NR_ia32_restart_syscall;
+#endif
+#ifdef CONFIG_X86_X32_ABI
+	return __NR_restart_syscall | (regs->orig_ax & __X32_SYSCALL_BIT);
+#else
 	return __NR_restart_syscall;
-#else /* !CONFIG_X86_32 && CONFIG_X86_64 */
-	return test_thread_flag(TIF_IA32) ? __NR_ia32_restart_syscall :
-		__NR_restart_syscall | (regs->orig_ax & __X32_SYSCALL_BIT);
-#endif /* CONFIG_X86_32 || !CONFIG_X86_64 */
+#endif
 }
 
 /*
-- 
ldv
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

#1295461 — Re: [PATCH v2] x86/signal: Cleanup get_nr_restart_syscall

FromAndy Lutomirski <luto@amacapital.net>
Date2015-12-19 21:50 +0100
SubjectRe: [PATCH v2] x86/signal: Cleanup get_nr_restart_syscall
Message-ID<qHwBY-814-13@gated-at.bofh.it>
In reply to#1295382
On Sat, Dec 19, 2015 at 6:43 AM, Dmitry V. Levin <ldv@altlinux.org> wrote:
> Check for TS_COMPAT instead of TIF_IA32 to distinguish ia32 tasks
> from 64-bit tasks.
> Check for __X32_SYSCALL_BIT iff CONFIG_X86_X32_ABI is defined.

LGTM.

--Andy
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web