[PATCH] Using sock_orphan in ax25 and netrom

[PATCH] Using sock_orphan in ax25 and netrom

[Jeroen Vreeken]

Hi all,

Yesterday's patch proved to be just a piece of bandaid for a larger
problem: ax25 doesn't use sock_orphan() to set sk->dead. Included is a
patch for both ax25 and netrom to fix this problem.

Jeroen


diff -ruN linux-2.4.14/net/ax25/af_ax25.c linux/net/ax25/af_ax25.c
--- linux-2.4.14/net/ax25/af_ax25.c	Fri Sep 14 02:16:23 2001
+++ linux/net/ax25/af_ax25.c	Mon Nov 19 21:36:09 2001
@@ -102,6 +102,7 @@
  *			Joerg(DL1BKE)		Added support for
SO_BINDTODEVICE
  *			Arnaldo C. Melo		s/suser/capable(CAP_NET_ADMIN)/,
some more cleanups
  *			Michal Ostrowski	Module initialization
cleanup.
+ *			Jeroen(PE1RXQ)		Use sock_orphan to
set sk->dead.
  */
 
 #include <linux/config.h>
@@ -423,7 +424,7 @@
 	if (ax25->sk != NULL) {
 		while ((skb = skb_dequeue(&ax25->sk->receive_queue)) !=
NULL) {
 			if (skb->sk != ax25->sk) {			/* A
pending connection */
-				skb->sk->dead = 1;	/* Queue the unaccepted
socket for death */
+				sock_orphan(skb->sk);	/* Queue the
unaccepted socket for death */
 				ax25_start_heartbeat(skb->sk->protinfo.ax25);
 				skb->sk->protinfo.ax25->state = AX25_STATE_0;
 			}
@@ -1018,7 +1019,7 @@
 				sk->state                = TCP_CLOSE;
 				sk->shutdown            |= SEND_SHUTDOWN;
 				sk->state_change(sk);
-				sk->dead                 = 1;
+				sock_orphan(sk);
 				sk->destroy              = 1;
 				break;
 
@@ -1029,7 +1030,7 @@
 		sk->state     = TCP_CLOSE;
 		sk->shutdown |= SEND_SHUTDOWN;
 		sk->state_change(sk);
-		sk->dead      = 1;
+		sock_orphan(sk);
 		ax25_destroy_socket(sk->protinfo.ax25);
 	}
 
diff -ruN linux-2.4.14/net/ax25/ax25_ds_timer.c
linux/net/ax25/ax25_ds_timer.c
--- linux-2.4.14/net/ax25/ax25_ds_timer.c	Fri Dec 29 23:35:47 2000
+++ linux/net/ax25/ax25_ds_timer.c	Mon Nov 19 21:57:29 2001
@@ -162,7 +162,7 @@
 		ax25->sk->shutdown |= SEND_SHUTDOWN;
 		if (!ax25->sk->dead)
 			ax25->sk->state_change(ax25->sk);
-		ax25->sk->dead      = 1;
+		sock_orphan(ax25->sk);
 	}
 }
 
diff -ruN linux-2.4.14/net/ax25/ax25_std_timer.c
linux/net/ax25/ax25_std_timer.c
--- linux-2.4.14/net/ax25/ax25_std_timer.c	Fri Dec 29 23:35:47 2000
+++ linux/net/ax25/ax25_std_timer.c	Mon Nov 19 21:57:29 2001
@@ -109,7 +109,7 @@
 		ax25->sk->shutdown |= SEND_SHUTDOWN;
 		if (!ax25->sk->dead)
 			ax25->sk->state_change(ax25->sk);
-		ax25->sk->dead      = 1;
+		sock_orphan(ax25->sk);
 	}
 }
 
diff -ruN linux-2.4.14/net/ax25/ax25_subr.c linux/net/ax25/ax25_subr.c
--- linux-2.4.14/net/ax25/ax25_subr.c	Sat Jun 30 04:38:26 2001
+++ linux/net/ax25/ax25_subr.c	Mon Nov 19 21:57:29 2001
@@ -310,6 +310,6 @@
 		ax25->sk->shutdown |= SEND_SHUTDOWN;
 		if (!ax25->sk->dead)
 			ax25->sk->state_change(ax25->sk);
-		ax25->sk->dead      = 1;
+		sock_orphan(ax25->sk);
 	}
 }
diff -ruN linux-2.4.14/net/netrom/af_netrom.c linux/net/netrom/af_netrom.c
--- linux-2.4.14/net/netrom/af_netrom.c	Mon Sep 10 16:58:35 2001
+++ linux/net/netrom/af_netrom.c	Mon Nov 19 21:36:09 2001
@@ -31,6 +31,7 @@
  *	NET/ROM 007	Jonathan(G4KLX)	New timer
architecture.
  *					Impmented Idle timer.
  *			Arnaldo C. Melo s/suser/capable/, micro cleanups
+ *			Jeroen (PE1RXQ)	Use sock_orphan to set
sk->dead.
  */
 
 #include <linux/config.h>
@@ -316,7 +317,7 @@
 
 	while ((skb = skb_dequeue(&sk->receive_queue)) != NULL) {
 		if (skb->sk != sk) {			/* A pending
connection */
-			skb->sk->dead = 1;	/* Queue the unaccepted
socket for death */
+			sock_orphan(skb->sk);	/* Queue the
unaccepted socket for death */
 			nr_start_heartbeat(skb->sk);
 			skb->sk->protinfo.nr->state = NR_STATE_0;
 		}
@@ -572,9 +573,8 @@
 			sk->state                = TCP_CLOSE;
 			sk->shutdown            |= SEND_SHUTDOWN;
 			sk->state_change(sk);
-			sk->dead                 = 1;
+			sock_orphan(sk);
 			sk->destroy              = 1;
-			sk->socket               = NULL;
 			break;
 
 		default:
diff -ruN linux-2.4.14/net/netrom/nr_subr.c linux/net/netrom/nr_subr.c
--- linux-2.4.14/net/netrom/nr_subr.c	Thu Jun 28 02:10:55 2001
+++ linux/net/netrom/nr_subr.c	Mon Nov 19 21:39:01 2001
@@ -284,5 +284,5 @@
 	if (!sk->dead)
 		sk->state_change(sk);
 
-	sk->dead = 1;
+	sock_orphan(sk);
 }
diff -ruN linux-2.4.14/net/netrom/nr_timer.c linux/net/netrom/nr_timer.c
--- linux-2.4.14/net/netrom/nr_timer.c	Fri Dec 29 23:44:46 2000
+++ linux/net/netrom/nr_timer.c	Mon Nov 19 21:39:17 2001
@@ -201,7 +201,7 @@
 	if (!sk->dead)
 		sk->state_change(sk);
 
-	sk->dead = 1;
+	sock_orphan(sk);
 }
 
 static void nr_t1timer_expiry(unsigned long param)

Re: [PATCH] Using sock_orphan in ax25 and netrom

[Jeroen Vreeken]

Appearantly my patch from yesterday was a bit premature...

It looks like the ax25 and netrom stack do some magic with the dead sockets
that is not compatible with sock_orphan. I will try and see if I can track
them down.

The result is that at the moment the only way to make ax25 stable in 2.4.x
is to add the sk->dead check in my first patch.

Jeroen

Re: [PATCH] Using sock_orphan in ax25 and netrom

[Jeroen Vreeken]

On 2001.11.21 19:42:23 +0100 Jeroen Vreeken wrote:
> Appearantly my patch from yesterday was a bit premature...
> 
> It looks like the ax25 and netrom stack do some magic with the dead
> sockets
> that is not compatible with sock_orphan. I will try and see if I can
> track
> them down.

Found the cause in datagram_poll() in datagram.c:


        if (sock_writeable(sk))
                mask |= POLLOUT | POLLWRNORM | POLLWRBAND;
        else
                set_bit(SOCK_ASYNC_NOSPACE, &sk->socket->flags);

Since our socket is dead its not writeable and the code tries to set
sk->socket->flags
However sk->socket has been set to NULL by sock_orphan()
Adding a check for this solves the problem:

        if (sock_writeable(sk))
                mask |= POLLOUT | POLLWRNORM | POLLWRBAND;
        else
                if (sk->socket)
                        set_bit(SOCK_ASYNC_NOSPACE, &sk->socket->flags);

Is there a reason datagram_poll() should not check this? (Or another place
to do it)
If not I will make a new patch including this change.

Jeroen

Re: [PATCH] Using sock_orphan in ax25 and netrom

[David S. Miller]

Nobody can use your patch because your email client put line breaks
into all the long lines.