Re: x86_64: Bug in new out of line put_user()

2005-04-22 Thread Alexander Nyberg
Brian, thanks for seeing this. (me goes hiding...)

The labels after the last put_user patch were misplaced so 
exceptions on the real mov instructions would not be handled.

Index: test/arch/x86_64/lib/putuser.S
===
--- test.orig/arch/x86_64/lib/putuser.S 2005-04-22 10:04:25.0 +0200
+++ test/arch/x86_64/lib/putuser.S  2005-04-22 10:06:29.0 +0200
@@ -49,8 +49,8 @@
jc 20f
cmpq threadinfo_addr_limit(%r8),%rcx
jae 20f
-2: decq %rcx
-   movw %dx,(%rcx)
+   decq %rcx
+2: movw %dx,(%rcx)
xorl %eax,%eax
ret
 20:decq %rcx
@@ -64,8 +64,8 @@
jc 30f
cmpq threadinfo_addr_limit(%r8),%rcx
jae 30f
-3: subq $3,%rcx
-   movl %edx,(%rcx)
+   subq $3,%rcx
+3: movl %edx,(%rcx)
xorl %eax,%eax
ret
 30:subq $3,%rcx
@@ -79,8 +79,8 @@
jc 40f
cmpq threadinfo_addr_limit(%r8),%rcx
jae 40f
-4: subq $7,%rcx
-   movq %rdx,(%rcx)
+   subq $7,%rcx
+4: movq %rdx,(%rcx)
xorl %eax,%eax
ret
 40:subq $7,%rcx


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


Re: x86_64: Bug in new out of line put_user()

2005-04-21 Thread Brian Gerst
Alexander Nyberg wrote:
The new out of line put_user() assembly on x86_64 changes %rcx without
telling GCC about it causing things like:
http://bugme.osdl.org/show_bug.cgi?id=4515 

See to it that %rcx is not changed (made it consistent with get_user()).
Signed-off-by: Alexander Nyberg <[EMAIL PROTECTED]>
Index: test/arch/x86_64/lib/getuser.S
===
--- test.orig/arch/x86_64/lib/getuser.S	2005-04-20 23:55:35.0 +0200
+++ test/arch/x86_64/lib/getuser.S	2005-04-21 00:54:16.0 +0200
@@ -78,9 +78,9 @@
 __get_user_8:
 	GET_THREAD_INFO(%r8)
 	addq $7,%rcx
-	jc bad_get_user
+	jc 40f
 	cmpq threadinfo_addr_limit(%r8),%rcx
-	jae	bad_get_user
+	jae	40f
 	subq	$7,%rcx
 4:	movq (%rcx),%rdx
 	xorl %eax,%eax
Index: test/arch/x86_64/lib/putuser.S
===
--- test.orig/arch/x86_64/lib/putuser.S	2005-04-21 00:50:24.0 +0200
+++ test/arch/x86_64/lib/putuser.S	2005-04-21 01:02:15.0 +0200
@@ -46,36 +46,45 @@
 __put_user_2:
 	GET_THREAD_INFO(%r8)
 	addq $1,%rcx
-	jc bad_put_user
+	jc 20f
 	cmpq threadinfo_addr_limit(%r8),%rcx
-	jae	 bad_put_user
-2:	movw %dx,-1(%rcx)
+	jae 20f
+2:	decq %rcx
+	movw %dx,(%rcx)
 	xorl %eax,%eax
 	ret
+20:	decq %rcx
+	jmp bad_put_user
 
 	.p2align 4
 .globl __put_user_4
 __put_user_4:
 	GET_THREAD_INFO(%r8)
 	addq $3,%rcx
-	jc bad_put_user
+	jc 30f
 	cmpq threadinfo_addr_limit(%r8),%rcx
-	jae bad_put_user
-3:	movl %edx,-3(%rcx)
+	jae 30f
+3:	subq $3,%rcx
+	movl %edx,(%rcx)
 	xorl %eax,%eax
 	ret
+30:	subq $3,%rcx
+	jmp bad_put_user
 
 	.p2align 4
 .globl __put_user_8
 __put_user_8:
 	GET_THREAD_INFO(%r8)
 	addq $7,%rcx
-	jc bad_put_user
+	jc 40f
 	cmpq threadinfo_addr_limit(%r8),%rcx
-	jae	bad_put_user
-4:	movq %rdx,-7(%rcx)
+	jae 40f
+4:	subq $7,%rcx
+	movq %rdx,(%rcx)
 	xorl %eax,%eax
 	ret
+40:	subq $7,%rcx
+	jmp bad_put_user
 
 bad_put_user:
 	movq $(-EFAULT),%rax

-
This patch has a serious bug.  The 2, 3 and 4 labels must be on the mov 
instructions in order to catch faults.

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


Re: x86_64: Bug in new out of line put_user()

2005-04-21 Thread Nicolas Boichat
Hello Alexander,

I have other kind of problems with this patch...

With 2.6.12-rc2 + your patch, when I run OpenOffice (a 32-bit
application), I get this in dmesg :

Unable to handle kernel paging request at 2d9280b0 RIP: 
{__put_user_4+32}
PGD 0 
Oops: 0002 [1] SMP 
CPU 0 
Modules linked in: cpufreq_ondemand sch_sfq sch_htb ipt_CONNMARK
ipt_ipp2p ipt_connmark ipt_mark ipt_tos ipt_length ipt_MARK
iptable_filter iptable_mangle ip_tables powernow_k8 tun nvidia
snd_pcm_oss snd_mixer_oss snd_via82xx snd_ac97_codec snd_pcm snd_timer
snd_page_alloc snd_mpu401_uart snd_rawmidi snd_seq_device snd cx8800
v4l1_compat v4l2_common cx88xx i2c_algo_bit video_buf ir_common
btcx_risc tveeprom videodev tuner usbhid w83627hf i2c_sensor i2c_isa
i2c_core usb_storage uhci_hcd ehci_hcd usbcore sd_mod scsi_mod
Pid: 11236, comm: soffice.bin Tainted: P  2.6.12-rc2
RIP: 0010:[] {__put_user_4+32}
RSP: :81001d32fea0  EFLAGS: 00010202
RAX: 0003 RBX: 8100255b0f70 RCX: 2d9280b0
RDX:  RSI: 810033534940 RDI: 2d9280b0
RBP:  R08: 81001d32e000 R09: 0002
R10: 81001d32e000 R11: 558740dc R12: 
R13: 810033534940 R14: 0001 R15: 
FS:  2e0ff820() GS:80489840()
knlGS:
CS:  0010 DS: 002b ES: 002b CR0: 8005003b
CR2: 2d9280b0 CR3: 2515 CR4: 06e0
Process soffice.bin (pid: 11236, threadinfo 81001d32e000, task
8100255b0f70)
Stack: 80133414 810019f11138 810033534940
8100255b0f70 
   8100255b0f70  80137c0a
 
    8100255b15a4 
Call Trace:{mm_release+116} {exit_mm
+42} 
   {do_exit+351} {do_group_exit
+252} 
   {ia32_sysret+0} 

It is easily reproducible on my system, I just have to start and exit
OpenOffice 1.1.4.

I got similar messages with 2.6.12-rc3 + your patch (please tell me if
you also want these messages).
I also had one system freeze and one reboot (when I clicked on a link in
Firefox (32-bit app too), I don't know if it is related). But I was
unable to reproduce these crashes, so I'm not sure whether it is caused
by rc3 or by your patch.

Please note that these problems did not occur with 2.6.12-rc2 + these
patches reverted:
 - x86_64-give-out-of-line-get_user-better-calling.patch
 - x86_64-move-put_user-out-of-line.patch
 - x86_64-remove-stale-unused-file.patch

Best regards,

Nicolas

On Thu, 2005-04-21 at 01:10 +0200, Alexander Nyberg wrote: 
> The new out of line put_user() assembly on x86_64 changes %rcx without
> telling GCC about it causing things like:
> 
> http://bugme.osdl.org/show_bug.cgi?id=4515 
> 
> See to it that %rcx is not changed (made it consistent with get_user()).
> 
> 
> Signed-off-by: Alexander Nyberg <[EMAIL PROTECTED]>
> 
> Index: test/arch/x86_64/lib/getuser.S
> ===
> --- test.orig/arch/x86_64/lib/getuser.S   2005-04-20 23:55:35.0 
> +0200
> +++ test/arch/x86_64/lib/getuser.S2005-04-21 00:54:16.0 +0200
> @@ -78,9 +78,9 @@
>  __get_user_8:
>   GET_THREAD_INFO(%r8)
>   addq $7,%rcx
> - jc bad_get_user
> + jc 40f
>   cmpq threadinfo_addr_limit(%r8),%rcx
> - jae bad_get_user
> + jae 40f
>   subq$7,%rcx
>  4:   movq (%rcx),%rdx
>   xorl %eax,%eax
> Index: test/arch/x86_64/lib/putuser.S
> ===
> --- test.orig/arch/x86_64/lib/putuser.S   2005-04-21 00:50:24.0 
> +0200
> +++ test/arch/x86_64/lib/putuser.S2005-04-21 01:02:15.0 +0200
> @@ -46,36 +46,45 @@
>  __put_user_2:
>   GET_THREAD_INFO(%r8)
>   addq $1,%rcx
> - jc bad_put_user
> + jc 20f
>   cmpq threadinfo_addr_limit(%r8),%rcx
> - jae  bad_put_user
> -2:   movw %dx,-1(%rcx)
> + jae 20f
> +2:   decq %rcx
> + movw %dx,(%rcx)
>   xorl %eax,%eax
>   ret
> +20:  decq %rcx
> + jmp bad_put_user
>  
>   .p2align 4
>  .globl __put_user_4
>  __put_user_4:
>   GET_THREAD_INFO(%r8)
>   addq $3,%rcx
> - jc bad_put_user
> + jc 30f
>   cmpq threadinfo_addr_limit(%r8),%rcx
> - jae bad_put_user
> -3:   movl %edx,-3(%rcx)
> + jae 30f
> +3:   subq $3,%rcx
> + movl %edx,(%rcx)
>   xorl %eax,%eax
>   ret
> +30:  subq $3,%rcx
> + jmp bad_put_user
>  
>   .p2align 4
>  .globl __put_user_8
>  __put_user_8:
>   GET_THREAD_INFO(%r8)
>   addq $7,%rcx
> - jc bad_put_user
> + jc 40f
>   cmpq threadinfo_addr_limit(%r8),%rcx
> - jae bad_put_user
> -4:   movq %rdx,-7(%rcx)
> + jae 40f
> +4:   subq $7,%rcx
> + movq %rdx,(%rcx)
>   xorl %eax,%eax
>   ret
> +40:  subq $7,%rcx
> + jmp bad_put_user
>  
>  bad_put_user:
>   movq $(-EFAULT),%rax
> 
> 
> 

-
To unsubscribe from this list: send the li

Re: x86_64: Bug in new out of line put_user()

2005-04-21 Thread Nicolas Boichat
Hello,

On Thu, 2005-04-21 at 01:10 +0200, Alexander Nyberg wrote:
> The new out of line put_user() assembly on x86_64 changes %rcx without
> telling GCC about it causing things like:
> 
> http://bugme.osdl.org/show_bug.cgi?id=4515 

Thank you, this patch fixes the message queue problem.

Best regards,

Nicolas

> See to it that %rcx is not changed (made it consistent with get_user()).
> 
> 
> Signed-off-by: Alexander Nyberg <[EMAIL PROTECTED]>
> 
> Index: test/arch/x86_64/lib/getuser.S
> ===
> --- test.orig/arch/x86_64/lib/getuser.S   2005-04-20 23:55:35.0 
> +0200
> +++ test/arch/x86_64/lib/getuser.S2005-04-21 00:54:16.0 +0200
> @@ -78,9 +78,9 @@
>  __get_user_8:
>   GET_THREAD_INFO(%r8)
>   addq $7,%rcx
> - jc bad_get_user
> + jc 40f
>   cmpq threadinfo_addr_limit(%r8),%rcx
> - jae bad_get_user
> + jae 40f
>   subq$7,%rcx
>  4:   movq (%rcx),%rdx
>   xorl %eax,%eax
> Index: test/arch/x86_64/lib/putuser.S
> ===
> --- test.orig/arch/x86_64/lib/putuser.S   2005-04-21 00:50:24.0 
> +0200
> +++ test/arch/x86_64/lib/putuser.S2005-04-21 01:02:15.0 +0200
> @@ -46,36 +46,45 @@
>  __put_user_2:
>   GET_THREAD_INFO(%r8)
>   addq $1,%rcx
> - jc bad_put_user
> + jc 20f
>   cmpq threadinfo_addr_limit(%r8),%rcx
> - jae  bad_put_user
> -2:   movw %dx,-1(%rcx)
> + jae 20f
> +2:   decq %rcx
> + movw %dx,(%rcx)
>   xorl %eax,%eax
>   ret
> +20:  decq %rcx
> + jmp bad_put_user
>  
>   .p2align 4
>  .globl __put_user_4
>  __put_user_4:
>   GET_THREAD_INFO(%r8)
>   addq $3,%rcx
> - jc bad_put_user
> + jc 30f
>   cmpq threadinfo_addr_limit(%r8),%rcx
> - jae bad_put_user
> -3:   movl %edx,-3(%rcx)
> + jae 30f
> +3:   subq $3,%rcx
> + movl %edx,(%rcx)
>   xorl %eax,%eax
>   ret
> +30:  subq $3,%rcx
> + jmp bad_put_user
>  
>   .p2align 4
>  .globl __put_user_8
>  __put_user_8:
>   GET_THREAD_INFO(%r8)
>   addq $7,%rcx
> - jc bad_put_user
> + jc 40f
>   cmpq threadinfo_addr_limit(%r8),%rcx
> - jae bad_put_user
> -4:   movq %rdx,-7(%rcx)
> + jae 40f
> +4:   subq $7,%rcx
> + movq %rdx,(%rcx)
>   xorl %eax,%eax
>   ret
> +40:  subq $7,%rcx
> + jmp bad_put_user
>  
>  bad_put_user:
>   movq $(-EFAULT),%rax
> 
> 
> 

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


Re: x86_64: Bug in new out of line put_user()

2005-04-21 Thread Andi Kleen
On Thu, Apr 21, 2005 at 01:10:09AM +0200, Alexander Nyberg wrote:
> The new out of line put_user() assembly on x86_64 changes %rcx without
> telling GCC about it causing things like:
> 
> http://bugme.osdl.org/show_bug.cgi?id=4515 
> 
> See to it that %rcx is not changed (made it consistent with get_user()).

Damn. I actually fixed this before submission, but it looks like
the old patch staid in the queue :-( Sorry for you having to debug
it again.

Linus, can can you please apply the patch?

-Andi

> 
> 
> Signed-off-by: Alexander Nyberg <[EMAIL PROTECTED]>
> 
> Index: test/arch/x86_64/lib/getuser.S
> ===
> --- test.orig/arch/x86_64/lib/getuser.S   2005-04-20 23:55:35.0 
> +0200
> +++ test/arch/x86_64/lib/getuser.S2005-04-21 00:54:16.0 +0200
> @@ -78,9 +78,9 @@
>  __get_user_8:
>   GET_THREAD_INFO(%r8)
>   addq $7,%rcx
> - jc bad_get_user
> + jc 40f
>   cmpq threadinfo_addr_limit(%r8),%rcx
> - jae bad_get_user
> + jae 40f
>   subq$7,%rcx
>  4:   movq (%rcx),%rdx
>   xorl %eax,%eax
> Index: test/arch/x86_64/lib/putuser.S
> ===
> --- test.orig/arch/x86_64/lib/putuser.S   2005-04-21 00:50:24.0 
> +0200
> +++ test/arch/x86_64/lib/putuser.S2005-04-21 01:02:15.0 +0200
> @@ -46,36 +46,45 @@
>  __put_user_2:
>   GET_THREAD_INFO(%r8)
>   addq $1,%rcx
> - jc bad_put_user
> + jc 20f
>   cmpq threadinfo_addr_limit(%r8),%rcx
> - jae  bad_put_user
> -2:   movw %dx,-1(%rcx)
> + jae 20f
> +2:   decq %rcx
> + movw %dx,(%rcx)
>   xorl %eax,%eax
>   ret
> +20:  decq %rcx
> + jmp bad_put_user
>  
>   .p2align 4
>  .globl __put_user_4
>  __put_user_4:
>   GET_THREAD_INFO(%r8)
>   addq $3,%rcx
> - jc bad_put_user
> + jc 30f
>   cmpq threadinfo_addr_limit(%r8),%rcx
> - jae bad_put_user
> -3:   movl %edx,-3(%rcx)
> + jae 30f
> +3:   subq $3,%rcx
> + movl %edx,(%rcx)
>   xorl %eax,%eax
>   ret
> +30:  subq $3,%rcx
> + jmp bad_put_user
>  
>   .p2align 4
>  .globl __put_user_8
>  __put_user_8:
>   GET_THREAD_INFO(%r8)
>   addq $7,%rcx
> - jc bad_put_user
> + jc 40f
>   cmpq threadinfo_addr_limit(%r8),%rcx
> - jae bad_put_user
> -4:   movq %rdx,-7(%rcx)
> + jae 40f
> +4:   subq $7,%rcx
> + movq %rdx,(%rcx)
>   xorl %eax,%eax
>   ret
> +40:  subq $7,%rcx
> + jmp bad_put_user
>  
>  bad_put_user:
>   movq $(-EFAULT),%rax
> 
> 
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


x86_64: Bug in new out of line put_user()

2005-04-20 Thread Alexander Nyberg
The new out of line put_user() assembly on x86_64 changes %rcx without
telling GCC about it causing things like:

http://bugme.osdl.org/show_bug.cgi?id=4515 

See to it that %rcx is not changed (made it consistent with get_user()).


Signed-off-by: Alexander Nyberg <[EMAIL PROTECTED]>

Index: test/arch/x86_64/lib/getuser.S
===
--- test.orig/arch/x86_64/lib/getuser.S 2005-04-20 23:55:35.0 +0200
+++ test/arch/x86_64/lib/getuser.S  2005-04-21 00:54:16.0 +0200
@@ -78,9 +78,9 @@
 __get_user_8:
GET_THREAD_INFO(%r8)
addq $7,%rcx
-   jc bad_get_user
+   jc 40f
cmpq threadinfo_addr_limit(%r8),%rcx
-   jae bad_get_user
+   jae 40f
subq$7,%rcx
 4: movq (%rcx),%rdx
xorl %eax,%eax
Index: test/arch/x86_64/lib/putuser.S
===
--- test.orig/arch/x86_64/lib/putuser.S 2005-04-21 00:50:24.0 +0200
+++ test/arch/x86_64/lib/putuser.S  2005-04-21 01:02:15.0 +0200
@@ -46,36 +46,45 @@
 __put_user_2:
GET_THREAD_INFO(%r8)
addq $1,%rcx
-   jc bad_put_user
+   jc 20f
cmpq threadinfo_addr_limit(%r8),%rcx
-   jae  bad_put_user
-2: movw %dx,-1(%rcx)
+   jae 20f
+2: decq %rcx
+   movw %dx,(%rcx)
xorl %eax,%eax
ret
+20:decq %rcx
+   jmp bad_put_user
 
.p2align 4
 .globl __put_user_4
 __put_user_4:
GET_THREAD_INFO(%r8)
addq $3,%rcx
-   jc bad_put_user
+   jc 30f
cmpq threadinfo_addr_limit(%r8),%rcx
-   jae bad_put_user
-3: movl %edx,-3(%rcx)
+   jae 30f
+3: subq $3,%rcx
+   movl %edx,(%rcx)
xorl %eax,%eax
ret
+30:subq $3,%rcx
+   jmp bad_put_user
 
.p2align 4
 .globl __put_user_8
 __put_user_8:
GET_THREAD_INFO(%r8)
addq $7,%rcx
-   jc bad_put_user
+   jc 40f
cmpq threadinfo_addr_limit(%r8),%rcx
-   jae bad_put_user
-4: movq %rdx,-7(%rcx)
+   jae 40f
+4: subq $7,%rcx
+   movq %rdx,(%rcx)
xorl %eax,%eax
ret
+40:subq $7,%rcx
+   jmp bad_put_user
 
 bad_put_user:
movq $(-EFAULT),%rax


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