2001-11-20 14:46:48

by Jeroen Vreeken

[permalink] [raw]
Subject: [PATCH] Using sock_orphan in ax25 and netrom

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)



2001-11-21 18:42:49

by Jeroen Vreeken

[permalink] [raw]
Subject: Re: [PATCH] Using sock_orphan in ax25 and netrom

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


2001-11-21 19:47:21

by Jeroen Vreeken

[permalink] [raw]
Subject: Re: [PATCH] Using sock_orphan in ax25 and netrom

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


2001-11-29 07:42:26

by David Miller

[permalink] [raw]
Subject: Re: [PATCH] Using sock_orphan in ax25 and netrom


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