Re: [PATCH][AX25] ax25_out: check skb for NULL in ax25_kick()

2008-02-17 Thread David Miller 
From: Jarek Poplawski [EMAIL PROTECTED]
Date: Wed, 13 Feb 2008 11:56:07 +

 [AX25] ax25_out: check skb for NULL in ax25_kick()
 
 According to some OOPS reports ax25_kick tries to clone NULL skbs
 sometimes. It looks like a race with ax25_clear_queues(). Probably
 there is no need to add more than a simple check for this yet.
 Another report suggested there are probably also cases where ax25
 -paclen == 0 can happen in ax25_output(); this wasn't confirmed
 during testing but let's leave this debugging check for some time.
 
 Reported-and-tested-by: Jann Traschewski [EMAIL PROTECTED]
 Signed-off-by: Jarek Poplawski [EMAIL PROTECTED]

Applied, thanks Jarek.
--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH][AX25] ax25_out: check skb for NULL in ax25_kick()

2008-02-13 Thread Jarek Poplawski 
Hi,

Here is an official version of testing patch #2 from this thread.
The only difference: ax25-vs is changed only after checking skb is
not NULL (plus a comment). IMHO it could be applied.

Thanks,
Jarek P.



Subject: [AX25] ax25_out: check skb for NULL in ax25_kick()

According to some OOPS reports ax25_kick tries to clone NULL skbs
sometimes. It looks like a race with ax25_clear_queues(). Probably
there is no need to add more than a simple check for this yet.
Another report suggested there are probably also cases where ax25
-paclen == 0 can happen in ax25_output(); this wasn't confirmed
during testing but let's leave this debugging check for some time.


Reported-and-tested-by: Jann Traschewski [EMAIL PROTECTED]
Signed-off-by: Jarek Poplawski [EMAIL PROTECTED]

---

diff -Nurp 2.6.24-mm1-/net/ax25/ax25_out.c 2.6.24-mm1+/net/ax25/ax25_out.c
--- 2.6.24-mm1-/net/ax25/ax25_out.c 2008-01-24 22:58:37.0 +
+++ 2.6.24-mm1+/net/ax25/ax25_out.c 2008-02-13 10:43:50.0 +
@@ -117,6 +117,12 @@ void ax25_output(ax25_cb *ax25, int pacl
unsigned char *p;
int frontlen, len, fragno, ka9qfrag, first = 1;
 
+   if (paclen  16) {
+   WARN_ON_ONCE(1);
+   kfree_skb(skb);
+   return;
+   }
+
if ((skb-len - 1)  paclen) {
if (*skb-data == AX25_P_TEXT) {
skb_pull(skb, 1); /* skip PID */
@@ -251,8 +257,6 @@ void ax25_kick(ax25_cb *ax25)
if (start == end)
return;
 
-   ax25-vs = start;
-
/*
 * Transmit data until either we're out of data to send or
 * the window is full. Send a poll on the final I frame if
@@ -261,8 +265,13 @@ void ax25_kick(ax25_cb *ax25)
 
/*
 * Dequeue the frame and copy it.
+* Check for race with ax25_clear_queues().
 */
skb  = skb_dequeue(ax25-write_queue);
+   if (!skb)
+   return;
+
+   ax25-vs = start;
 
do {
if ((skbn = skb_clone(skb, GFP_ATOMIC)) == NULL) {
--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH][AX25] ax25_out: check skb for NULL in ax25_kick()

2008-02-13 Thread Jann Traschewski 
Applied and stable with Kernel 2.6.24.2 since 12 hours.
Regards,
Jann

 -Ursprüngliche Nachricht-
 Von: Jarek Poplawski [mailto:[EMAIL PROTECTED] 
 Gesendet: Mittwoch, 13. Februar 2008 12:56
 An: David Miller
 Cc: Jann Traschewski; Bernard Pidoux F6BVP; Ralf Baechle; 
 netdev@vger.kernel.org
 Betreff: [PATCH][AX25] ax25_out: check skb for NULL in ax25_kick()
 
 Hi,
 
 Here is an official version of testing patch #2 from this thread.
 The only difference: ax25-vs is changed only after checking 
 skb is not NULL (plus a comment). IMHO it could be applied.
 
 Thanks,
 Jarek P.
 
 
 
 Subject: [AX25] ax25_out: check skb for NULL in ax25_kick()
 
 According to some OOPS reports ax25_kick tries to clone NULL 
 skbs sometimes. It looks like a race with 
 ax25_clear_queues(). Probably there is no need to add more 
 than a simple check for this yet.
 Another report suggested there are probably also cases where ax25
 -paclen == 0 can happen in ax25_output(); this wasn't confirmed
 during testing but let's leave this debugging check for some time.
 
 
 Reported-and-tested-by: Jann Traschewski [EMAIL PROTECTED]
 Signed-off-by: Jarek Poplawski [EMAIL PROTECTED]
 
 ---
 
 diff -Nurp 2.6.24-mm1-/net/ax25/ax25_out.c 
 2.6.24-mm1+/net/ax25/ax25_out.c
 --- 2.6.24-mm1-/net/ax25/ax25_out.c   2008-01-24 
 22:58:37.0 +
 +++ 2.6.24-mm1+/net/ax25/ax25_out.c   2008-02-13 
 10:43:50.0 +
 @@ -117,6 +117,12 @@ void ax25_output(ax25_cb *ax25, int pacl
   unsigned char *p;
   int frontlen, len, fragno, ka9qfrag, first = 1;
  
 + if (paclen  16) {
 + WARN_ON_ONCE(1);
 + kfree_skb(skb);
 + return;
 + }
 +
   if ((skb-len - 1)  paclen) {
   if (*skb-data == AX25_P_TEXT) {
   skb_pull(skb, 1); /* skip PID */
 @@ -251,8 +257,6 @@ void ax25_kick(ax25_cb *ax25)
   if (start == end)
   return;
  
 - ax25-vs = start;
 -
   /*
* Transmit data until either we're out of data to send or
* the window is full. Send a poll on the final I frame 
 if @@ -261,8 +265,13 @@ void ax25_kick(ax25_cb *ax25)
  
   /*
* Dequeue the frame and copy it.
 +  * Check for race with ax25_clear_queues().
*/
   skb  = skb_dequeue(ax25-write_queue);
 + if (!skb)
 + return;
 +
 + ax25-vs = start;
  
   do {
   if ((skbn = skb_clone(skb, GFP_ATOMIC)) == NULL) {

--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html