Patchwork [v15,04/13] arch/x86: add syscall_get_arch to syscall.h

login
register
mail settings
Submitter Will Drewry
Date March 15, 2012, 3:11 a.m.
Message ID <1331781125-15658-5-git-send-email-wad@chromium.org>
Download mbox | patch
Permalink /patch/146832/
State Not Applicable
Delegated to: David Miller
Headers show

Comments

Will Drewry - March 15, 2012, 3:11 a.m.
Add syscall_get_arch() to export the current AUDIT_ARCH_* based on system call
entry path.

Acked-by: Serge Hallyn <serge.hallyn@canonical.com>
Signed-off-by: Will Drewry <wad@chromium.org>

v14: rebase/nochanges
v13: rebase on to 88ebdda6159ffc15699f204c33feb3e431bf9bdc
---
 arch/x86/include/asm/syscall.h |   23 +++++++++++++++++++++++
 1 files changed, 23 insertions(+), 0 deletions(-)
H. Peter Anvin - March 25, 2012, 7:34 p.m.
On 03/14/2012 08:11 PM, Will Drewry wrote:
>  
> +static inline int syscall_get_arch(struct task_struct *task,
> +				   struct pt_regs *regs)
> +{
> +#ifdef CONFIG_IA32_EMULATION
> +	/*
> +	 * TS_COMPAT is set for 32-bit syscall entries and then
> +	 * remains set until we return to user mode.
> +	 *
> +	 * TIF_IA32 tasks should always have TS_COMPAT set at
> +	 * system call time.
> +	 */
> +	if (task_thread_info(task)->status & TS_COMPAT)
> +		return AUDIT_ARCH_I386;
> +#endif
> +	return AUDIT_ARCH_X86_64;
> +}
>  #endif	/* CONFIG_X86_32 */
>  
>  #endif	/* _ASM_X86_SYSCALL_H */

Just one FYI on this: after the x32 changes are upstream this can be
implemented in terms of is_ia32_task().

	-hpa
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Will Drewry - April 11, 2012, 3:13 a.m.
On Sun, Mar 25, 2012 at 2:34 PM, H. Peter Anvin <hpa@zytor.com> wrote:
> On 03/14/2012 08:11 PM, Will Drewry wrote:
>>
>> +static inline int syscall_get_arch(struct task_struct *task,
>> +                                struct pt_regs *regs)
>> +{
>> +#ifdef CONFIG_IA32_EMULATION
>> +     /*
>> +      * TS_COMPAT is set for 32-bit syscall entries and then
>> +      * remains set until we return to user mode.
>> +      *
>> +      * TIF_IA32 tasks should always have TS_COMPAT set at
>> +      * system call time.
>> +      */
>> +     if (task_thread_info(task)->status & TS_COMPAT)
>> +             return AUDIT_ARCH_I386;
>> +#endif
>> +     return AUDIT_ARCH_X86_64;
>> +}
>>  #endif       /* CONFIG_X86_32 */
>>
>>  #endif       /* _ASM_X86_SYSCALL_H */
>
> Just one FYI on this: after the x32 changes are upstream this can be
> implemented in terms of is_ia32_task().

Now that I've seen is_ia32_task(), it appears to be exactly the same as above:
(1)  If we're x86_32, it's ia32
(2)  If we're x86_64, ia32 == !!(status & TS_COMPAT)
(3)  Otherwise, it's x86_64, including x32

Am I missing something? Should is_ia32_task(void) take a task_struct?
Right now, I don't see any reason to change the code, as posted, but
maybe I am mis-reading?

thanks!
will
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
H. Peter Anvin - April 11, 2012, 3:16 a.m.
On 04/10/2012 08:13 PM, Will Drewry wrote:
> 
> Now that I've seen is_ia32_task(), it appears to be exactly the same as above:
> (1)  If we're x86_32, it's ia32
> (2)  If we're x86_64, ia32 == !!(status & TS_COMPAT)
> (3)  Otherwise, it's x86_64, including x32
> 
> Am I missing something? Should is_ia32_task(void) take a task_struct?
> Right now, I don't see any reason to change the code, as posted, but
> maybe I am mis-reading?
> 

is_compat_task() is true for x32, is_ia32_task() is false.

	-hpa

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
H. Peter Anvin - April 11, 2012, 3:20 a.m.
On 04/10/2012 08:13 PM, Will Drewry wrote:
> On Sun, Mar 25, 2012 at 2:34 PM, H. Peter Anvin <hpa@zytor.com> wrote:
>> On 03/14/2012 08:11 PM, Will Drewry wrote:
>>>
>>> +static inline int syscall_get_arch(struct task_struct *task,
>>> +                                struct pt_regs *regs)
>>> +{
>>> +#ifdef CONFIG_IA32_EMULATION
>>> +     /*
>>> +      * TS_COMPAT is set for 32-bit syscall entries and then
>>> +      * remains set until we return to user mode.
>>> +      *
>>> +      * TIF_IA32 tasks should always have TS_COMPAT set at
>>> +      * system call time.
>>> +      */
>>> +     if (task_thread_info(task)->status & TS_COMPAT)
>>> +             return AUDIT_ARCH_I386;
>>> +#endif
>>> +     return AUDIT_ARCH_X86_64;
>>> +}
>>>  #endif       /* CONFIG_X86_32 */
>>>
>>>  #endif       /* _ASM_X86_SYSCALL_H */
>>
>> Just one FYI on this: after the x32 changes are upstream this can be
>> implemented in terms of is_ia32_task().
> 
> Now that I've seen is_ia32_task(), it appears to be exactly the same as above:
> (1)  If we're x86_32, it's ia32
> (2)  If we're x86_64, ia32 == !!(status & TS_COMPAT)
> (3)  Otherwise, it's x86_64, including x32
> 
> Am I missing something? Should is_ia32_task(void) take a task_struct?
> Right now, I don't see any reason to change the code, as posted, but
> maybe I am mis-reading?
> 

Sorry, answered the wrong question.  Yes, it is the same as above...
just wandered if we could centralize this test.  It might indeed make
sense to provide general predicates which take a task pointer.

	-hpa

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Will Drewry - April 11, 2012, 3:41 p.m.
On Tue, Apr 10, 2012 at 10:20 PM, H. Peter Anvin <hpa@zytor.com> wrote:
> On 04/10/2012 08:13 PM, Will Drewry wrote:
>> On Sun, Mar 25, 2012 at 2:34 PM, H. Peter Anvin <hpa@zytor.com> wrote:
>>> On 03/14/2012 08:11 PM, Will Drewry wrote:
>>>>
>>>> +static inline int syscall_get_arch(struct task_struct *task,
>>>> +                                struct pt_regs *regs)
>>>> +{
>>>> +#ifdef CONFIG_IA32_EMULATION
>>>> +     /*
>>>> +      * TS_COMPAT is set for 32-bit syscall entries and then
>>>> +      * remains set until we return to user mode.
>>>> +      *
>>>> +      * TIF_IA32 tasks should always have TS_COMPAT set at
>>>> +      * system call time.
>>>> +      */
>>>> +     if (task_thread_info(task)->status & TS_COMPAT)
>>>> +             return AUDIT_ARCH_I386;
>>>> +#endif
>>>> +     return AUDIT_ARCH_X86_64;
>>>> +}
>>>>  #endif       /* CONFIG_X86_32 */
>>>>
>>>>  #endif       /* _ASM_X86_SYSCALL_H */
>>>
>>> Just one FYI on this: after the x32 changes are upstream this can be
>>> implemented in terms of is_ia32_task().
>>
>> Now that I've seen is_ia32_task(), it appears to be exactly the same as above:
>> (1)  If we're x86_32, it's ia32
>> (2)  If we're x86_64, ia32 == !!(status & TS_COMPAT)
>> (3)  Otherwise, it's x86_64, including x32
>>
>> Am I missing something? Should is_ia32_task(void) take a task_struct?
>> Right now, I don't see any reason to change the code, as posted, but
>> maybe I am mis-reading?
>>
>
> Sorry, answered the wrong question.  Yes, it is the same as above...
> just wandered if we could centralize this test.  It might indeed make
> sense to provide general predicates which take a task pointer.

Makes sense to me. I'm leaving this specific patch alone at present.

That said, a quick grep shows only  a handful of ia32 references:
./arch/x86/include/asm/compat.h:        return is_ia32_task() || is_x32_task();
./arch/x86/ia32/ia32_signal.c:  bool ia32 = is_ia32_task();
./arch/x86/kernel/ptrace.c:     if (!is_ia32_task())

Would it make sense to make a new predicate or just expand the one
added in 3.4 to take a task_struct parameter? I'm not sure if there'd
be much fallout in converting these from directly checking
current_thread_info to task_thread_info.

It's a small patch either way.

cheers!
will
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/arch/x86/include/asm/syscall.h b/arch/x86/include/asm/syscall.h
index d962e56..1d713e4 100644
--- a/arch/x86/include/asm/syscall.h
+++ b/arch/x86/include/asm/syscall.h
@@ -13,6 +13,7 @@ 
 #ifndef _ASM_X86_SYSCALL_H
 #define _ASM_X86_SYSCALL_H
 
+#include <linux/audit.h>
 #include <linux/sched.h>
 #include <linux/err.h>
 #include <asm/asm-offsets.h>	/* For NR_syscalls */
@@ -87,6 +88,12 @@  static inline void syscall_set_arguments(struct task_struct *task,
 	memcpy(&regs->bx + i, args, n * sizeof(args[0]));
 }
 
+static inline int syscall_get_arch(struct task_struct *task,
+				   struct pt_regs *regs)
+{
+	return AUDIT_ARCH_I386;
+}
+
 #else	 /* CONFIG_X86_64 */
 
 static inline void syscall_get_arguments(struct task_struct *task,
@@ -211,6 +218,22 @@  static inline void syscall_set_arguments(struct task_struct *task,
 		}
 }
 
+static inline int syscall_get_arch(struct task_struct *task,
+				   struct pt_regs *regs)
+{
+#ifdef CONFIG_IA32_EMULATION
+	/*
+	 * TS_COMPAT is set for 32-bit syscall entries and then
+	 * remains set until we return to user mode.
+	 *
+	 * TIF_IA32 tasks should always have TS_COMPAT set at
+	 * system call time.
+	 */
+	if (task_thread_info(task)->status & TS_COMPAT)
+		return AUDIT_ARCH_I386;
+#endif
+	return AUDIT_ARCH_X86_64;
+}
 #endif	/* CONFIG_X86_32 */
 
 #endif	/* _ASM_X86_SYSCALL_H */