From: Bernard Pidoux <redacted>
Date: Tue, 30 Sep 2008 23:44:55 +0200

I suspect that the bug was unravelled when we added more than one
neighbour per route. The protocole accepts three, but this was not
much used during the early days since the density of radio stations
on the network was not big (only one node station per destination
address usually). The network is now denser with Internet links.

However, I don't understand why the test

if (rose->neighbour == neigh)

does not work, for
rose->neighbour = NULL;
should prevent next comparison to be valid and thus instruction
rose->neighbour->use--; not executed.

I have seen that a problem with sk_for_each() macro was identified a
while ago into ax25 code. The problem here could be similar ?

I took a look at this some more.

That neighbour case loop you mention does set ->neighbour to NULL.

But other paths do not.  Just look for all of the pieces of code
that do "rose->neighbour->use--;" and you'll find a few that do not
NULL it out.

One such example is rose_kill_by_device().

That would cause a problem because, while rose_disconnect() marks
the socket DEAD, it doesn't actually remove it from "rose_list".
That happens later when the user actually closes the socket or
some other similar event occurs.

Therefore if rose_kill_by_neigh() happens next, the ->neighbour could
match and we'll decrement again.

But I have no idea how safe it is to NULL out this ->neighbour
unconditionally.  Lots of code seems to deref the thing unconditionally.
For example the ROSE_STATE_2 handling in rose_release().

I guess since rose_disconnect() sets sk->sk_state to TCP_CLOSE, we'll
be OK here.

Can you try this patch?

diff --git a/net/rose/af_rose.c b/net/rose/af_rose.c
index a7f1ce1..41dd630 100644
--- a/net/rose/af_rose.c
+++ b/net/rose/af_rose.c

@@ -197,6 +197,7 @@ static void rose_kill_by_device(struct net_device *dev)
 		if (rose->device == dev) {
 			rose_disconnect(s, ENETUNREACH, ROSE_OUT_OF_ORDER, 0);
 			rose->neighbour->use--;
+			rose->neighbour = NULL;
 			rose->device = NULL;
 		}
 	}

@@ -625,6 +626,7 @@ static int rose_release(struct socket *sock)
 
 	case ROSE_STATE_2:
 		rose->neighbour->use--;
+		rose->neighbour = NULL;
 		release_sock(sk);
 		rose_disconnect(sk, 0, -1, -1);
 		lock_sock(sk);