A bug about system call on ARM

Commit Message

Wang, Yalin May 31, 2013, 2:56 a.m. UTC

Hi Will,

Thanks for your patch ,

But I found  I don't have ct_user_exit  macro 
In my arch/arm/kernel/entry-common.S 

My kernel version is 3.4.0 

I have add the file as attachment,

Could you make a patch for this file ?

Thank you !

-----Original Message-----
From: Will Deacon [mailto:[e-mail]] 
Sent: Thursday, May 30, 2013 7:41 PM
To: Wang, Yalin
Cc: 'richard -rw- weinberger'; '[e-mail]'; '[e-mail]'; '[e-mail]'
Subject: Re: A bug about system call on ARM

On Thu, May 30, 2013 at 10:09:49AM +0100, Will Deacon wrote:
> On Thu, May 30, 2013 at 02:41:42AM +0100, Wang, Yalin wrote:
> > If you have some patch for this issue, I can do the test for it .
> 
> I'll have a look at cooking something which uses an exception table 
> entry to rewind the PC and retry the system call. That's simpler than 
> directly injecting a user page fault from the system call path.

Ok, please can you try the following?

Will

--->8

Comments

Will Deacon May 31, 2013, 8:46 a.m. UTC | #1

On Fri, May 31, 2013 at 03:56:31AM +0100, Wang, Yalin wrote:
> Hi Will,
> 
> Thanks for your patch ,
> 
> But I found  I don't have ct_user_exit  macro 
> In my arch/arm/kernel/entry-common.S 
> 
> My kernel version is 3.4.0 

Well things have moved on this since then (we're approaching 3.10, so you
might consider an upgrade!).

For the purposes of this patch, you can just delete the ct_user_exit line.

Will

Wang, Yalin May 31, 2013, 11:02 a.m. UTC | #2

Hi  Will,

I have merge your code ,
But there is a different ,

+	
+	ct_user_exit
+
+#ifdef CONFIG_ALIGNMENT_TRAP
+	ldr	ip, __cr_alignment
+	ldr	ip, [ip]
+	mcr	p15, 0, ip, c1, c0		@ update control register
+#endif 

+	enable_irq
+	get_thread_info tsk

Is this change ok ?


Thanks 

-----Original Message-----
From: [e-mail] [mailto:[e-mail]] On Behalf Of Will Deacon
Sent: Friday, May 31, 2013 4:47 PM
To: Wang, Yalin
Cc: 'richard -rw- weinberger'; '[e-mail]'; '[e-mail]'; '[e-mail]'
Subject: Re: A bug about system call on ARM

On Fri, May 31, 2013 at 03:56:31AM +0100, Wang, Yalin wrote:
> Hi Will,
> 
> Thanks for your patch ,
> 
> But I found  I don't have ct_user_exit  macro In my 
> arch/arm/kernel/entry-common.S
> 
> My kernel version is 3.4.0

Well things have moved on this since then (we're approaching 3.10, so you might consider an upgrade!).

For the purposes of this patch, you can just delete the ct_user_exit line.

Will
--
To unsubscribe from this list: send the line "unsubscribe linux-arch" in the body of a message to [e-mail] More majordomo info at  http://vger.kernel.org/majordomo-info.html

Will Deacon May 31, 2013, 11:13 a.m. UTC | #3

On Fri, May 31, 2013 at 12:02:49PM +0100, Wang, Yalin wrote:
> Hi  Will,
> 
> I have merge your code ,
> But there is a different ,
> 
> +	
> +	ct_user_exit

I thought you didn't have ct_user_exit? In which case, just delete this
line.

> +#ifdef CONFIG_ALIGNMENT_TRAP
> +	ldr	ip, __cr_alignment
> +	ldr	ip, [ip]
> +	mcr	p15, 0, ip, c1, c0		@ update control register
> +#endif 
> 
> +	enable_irq
> +	get_thread_info tsk

Hard to tell without context. You can take a look at my git tree if you
like (I fixed it up based on Nico's comment):

  https://git.kernel.org/cgit/linux/kernel/git/will/linux.git/commit/?h=misc-patches

Will

Wang, Yalin May 31, 2013, 11:30 a.m. UTC | #4

Hi Will,

I see,
I will make one more test .

Thanks for your clarification .

-----Original Message-----
From: Will Deacon [mailto:[e-mail]] 
Sent: Friday, May 31, 2013 7:13 PM
To: Wang, Yalin
Cc: 'richard -rw- weinberger'; '[e-mail]'; '[e-mail]'; '[e-mail]'
Subject: Re: A bug about system call on ARM

On Fri, May 31, 2013 at 12:02:49PM +0100, Wang, Yalin wrote:
> Hi  Will,
> 
> I have merge your code ,
> But there is a different ,
> 
> +	
> +	ct_user_exit

I thought you didn't have ct_user_exit? In which case, just delete this line.

> +#ifdef CONFIG_ALIGNMENT_TRAP
> +	ldr	ip, __cr_alignment
> +	ldr	ip, [ip]
> +	mcr	p15, 0, ip, c1, c0		@ update control register
> +#endif
> 
> +	enable_irq
> +	get_thread_info tsk

Hard to tell without context. You can take a look at my git tree if you like (I fixed it up based on Nico's comment):

  https://git.kernel.org/cgit/linux/kernel/git/will/linux.git/commit/?h=misc-patches

Will

Nicolas Pitre May 31, 2013, 4:48 p.m. UTC | #5

On Fri, 31 May 2013, Will Deacon wrote:

> Hard to tell without context. You can take a look at my git tree if you
> like (I fixed it up based on Nico's comment):
> 
>   https://git.kernel.org/cgit/linux/kernel/git/will/linux.git/commit/?h=misc-patches

If you like, you may add "Reviewed-by: Nicolas Pitre <[e-mail]>" 
to it.


Nicolas

Will Deacon May 31, 2013, 4:52 p.m. UTC | #6

On Fri, May 31, 2013 at 05:48:56PM +0100, Nicolas Pitre wrote:
> On Fri, 31 May 2013, Will Deacon wrote:
> 
> > Hard to tell without context. You can take a look at my git tree if you
> > like (I fixed it up based on Nico's comment):
> > 
> >   https://git.kernel.org/cgit/linux/kernel/git/will/linux.git/commit/?h=misc-patches
> 
> If you like, you may add "Reviewed-by: Nicolas Pitre <[e-mail]>" 
> to it.

Cheers Nicolas, will do.

Will

Wang, Yalin June 3, 2013, 5:25 a.m. UTC | #7

Hi  Will,

I have a question about this patch .

If the user space is thumb mode,
The PC should be rewind by 2 bytes,
So the fix_up code should be 

Sub lr, lr, #2 .


Am I right ?


Thanks for your help .

-----Original Message-----
From: Wang, Yalin 
Sent: Friday, May 31, 2013 7:31 PM
To: 'Will Deacon'
Cc: 'richard -rw- weinberger'; '[e-mail]'; '[e-mail]'; '[e-mail]'
Subject: RE: A bug about system call on ARM

Hi Will,

I see,
I will make one more test .

Thanks for your clarification .

-----Original Message-----
From: Will Deacon [mailto:[e-mail]] 
Sent: Friday, May 31, 2013 7:13 PM
To: Wang, Yalin
Cc: 'richard -rw- weinberger'; '[e-mail]'; '[e-mail]'; '[e-mail]'
Subject: Re: A bug about system call on ARM

On Fri, May 31, 2013 at 12:02:49PM +0100, Wang, Yalin wrote:
> Hi  Will,
> 
> I have merge your code ,
> But there is a different ,
> 
> +	
> +	ct_user_exit

I thought you didn't have ct_user_exit? In which case, just delete this line.

> +#ifdef CONFIG_ALIGNMENT_TRAP
> +	ldr	ip, __cr_alignment
> +	ldr	ip, [ip]
> +	mcr	p15, 0, ip, c1, c0		@ update control register
> +#endif
> 
> +	enable_irq
> +	get_thread_info tsk

Hard to tell without context. You can take a look at my git tree if you like (I fixed it up based on Nico's comment):

  https://git.kernel.org/cgit/linux/kernel/git/will/linux.git/commit/?h=misc-patches

Will

Will Deacon June 3, 2013, 9:54 a.m. UTC | #8

On Mon, Jun 03, 2013 at 06:25:26AM +0100, Wang, Yalin wrote:
> Hi  Will,
> 
> I have a question about this patch .
> 
> If the user space is thumb mode,
> The PC should be rewind by 2 bytes,
> So the fix_up code should be 
> 
> Sub lr, lr, #2 .
> 
> 
> Am I right ?

No, because we don't have OABI-compat support for Thumb applications and
force everything down the EABI path instead.

Did you manage to test the patch?

Will

Wang, Yalin June 3, 2013, 9:58 a.m. UTC | #9

Hi  Will

Oh  I see,
Thanks for your reply 

Yes ,  we are testing for it ,
But need some time to wait for the result ,
Because The stability test need some time to reproduce
this issue , And this issue doesn't reproduce 100% .


-----Original Message-----
From: Will Deacon [mailto:[e-mail]] 
Sent: Monday, June 03, 2013 5:54 PM
To: Wang, Yalin
Cc: 'richard -rw- weinberger'; '[e-mail]'; '[e-mail]'; '[e-mail]'
Subject: Re: A bug about system call on ARM

On Mon, Jun 03, 2013 at 06:25:26AM +0100, Wang, Yalin wrote:
> Hi  Will,
> 
> I have a question about this patch .
> 
> If the user space is thumb mode,
> The PC should be rewind by 2 bytes,
> So the fix_up code should be
> 
> Sub lr, lr, #2 .
> 
> 
> Am I right ?

No, because we don't have OABI-compat support for Thumb applications and force everything down the EABI path instead.

Did you manage to test the patch?

Will

Wang, Yalin June 4, 2013, 5:33 a.m. UTC | #10

Hi  Will,

Could I know  what's your git branch is  mainly used for ?

https://git.kernel.org/cgit/linux/kernel/git/will/linux.git


I mean if the branch is used for ARM arch maintenance ?
If yes, I think I can send future bugs about ARM to you directly,
And do not need ping-pang in the mail list .

Thanks for your help .

-----Original Message-----
From: Wang, Yalin 
Sent: Monday, June 03, 2013 5:58 PM
To: 'Will Deacon'
Cc: 'richard -rw- weinberger'; '[e-mail]'; '[e-mail]'; '[e-mail]'
Subject: RE: A bug about system call on ARM

Hi  Will

Oh  I see,
Thanks for your reply 

Yes ,  we are testing for it ,
But need some time to wait for the result , Because The stability test need some time to reproduce this issue , And this issue doesn't reproduce 100% .


-----Original Message-----
From: Will Deacon [mailto:[e-mail]]
Sent: Monday, June 03, 2013 5:54 PM
To: Wang, Yalin
Cc: 'richard -rw- weinberger'; '[e-mail]'; '[e-mail]'; '[e-mail]'
Subject: Re: A bug about system call on ARM

On Mon, Jun 03, 2013 at 06:25:26AM +0100, Wang, Yalin wrote:
> Hi  Will,
> 
> I have a question about this patch .
> 
> If the user space is thumb mode,
> The PC should be rewind by 2 bytes,
> So the fix_up code should be
> 
> Sub lr, lr, #2 .
> 
> 
> Am I right ?

No, because we don't have OABI-compat support for Thumb applications and force everything down the EABI path instead.

Did you manage to test the patch?

Will

Will Deacon June 4, 2013, 8:48 a.m. UTC | #11

On Tue, Jun 04, 2013 at 06:33:20AM +0100, Wang, Yalin wrote:
> Hi  Will,

Hello,

> Could I know  what's your git branch is  mainly used for ?
> 
> https://git.kernel.org/cgit/linux/kernel/git/will/linux.git
> 
> 
> I mean if the branch is used for ARM arch maintenance ?

I send most of my patches via Russell, and the tree above is where I keep my
patches whilst they're not yet in mainline.

> If yes, I think I can send future bugs about ARM to you directly,
> And do not need ping-pang in the mail list .

Quite the opposite! Having discussions on the mailing list is key to how the
kernel is developed, so please continue to send questions, bug reports and
patches there. As has been pointed out previously in this thread, you need
to choose the right list in the first place, rather than just sending to
LKML.

Of course, you can always CC me on arm/arm64 patches if you like.

Cheers,

Will

Wang, Yalin June 4, 2013, 9:30 a.m. UTC | #12

Hi Will,

Thanks for your reply,
I see your meaning ,
But it seems my apply for joining into '[e-mail]'
Is not approved ,  
How to join in this mail-list ?

Thanks for your help .

-----Original Message-----
From: Will Deacon [mailto:[e-mail]] 
Sent: Tuesday, June 04, 2013 4:49 PM
To: Wang, Yalin
Cc: 'richard -rw- weinberger'; '[e-mail]'; '[e-mail]'; '[e-mail]'
Subject: Re: A bug about system call on ARM

On Tue, Jun 04, 2013 at 06:33:20AM +0100, Wang, Yalin wrote:
> Hi  Will,

Hello,

> Could I know  what's your git branch is  mainly used for ?
> 
> https://git.kernel.org/cgit/linux/kernel/git/will/linux.git
> 
> 
> I mean if the branch is used for ARM arch maintenance ?

I send most of my patches via Russell, and the tree above is where I keep my patches whilst they're not yet in mainline.

> If yes, I think I can send future bugs about ARM to you directly, And 
> do not need ping-pang in the mail list .

Quite the opposite! Having discussions on the mailing list is key to how the kernel is developed, so please continue to send questions, bug reports and patches there. As has been pointed out previously in this thread, you need to choose the right list in the first place, rather than just sending to LKML.

Of course, you can always CC me on arm/arm64 patches if you like.

Cheers,

Will

Will Deacon June 4, 2013, 11:27 a.m. UTC | #13

On Tue, Jun 04, 2013 at 10:30:04AM +0100, Wang, Yalin wrote:
> Hi Will,
> 
> Thanks for your reply,
> I see your meaning ,
> But it seems my apply for joining into '[e-mail]'
> Is not approved ,  
> How to join in this mail-list ?

There's a mailman frontend here:

  http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Will

Wang, Yalin June 14, 2013, 6:53 a.m. UTC | #14

Hi Will,

We have tested the patch,
It seems ok in the stability test .

We have merged it into our main branch .

Thanks for your patch !

-----Original Message-----
From: Will Deacon [mailto:[e-mail]] 
Sent: Monday, June 03, 2013 5:54 PM
To: Wang, Yalin
Cc: 'richard -rw- weinberger'; '[e-mail]'; '[e-mail]'; '[e-mail]'
Subject: Re: A bug about system call on ARM

On Mon, Jun 03, 2013 at 06:25:26AM +0100, Wang, Yalin wrote:
> Hi  Will,
> 
> I have a question about this patch .
> 
> If the user space is thumb mode,
> The PC should be rewind by 2 bytes,
> So the fix_up code should be
> 
> Sub lr, lr, #2 .
> 
> 
> Am I right ?

No, because we don't have OABI-compat support for Thumb applications and force everything down the EABI path instead.

Did you manage to test the patch?

Will

Patch

diff --git a/arch/arm/kernel/entry-common.S b/arch/arm/kernel/entry-common.S index bc5bc0a..855926e 100644
--- a/arch/arm/kernel/entry-common.S
+++ b/arch/arm/kernel/entry-common.S
@@ -361,6 +361,15 @@  ENTRY(vector_swi)
 	str	r8, [sp, #S_PSR]		@ Save CPSR
 	str	r0, [sp, #S_OLD_R0]		@ Save OLD_R0
 	zero_fp
+	enable_irq
+	ct_user_exit
+
+#ifdef CONFIG_ALIGNMENT_TRAP
+	ldr	ip, __cr_alignment
+	ldr	ip, [ip]
+	mcr	p15, 0, ip, c1, c0		@ update control register
+#endif
+	get_thread_info tsk
 
 	/*
 	 * Get the system call number.
@@ -375,9 +384,9 @@  ENTRY(vector_swi)
 #ifdef CONFIG_ARM_THUMB
 	tst	r8, #PSR_T_BIT
 	movne	r10, #0				@ no thumb OABI emulation
-	ldreq	r10, [lr, #-4]			@ get SWI instruction
+ USER(	ldreq	r10, [lr, #-4]		)	@ get SWI instruction
 #else
-	ldr	r10, [lr, #-4]			@ get SWI instruction
+ USER(	ldr	r10, [lr, #-4]		)	@ get SWI instruction
 #endif
 #ifdef CONFIG_CPU_ENDIAN_BE8
 	rev	r10, r10			@ little endian instruction
@@ -392,22 +401,13 @@  ENTRY(vector_swi)
 	/* Legacy ABI only, possibly thumb mode. */
 	tst	r8, #PSR_T_BIT			@ this is SPSR from save_user_regs
 	addne	scno, r7, #__NR_SYSCALL_BASE	@ put OS number in
-	ldreq	scno, [lr, #-4]
+ USER(	ldreq	scno, [lr, #-4]		)
 
 #else
 	/* Legacy ABI only. */
-	ldr	scno, [lr, #-4]			@ get SWI instruction
-#endif
-
-#ifdef CONFIG_ALIGNMENT_TRAP
-	ldr	ip, __cr_alignment
-	ldr	ip, [ip]
-	mcr	p15, 0, ip, c1, c0		@ update control register
+ USER(	ldr	scno, [lr, #-4]		)	@ get SWI instruction
 #endif
-	enable_irq
-	ct_user_exit
 
-	get_thread_info tsk
 	adr	tbl, sys_call_table		@ load syscall table pointer
 
 #if defined(CONFIG_OABI_COMPAT)
@@ -442,6 +442,18 @@  local_restart:
 	eor	r0, scno, #__NR_SYSCALL_BASE	@ put OS number back
 	bcs	arm_syscall	
 	b	sys_ni_syscall			@ not private func
+
+#if defined(CONFIG_OABI_COMPAT) || !defined(CONFIG_AEABI)
+	/*
+	 * We may have faulted trying to load the SWI instruction due to
+	 * concurrent page aging on another CPU. In this case, return
+	 * back to the swi instruction and fault the page back.
+	 */
+9001:
+	sub	lr, lr, #4
+	str	lr, [sp, #S_PC]
+	b	ret_fast_syscall
+#endif
 ENDPROC(vector_swi)
 
 	/*