[PATCH v3] microblaze: Fix clone syscall

[PATCH v3] microblaze: Fix clone syscall

[Michal Simek]

[-- Attachment #1: Type: text/plain, Size: 3029 bytes --]

Fix inadvertent breakage in the clone syscall ABI for Microblaze
that was introduced in this patch:
"microblaze: switch to generic fork/vfork/clone"
(sha1: f3268edbe6fe0ce56e62c6d6b14640aeb04864b7)

The Microblaze syscall ABI for clone takes the parent tid address in
the 4th argument; the third argument slot is used for the stack size.
The incorrectly-used CLONE_BACKWARDS type assigned parent tid to the
3rd slot.

This commit restores the original ABI so that existing userspace libc
code will work correctly.

All kernel versions from v3.8-rc1 were affected.

Signed-off-by: Michal Simek <michal.simek@xilinx.com>

---
Changes in v3:
 - Change patch description based on Rich comments.

Changes in v2:
 - Add information about the first broken patch - Suggested by Andrew

 arch/Kconfig             | 6 ++++++
 arch/microblaze/Kconfig  | 2 +-
 include/linux/syscalls.h | 5 +++++
 kernel/fork.c            | 6 ++++++
 4 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/arch/Kconfig b/arch/Kconfig
index 8d2ae24..1feb169 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -407,6 +407,12 @@ config CLONE_BACKWARDS2
 	help
 	  Architecture has the first two arguments of clone(2) swapped.

+config CLONE_BACKWARDS3
+	bool
+	help
+	  Architecture has tls passed as the 3rd argument of clone(2),
+	  not the 5th one.
+
 config ODD_RT_SIGACTION
 	bool
 	help
diff --git a/arch/microblaze/Kconfig b/arch/microblaze/Kconfig
index d22a4ec..4fab522 100644
--- a/arch/microblaze/Kconfig
+++ b/arch/microblaze/Kconfig
@@ -28,7 +28,7 @@ config MICROBLAZE
 	select GENERIC_CLOCKEVENTS
 	select GENERIC_IDLE_POLL_SETUP
 	select MODULES_USE_ELF_RELA
-	select CLONE_BACKWARDS
+	select CLONE_BACKWARDS3

 config SWAP
 	def_bool n
diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
index 4147d70..71d8931 100644
--- a/include/linux/syscalls.h
+++ b/include/linux/syscalls.h
@@ -802,9 +802,14 @@ asmlinkage long sys_vfork(void);
 asmlinkage long sys_clone(unsigned long, unsigned long, int __user *, int,
 	       int __user *);
 #else
+#if CONFIG_CLONE_BACKWARDS3
+asmlinkage long sys_clone(unsigned long, unsigned long, int, int __user *,
+			  int __user *, int);
+#else
 asmlinkage long sys_clone(unsigned long, unsigned long, int __user *,
 	       int __user *, int);
 #endif
+#endif

 asmlinkage long sys_execve(const char __user *filename,
 		const char __user *const __user *argv,
diff --git a/kernel/fork.c b/kernel/fork.c
index 403d2bb..e23bb19 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -1679,6 +1679,12 @@ SYSCALL_DEFINE5(clone, unsigned long, newsp, unsigned long, clone_flags,
 		 int __user *, parent_tidptr,
 		 int __user *, child_tidptr,
 		 int, tls_val)
+#elif defined(CONFIG_CLONE_BACKWARDS3)
+SYSCALL_DEFINE6(clone, unsigned long, clone_flags, unsigned long, newsp,
+		int, stack_size,
+		int __user *, parent_tidptr,
+		int __user *, child_tidptr,
+		int, tls_val)
 #else
 SYSCALL_DEFINE5(clone, unsigned long, clone_flags, unsigned long, newsp,
 		 int __user *, parent_tidptr,
--
1.8.2.3


[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH v3] microblaze: Fix clone syscall

[Andrew Morton]

On Mon, 29 Jul 2013 09:01:48 +0200 Michal Simek <michal.simek@xilinx.com> wrote:

> Fix inadvertent breakage in the clone syscall ABI for Microblaze
> that was introduced in this patch:
> "microblaze: switch to generic fork/vfork/clone"
> (sha1: f3268edbe6fe0ce56e62c6d6b14640aeb04864b7)
> 
> The Microblaze syscall ABI for clone takes the parent tid address in
> the 4th argument; the third argument slot is used for the stack size.
> The incorrectly-used CLONE_BACKWARDS type assigned parent tid to the
> 3rd slot.
> 
> This commit restores the original ABI so that existing userspace libc
> code will work correctly.
> 
> All kernel versions from v3.8-rc1 were affected.

x86_64 allnoconfig generates screenfuls of

In file included from fs/signalfd.c:31:
include/linux/syscalls.h:805:5: warning: "CONFIG_CLONE_BACKWARDS3" is not defined

> --- a/include/linux/syscalls.h
> +++ b/include/linux/syscalls.h
> @@ -802,9 +802,14 @@ asmlinkage long sys_vfork(void);
>  asmlinkage long sys_clone(unsigned long, unsigned long, int __user *, int,
>  	       int __user *);
>  #else
> +#if CONFIG_CLONE_BACKWARDS3

Presumably ifdef here will fix that.  Please redo, retest and resend?

Re: [PATCH v3] microblaze: Fix clone syscall

[Michal Simek]

[-- Attachment #1: Type: text/plain, Size: 1742 bytes --]

On 07/31/2013 10:10 PM, Andrew Morton wrote:
> On Mon, 29 Jul 2013 09:01:48 +0200 Michal Simek <michal.simek@xilinx.com> wrote:
> 
>> Fix inadvertent breakage in the clone syscall ABI for Microblaze
>> that was introduced in this patch:
>> "microblaze: switch to generic fork/vfork/clone"
>> (sha1: f3268edbe6fe0ce56e62c6d6b14640aeb04864b7)
>>
>> The Microblaze syscall ABI for clone takes the parent tid address in
>> the 4th argument; the third argument slot is used for the stack size.
>> The incorrectly-used CLONE_BACKWARDS type assigned parent tid to the
>> 3rd slot.
>>
>> This commit restores the original ABI so that existing userspace libc
>> code will work correctly.
>>
>> All kernel versions from v3.8-rc1 were affected.
> 
> x86_64 allnoconfig generates screenfuls of
> 
> In file included from fs/signalfd.c:31:
> include/linux/syscalls.h:805:5: warning: "CONFIG_CLONE_BACKWARDS3" is not defined
> 
>> --- a/include/linux/syscalls.h
>> +++ b/include/linux/syscalls.h
>> @@ -802,9 +802,14 @@ asmlinkage long sys_vfork(void);
>>  asmlinkage long sys_clone(unsigned long, unsigned long, int __user *, int,
>>  	       int __user *);
>>  #else
>> +#if CONFIG_CLONE_BACKWARDS3
> 
> Presumably ifdef here will fix that.  Please redo, retest and resend?

Yes,
I have got that email from Zero day testing system too.

Fixed in v4. I have also retest it for Microblaze.

Thanks,
Michal

-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 263 bytes --]