alignment exceptionhandler sleeps in invalid context

alignment exceptionhandler sleeps in invalid context

[Olaf Hering]

I'm not sure where the bug is. Does it mean the network stack does
something nasty, or is the exception handler itself broken? (probably the latter)
This is 2.6.16.9 on a p270.

<3>Debug: sleeping function called from invalid context at arch/powerpc/kernel/align.c:440
<4>in_atomic():1, irqs_disabled():0
<4>Call Trace:
<4>[C00000000FFE6D10] [C00000000000EE20] .show_stack+0x68/0x1b0 (unreliable)
<4>[C00000000FFE6DB0] [C000000000054248] .__might_sleep+0xd8/0xf4
<4>[C00000000FFE6E30] [C00000000000C490] .fix_alignment+0x50c/0x9c8
<4>[C00000000FFE6F10] [C000000000024E20] .alignment_exception+0x18/0xc8
<4>[C00000000FFE6F90] [C00000000000430C] alignment_common+0x10c/0x180
<4>--- Exception: 600 at .csum_partial+0x34/0x9c
<4>    LR = .skb_checksum+0x194/0x35c
<4>[C00000000FFE7280] [D00000000033F3D0] 0xd00000000033f3d0 (unreliable)
<4>[C00000000FFE7340] [C0000000002EB558] .skb_checksum_help+0x98/0x124
<4>[C00000000FFE73D0] [D000000000330748] .ip_nat_fn+0x84/0x278 [iptable_nat]
<4>[C00000000FFE7490] [D000000000330C34] .ip_nat_local_fn+0x40/0xec [iptable_nat]
<4>[C00000000FFE7520] [C00000000030DAE0] .nf_iterate+0x80/0x11c
<4>[C00000000FFE75E0] [C00000000030DEC8] .nf_hook_slow+0x8c/0x17c
<4>[C00000000FFE76B0] [C00000000031D2B0] .ip_queue_xmit+0x4bc/0x5d0
<4>[C00000000FFE7800] [C00000000032FBA0] .tcp_transmit_skb+0x7d8/0x844
<4>[C00000000FFE78B0] [C0000000003318F0] .__tcp_push_pending_frames+0x2d4/0x3fc
<4>[C00000000FFE7970] [C00000000032EAD4] .tcp_rcv_established+0x894/0x964
<4>[C00000000FFE7A20] [C0000000003355C8] .tcp_v4_do_rcv+0x5c/0x48c
<4>[C00000000FFE7AF0] [C000000000338A48] .tcp_v4_rcv+0xc78/0xcfc
<4>[C00000000FFE7BC0] [C000000000316310] .ip_local_deliver+0x208/0x36c
<4>[C00000000FFE7C50] [C000000000316024] .ip_rcv+0x600/0x6e4
<4>[C00000000FFE7CF0] [C0000000002EBB34] .netif_receive_skb+0x500/0x598
<4>[C00000000FFE7D90] [C0000000002EE37C] .process_backlog+0xcc/0x1c8
<4>[C00000000FFE7E40] [C0000000002EE55C] .net_rx_action+0xe4/0x230
<4>[C00000000FFE7EF0] [C000000000066524] .__do_softirq+0x98/0x164
<4>[C00000000FFE7F90] [C000000000025854] .call_do_softirq+0x14/0x24
<4>[C0000003BA3C3370] [C00000000000BCC8] .do_softirq+0x8c/0xd8
<4>[C0000003BA3C3400] [C0000000000663E4] .local_bh_enable+0x58/0x8c
<4>[C0000003BA3C3480] [C0000000002EEA34] .dev_queue_xmit+0x2f0/0x318
<4>[C0000003BA3C3510] [C00000000031CA60] .ip_output+0x348/0x3f0
<4>[C0000003BA3C35B0] [C00000000031D2E0] .ip_queue_xmit+0x4ec/0x5d0
<4>[C0000003BA3C3700] [C00000000032FBA0] .tcp_transmit_skb+0x7d8/0x844
<4>[C0000003BA3C37B0] [C0000000003231A8] .tcp_cleanup_rbuf+0x148/0x168
<4>[C0000003BA3C3840] [C000000000325B24] .tcp_recvmsg+0x8a8/0xa00
<4>[C0000003BA3C3930] [C0000000002E0C64] .sock_common_recvmsg+0x5c/0x84
<4>[C0000003BA3C39C0] [C0000000002DD408] .do_sock_read+0x120/0x14c
<4>[C0000003BA3C3A60] [C0000000002DF034] .sock_aio_read+0x58/0x74
<4>[C0000003BA3C3B70] [C0000000000C6050] .do_sync_read+0xd4/0x130
<4>[C0000003BA3C3CF0] [C0000000000C6ECC] .vfs_read+0x134/0x1fc
<4>[C0000003BA3C3D90] [C0000000000C7390] .sys_read+0x4c/0x8c
<4>[C0000003BA3C3E30] [C00000000000871C] syscall_exit+0x0/0x40

Re: alignment exceptionhandler sleeps in invalid context

[Paul Mackerras]

Olaf Hering writes:

> I'm not sure where the bug is. Does it mean the network stack does
> something nasty, or is the exception handler itself broken? (probably the latter)
> This is 2.6.16.9 on a p270.

The alignment handler does a get_user to read the instruction.  I
suppose it will have to read the instruction directly if the exception
occurred in kernel mode.

Paul.

Re: alignment exceptionhandler sleeps in invalid context

[Paul Mackerras]

Olaf Hering writes:

> I'm not sure where the bug is. Does it mean the network stack does
> something nasty, or is the exception handler itself broken? (probably the latter)
> This is 2.6.16.9 on a p270.

This patch should fix it, I hope.  If you can verify that it fixes it
I'll send it to Linus.

Paul.

diff --git a/include/asm-powerpc/uaccess.h b/include/asm-powerpc/uaccess.h
index 3872e92..b02d858 100644
--- a/include/asm-powerpc/uaccess.h
+++ b/include/asm-powerpc/uaccess.h
@@ -179,7 +179,8 @@ do {								\
 #define __put_user_nocheck(x, ptr, size)			\
 ({								\
 	long __pu_err;						\
-	might_sleep();						\
+	if ((unsigned long)ptr < PAGE_OFFSET)			\
+		might_sleep();					\
 	__chk_user_ptr(ptr);					\
 	__put_user_size((x), (ptr), (size), __pu_err);		\
 	__pu_err;						\
@@ -259,7 +260,8 @@ ({								\
 	long __gu_err;						\
 	unsigned long __gu_val;					\
 	__chk_user_ptr(ptr);					\
-	might_sleep();						\
+	if ((unsigned long)ptr < PAGE_OFFSET)			\
+		might_sleep();					\
 	__get_user_size(__gu_val, (ptr), (size), __gu_err);	\
 	(x) = (__typeof__(*(ptr)))__gu_val;			\
 	__gu_err;						\
@@ -271,7 +273,8 @@ ({								\
 	long __gu_err;						\
 	long long __gu_val;					\
 	__chk_user_ptr(ptr);					\
-	might_sleep();						\
+	if ((unsigned long)ptr < PAGE_OFFSET)			\
+		might_sleep();					\
 	__get_user_size(__gu_val, (ptr), (size), __gu_err);	\
 	(x) = (__typeof__(*(ptr)))__gu_val;			\
 	__gu_err;						\

Re: alignment exceptionhandler sleeps in invalid context

[Olaf Hering]

 On Fri, Apr 28, Paul Mackeras wrote:

> Olaf Hering writes:
> 
> > I'm not sure where the bug is. Does it mean the network stack does
> > something nasty, or is the exception handler itself broken? (probably the latter)
> > This is 2.6.16.9 on a p270.
> 
> This patch should fix it, I hope.  If you can verify that it fixes it
> I'll send it to Linus.

I dont have a testcase, it just happend once on the p270.

Re: alignment exceptionhandler sleeps in invalid context

[Kumar Gala]

On Apr 28, 2006, at 6:19 AM, Olaf Hering wrote:

>  On Fri, Apr 28, Paul Mackeras wrote:
>
>> Olaf Hering writes:
>>
>>> I'm not sure where the bug is. Does it mean the network stack does
>>> something nasty, or is the exception handler itself broken?  
>>> (probably the latter)
>>> This is 2.6.16.9 on a p270.
>>
>> This patch should fix it, I hope.  If you can verify that it fixes it
>> I'll send it to Linus.
>
> I dont have a testcase, it just happend once on the p270.

I was going to ask how this was actually hit.  It seems odd that we  
would have an alignment error in the network stack.  From the oops  
can we determine what the instruction was that faulted and what  
address it was trying to access.

- k

Re: alignment exceptionhandler sleeps in invalid context

[Paul Mackerras]

Kumar Gala writes:

> I was going to ask how this was actually hit.  It seems odd that we  
> would have an alignment error in the network stack.  From the oops  
> can we determine what the instruction was that faulted and what  
> address it was trying to access.

No, it's not particularly odd, given that our get_unaligned() and
put_unaligned() in include/asm-powerpc/unaligned.h just do a single
access.  That makes the common case (address is actually aligned, or
cpu handles it in hardware) fast at the expense of occasionally taking
an alignment exception.

Paul.

Re: alignment exceptionhandler sleeps in invalid context

[Michael Ellerman]

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

On Fri, 2006-04-28 at 21:02 +1000, Paul Mackerras wrote:
> Olaf Hering writes:
> 
> > I'm not sure where the bug is. Does it mean the network stack does
> > something nasty, or is the exception handler itself broken? (probably the latter)
> > This is 2.6.16.9 on a p270.
> 
> This patch should fix it, I hope.  If you can verify that it fixes it
> I'll send it to Linus.
> 
> Paul.
> 
> diff --git a/include/asm-powerpc/uaccess.h b/include/asm-powerpc/uaccess.h
> index 3872e92..b02d858 100644
> --- a/include/asm-powerpc/uaccess.h
> +++ b/include/asm-powerpc/uaccess.h
> @@ -179,7 +179,8 @@ do {								\
>  #define __put_user_nocheck(x, ptr, size)			\
>  ({								\
>  	long __pu_err;						\
> -	might_sleep();						\
> +	if ((unsigned long)ptr < PAGE_OFFSET)			\
> +		might_sleep();					\

+	if (!is_kernel_addr((unsigned long)ptr))		\
+		might_sleep();					\

In asm/page.h :)

cheers

-- 
Michael Ellerman
IBM OzLabs

wwweb: http://michael.ellerman.id.au
phone: +61 2 6212 1183 (tie line 70 21183)

We do not inherit the earth from our ancestors,
we borrow it from our children. - S.M.A.R.T Person

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 191 bytes --]

Re: alignment exceptionhandler sleeps in invalid context

[Paul Mackerras]

I wrote:

> This patch should fix it, I hope.  If you can verify that it fixes it
> I'll send it to Linus.

but init barfs at startup with that patch, due to the double
evaluation of `ptr'.  This one runs a bit better...

Paul.

diff --git a/include/asm-powerpc/uaccess.h b/include/asm-powerpc/uaccess.h
index 3872e92..d83fc29 100644
--- a/include/asm-powerpc/uaccess.h
+++ b/include/asm-powerpc/uaccess.h
@@ -7,6 +7,7 @@ #ifndef __ASSEMBLY__
 #include <linux/sched.h>
 #include <linux/errno.h>
 #include <asm/processor.h>
+#include <asm/page.h>
 
 #define VERIFY_READ	0
 #define VERIFY_WRITE	1
@@ -179,9 +180,11 @@ do {								\
 #define __put_user_nocheck(x, ptr, size)			\
 ({								\
 	long __pu_err;						\
-	might_sleep();						\
-	__chk_user_ptr(ptr);					\
-	__put_user_size((x), (ptr), (size), __pu_err);		\
+	__typeof__(*(ptr)) __user *__pu_addr = (ptr);		\
+	if (!is_kernel_addr((unsigned long)__pu_addr))		\
+		might_sleep();					\
+	__chk_user_ptr(ptr);					\
+	__put_user_size((x), __pu_addr, (size), __pu_err);	\
 	__pu_err;						\
 })
 
@@ -258,9 +261,11 @@ #define __get_user_nocheck(x, ptr, size)
 ({								\
 	long __gu_err;						\
 	unsigned long __gu_val;					\
+	const __typeof__(*(ptr)) __user *__gu_addr = (ptr);	\
 	__chk_user_ptr(ptr);					\
-	might_sleep();						\
-	__get_user_size(__gu_val, (ptr), (size), __gu_err);	\
+	if (!is_kernel_addr((unsigned long)__gu_addr))		\
+		might_sleep();					\
+	__get_user_size(__gu_val, __gu_addr, (size), __gu_err);	\
 	(x) = (__typeof__(*(ptr)))__gu_val;			\
 	__gu_err;						\
 })
@@ -270,9 +275,11 @@ #define __get_user64_nocheck(x, ptr, siz
 ({								\
 	long __gu_err;						\
 	long long __gu_val;					\
-	__chk_user_ptr(ptr);					\
-	might_sleep();						\
-	__get_user_size(__gu_val, (ptr), (size), __gu_err);	\
+	const __typeof__(*(ptr)) __user *__gu_addr = (ptr);	\
+	__chk_user_ptr(ptr);					\
+	if (!is_kernel_addr((unsigned long)__gu_addr))		\
+		might_sleep();					\
+	__get_user_size(__gu_val, __gu_addr, (size), __gu_err);	\
 	(x) = (__typeof__(*(ptr)))__gu_val;			\
 	__gu_err;						\
 })