[BUG][AX25] spinlock lockup

[BUG][AX25] spinlock lockup

[Jann Traschewski]

Hello,

I got a "spinlock lockup" using the latest Kernel 2.6.24.2 with recent
patches from Jarek Poplawski applied.


 ppp_deflate nf_nat zlib_deflateBUG: unable to handle kernel NULL pointer
dereference zlib_inflate nf_conntrack_ipv4 bsd_comp slhc ppp_async xt_state
tun ppp_genericprinting eip: c02338c2  nf_conntrack bitrev ipt_REJECT*pde =
00000000  iptable_filter crc32
 iptable_mangle mkiss xt_MARK at virtual address 00000004
 ax25 ipv6Oops: 0002 [#1] SMP  ipip crc16
 tunnel4Modules linked in: iptable_nat ide_cd cdrom aic7xxx
scsi_transport_spi parport_serial parport_pc parport i2c_piix4 netconsole
genrtc

Pid: 3032, comm: linuxnet Not tainted (2.6.24.2-dg8ngn-p02 #1)
EIP: 0060:[<c02338c2>] EFLAGS: 00010092 CPU: 0
EIP is at skb_append+0x1b/0x30
EAX: 00000292 EBX: c9318980 ECX: c0466a80 EDX: 00000000
ESI: c9e085c0 EDI: f40846ac EBP: f40846b8 ESP: f72cbd04
 DS: 007b ES: 007b FS: 00d8 GS: 0033 SS: 0068
Process linuxnet (pid: 3032, ti=f72ca000 task=f7d08030 task.ti=f72ca000)
Stack: c9318980 c9e085c0 f4084600 00000000 f8a0e2a3 f4084600 00000005
00000010 BUG: spinlock lockup on CPU#1, bcm/12213, f40846b8
Pid: 12213, comm: bcm Tainted: G      D 2.6.24.2-dg8ngn-p02 #1
 [<c01cb96f>] _raw_spin_lock+0xbb/0xdc
 [<c0299fdc>] _spin_lock_irqsave+0x39/0x41
 [<c02339bf>] skb_dequeue+0xf/0x3f
 [<c02339bf>] skb_dequeue+0xf/0x3f
 [<f8a0c998>] ax25_kick+0x14f/0x18c [ax25]
 [<f8a10a26>] ax25_sendmsg+0x35f/0x49a [ax25]
 [<c01473d1>] file_read_actor+0x7d/0xd5
 [<c0147c35>] do_generic_mapping_read+0x1d5/0x3b2
 [<c0147354>] file_read_actor+0x0/0xd5
 [<c022e404>] sock_aio_write+0xbf/0xcb
 [<c01494ba>] generic_file_aio_read+0x161/0x19c
 [<c01627bb>] do_sync_write+0xc7/0x10a
 [<c0130ea9>] autoremove_wake_function+0x0/0x35
 [<c0162f48>] vfs_write+0x9e/0x10c
 [<c01634b0>] sys_write+0x41/0x67
 [<c0103ea2>] syscall_call+0x7/0xb
 =======================


--
Jann Traschewski, Drosselstr.1, D-90513 Zirndorf, Germany
Tel.: +49-911-696971, Mobile: +49-170-1045937, EMail: jann@gmx.de
Ham: DG8NGN / DB0VOX, http://www.qsl.net/db0fhn, ICQ UIN: 4130182 

Re: [BUG][AX25] spinlock lockup

[Jarek Poplawski]

On Sun, Feb 24, 2008 at 04:10:29AM +0100, Jann Traschewski wrote:
> Hello,

Hi!

> I got a "spinlock lockup" using the latest Kernel 2.6.24.2 with recent
> patches from Jarek Poplawski applied.
...
>  ppp_deflate nf_nat zlib_deflateBUG: unable to handle kernel NULL pointer
> dereference zlib_inflate nf_conntrack_ipv4 bsd_comp slhc ppp_async xt_state
...
> EIP is at skb_append+0x1b/0x30
...
> 00000010 BUG: spinlock lockup on CPU#1, bcm/12213, f40846b8

Looks like 2 in 1: NULL pointer dereference and (later?) lockup.

There is only one function in AX25 calling skb_append(), and it really
looks suspicious: appends skb after previously enqueued one, but in
the meantime this previous skb could be removed from the queue.

Here is a patch for testing: it fixes this simple way, so this is not
fully compatible with the current method, but let's check if this could
be a problem?

Regards,
Jarek P.

(testing patch #1)

---

 net/ax25/ax25_subr.c |   11 +++--------
 1 files changed, 3 insertions(+), 8 deletions(-)

diff --git a/net/ax25/ax25_subr.c b/net/ax25/ax25_subr.c
index d8f2157..034aa10 100644
--- a/net/ax25/ax25_subr.c
+++ b/net/ax25/ax25_subr.c
@@ -64,20 +64,15 @@ void ax25_frames_acked(ax25_cb *ax25, unsigned short nr)
 
 void ax25_requeue_frames(ax25_cb *ax25)
 {
-	struct sk_buff *skb, *skb_prev = NULL;
+	struct sk_buff *skb;
 
 	/*
 	 * Requeue all the un-ack-ed frames on the output queue to be picked
 	 * up by ax25_kick called from the timer. This arrangement handles the
 	 * possibility of an empty output queue.
 	 */
-	while ((skb = skb_dequeue(&ax25->ack_queue)) != NULL) {
-		if (skb_prev == NULL)
-			skb_queue_head(&ax25->write_queue, skb);
-		else
-			skb_append(skb_prev, skb, &ax25->write_queue);
-		skb_prev = skb;
-	}
+	while ((skb = skb_dequeue_tail(&ax25->ack_queue)) != NULL)
+		skb_queue_head(&ax25->write_queue, skb);
 }
 
 /*

Re: [BUG][AX25] spinlock lockup

[Jann Traschewski]

Applied. Thanks!
Regards,
Jann

> -----Ursprüngliche Nachricht-----
> Von: Jarek Poplawski [mailto:jarkao2@gmail.com] 
> Gesendet: Sonntag, 24. Februar 2008 20:51
> An: Jann Traschewski
> Cc: netdev@vger.kernel.org; 'Ralf Baechle'
> Betreff: Re: [BUG][AX25] spinlock lockup
> 
> On Sun, Feb 24, 2008 at 04:10:29AM +0100, Jann Traschewski wrote:
> > Hello,
> 
> Hi!
> 
> > I got a "spinlock lockup" using the latest Kernel 2.6.24.2 
> with recent 
> > patches from Jarek Poplawski applied.
> ...
> >  ppp_deflate nf_nat zlib_deflateBUG: unable to handle kernel NULL 
> > pointer dereference zlib_inflate nf_conntrack_ipv4 bsd_comp slhc 
> > ppp_async xt_state
> ...
> > EIP is at skb_append+0x1b/0x30
> ...
> > 00000010 BUG: spinlock lockup on CPU#1, bcm/12213, f40846b8
> 
> Looks like 2 in 1: NULL pointer dereference and (later?) lockup.
> 
> There is only one function in AX25 calling skb_append(), and 
> it really looks suspicious: appends skb after previously 
> enqueued one, but in the meantime this previous skb could be 
> removed from the queue.
> 
> Here is a patch for testing: it fixes this simple way, so 
> this is not fully compatible with the current method, but 
> let's check if this could be a problem?
> 
> Regards,
> Jarek P.
> 
> (testing patch #1)
> 
> ---
> 
>  net/ax25/ax25_subr.c |   11 +++--------
>  1 files changed, 3 insertions(+), 8 deletions(-)
> 
> diff --git a/net/ax25/ax25_subr.c b/net/ax25/ax25_subr.c 
> index d8f2157..034aa10 100644
> --- a/net/ax25/ax25_subr.c
> +++ b/net/ax25/ax25_subr.c
> @@ -64,20 +64,15 @@ void ax25_frames_acked(ax25_cb *ax25, 
> unsigned short nr)
>  
>  void ax25_requeue_frames(ax25_cb *ax25)  {
> -	struct sk_buff *skb, *skb_prev = NULL;
> +	struct sk_buff *skb;
>  
>  	/*
>  	 * Requeue all the un-ack-ed frames on the output queue 
> to be picked
>  	 * up by ax25_kick called from the timer. This 
> arrangement handles the
>  	 * possibility of an empty output queue.
>  	 */
> -	while ((skb = skb_dequeue(&ax25->ack_queue)) != NULL) {
> -		if (skb_prev == NULL)
> -			skb_queue_head(&ax25->write_queue, skb);
> -		else
> -			skb_append(skb_prev, skb, &ax25->write_queue);
> -		skb_prev = skb;
> -	}
> +	while ((skb = skb_dequeue_tail(&ax25->ack_queue)) != NULL)
> +		skb_queue_head(&ax25->write_queue, skb);
>  }
>  
>  /*

[PATCH] Fix NULL pointer dereference and lockup.

[Ralf Baechle]

From: Jarek Poplawski <jarkao2@gmail.com>

There is only one function in AX25 calling skb_append(), and it really
looks suspicious: appends skb after previously enqueued one, but in
the meantime this previous skb could be removed from the queue.

This patch Fixes it the simple way, so this is not fully compatible with
the current method, but testing hasn't shown any problems.

Signed-off-by: Ralf Baechle <ralf@linux-mips.org>

---
I'm told Jarek is currently not reachable, so I'm submitting this for 2.6.26
and -stable.

diff --git a/net/ax25/ax25_subr.c b/net/ax25/ax25_subr.c
index d8f2157..034aa10 100644
--- a/net/ax25/ax25_subr.c
+++ b/net/ax25/ax25_subr.c
@@ -64,20 +64,15 @@ void ax25_frames_acked(ax25_cb *ax25, unsigned short nr)
 
 void ax25_requeue_frames(ax25_cb *ax25)
 {
-	struct sk_buff *skb, *skb_prev = NULL;
+	struct sk_buff *skb;
 
 	/*
 	 * Requeue all the un-ack-ed frames on the output queue to be picked
 	 * up by ax25_kick called from the timer. This arrangement handles the
 	 * possibility of an empty output queue.
 	 */
-	while ((skb = skb_dequeue(&ax25->ack_queue)) != NULL) {
-		if (skb_prev == NULL)
-			skb_queue_head(&ax25->write_queue, skb);
-		else
-			skb_append(skb_prev, skb, &ax25->write_queue);
-		skb_prev = skb;
-	}
+	while ((skb = skb_dequeue_tail(&ax25->ack_queue)) != NULL)
+		skb_queue_head(&ax25->write_queue, skb);
 }
 
 /*

Re: [PATCH] Fix NULL pointer dereference and lockup.

[Jarek Poplawski]

On Mon, May 26, 2008 at 09:23:45AM +0100, Ralf Baechle wrote:
> From: Jarek Poplawski <jarkao2@gmail.com>
> 
> There is only one function in AX25 calling skb_append(), and it really
> looks suspicious: appends skb after previously enqueued one, but in
> the meantime this previous skb could be removed from the queue.
> 
> This patch Fixes it the simple way, so this is not fully compatible with
> the current method, but testing hasn't shown any problems.
> 
> Signed-off-by: Ralf Baechle <ralf@linux-mips.org>
> 
> ---
> I'm told Jarek is currently not reachable, so I'm submitting this for 2.6.26
> and -stable.

Acked-by: Jarek Poplawski <jarkao2@gmail.com>

Re: [PATCH] Fix NULL pointer dereference and lockup.

[David Miller]

From: Ralf Baechle <ralf@linux-mips.org>
Date: Mon, 26 May 2008 09:23:45 +0100

> From: Jarek Poplawski <jarkao2@gmail.com>
> 
> There is only one function in AX25 calling skb_append(), and it really
> looks suspicious: appends skb after previously enqueued one, but in
> the meantime this previous skb could be removed from the queue.
> 
> This patch Fixes it the simple way, so this is not fully compatible with
> the current method, but testing hasn't shown any problems.
> 
> Signed-off-by: Ralf Baechle <ralf@linux-mips.org>
> 
> ---
> I'm told Jarek is currently not reachable, so I'm submitting this for 2.6.26
> and -stable.

Applied, and queued up for -stable, thanks everyone.