AX.25 unaccepted socket makes problems
Hi linux-hams,
finaly I found a way how to produce AX.25 kernel get mad, I mean, when "cat /proc/net/ax25" produce segmentation fault, and everything got unstable.
If your program bind ax.25 socket for listening, other station connects to that listening socket and if listening socket dies without accepting new connection from you program, after short period the things got mad...
In ax25_destroy_socket function (af_ax25.c), there is a while loop for dequeuing those unaccepted connections and preparing them to die when heardbeat catch them. Also, there is code in ax25_std_heartbeat_expiry function (ax25_std_timer.c) that should destroy those connections, unfortunately those connection are NOT destroyed here ! A condition:
(ax25->sk->state == TCP_LISTEN && ax25->sk->dead)
is not true because state of those connections are not TCP_LISTEN. State is TCP_ESTABLISHED or TCP_CLOSE, depending if other end disconnected or not. Cannot get the reason why segmentation fault is happening, but hope someone else can find a reason when problem is reproducable.
To avoid this problem I added to ax25_destroy_socket function (af_ax25.c) following:
if (skb->sk != ax25->sk) {+ skb->sk->state = TCP_LISTEN;
+ if (!skb->sk->dead) {
+ skb->sk->state_change(skb->sk);
+ }
skb->sk->dead = 1;
ax25_start_heartbeat(skb->sk->protinfo.ax25);
skb->sk->protinfo.ax25->state = AX25_STATE_0;
}I am pretty sure that this is not the way how problem should be fixed, there should be a different way how to mark socket for destroying, but at least, I got rid of this problem.
Also, some months ago I mention here that regulary I get this AX.25 kernel behavior after few days of running 9A0TCP gateway machine. I noticed that very often ax25d died or had to restart ax25d because it was not handling connections. Think this bind/non-accept kernel problem is very probably the reason.
Anyone agree with me ? Can anyone make a resonable patch/fix for this problem ? Maybe setting skb->sk->destroy to 1 is right way ?
73 de Tihomir Heidelberg, [email protected]
Quoted 3 lines
are NOT destroyed here ! A condition:
(ax25->sk->state == TCP_LISTEN && ax25->sk->dead)is not true because state of those connections are not
as far as i got into the code when i looked for the reason for the segfault problem, former connected sessions come to the state TCP_LISTEN when they get disconnected.
Review
Quoted 4 lines
(quoted diff) +4
-0+ skb->sk->state = TCP_LISTEN; + if (!skb->sk->dead) { + skb->sk->state_change(skb->sk); + }
by insuring with with your patch should at least do not harm.
perhaps there's the problem for this phaenome:
| DB0TUD-0 DB0TUD-1 ax0 LISTENING 007/005 0 320 | DL6MPG-1 DB0TUD-15 ax0 LISTENING 005/007 0 432 | DB0TUD-0 DB0TUD-4 ax0 LISTENING 002/005 0 80 | DB0DSD-0 DB0TUD-1 ax0 LISTENING 001/000 0 80 | DL6MPG-4 DB0TUD-15 ax0 LISTENING 002/001 0 816
Quoted 3 lines
Cannot get the reason why
segmentation fault is happening, but hope someone else can find a reason when problem is reproducable.
as i noted abt a month ago in a mail to this list, i tracked the problem down to the following point:
there's a list of active ax25 control blocks (ax25_cb *ax25_list). this list gets corrupted. while walking through ax25_list->next->....->next, somewhere in this reference is pointing to somewhere where it should not.
ax25_get_info(), which is called when doing "netstat -a" or
"cat /proc/net/ax25" from userspace, walks through this list. i had a chance to let show me the data of all ax25_cb elements while walking through the list (after i spicked my kernel with debug routines). somewhere in the middle of the list, the reference to "next" was inkonsistant in comparison to the time it was before, and the kernel oopsed while walking over the next list element pointer.
i looked several times over the code (routines adding and removing list elements, checking for cli() and restore_flags()). everything looks ok for me.
currently, i assume this error is such difficile, because it may not be caused by the routines which change the list, but because _maybe_ another routine in ax25.o allocates a buffer / struct, writes more data as it should to it, and overwrites parts of the struct where the ax25 control block is stored, causing corrupted data in the linked list.
[as i mentioned also, ax25rtd which adds ax25 route lists to the kernel, causes troubles to the kernel. perhaps it's one of those routines which messes up the ax25 cb lists as side effect]
btw, a few days ago i announced a bugfix (the [invalid] to [invalid] SABM problem). it does fix the problem. but my hope that the kernel oopses will go away was not proven: they still occured.
Quoted 5 lines
Also, some months ago I mention here that regulary I get this AX.25 kernel behavior after few days of running 9A0TCP gateway machine. I noticed that very often ax25d died or had to restart ax25d because it was not handling connections. Think this bind/non-accept kernel problem is very probably the reason.
afik, it's caused by the inkonsistent list:ax25_find_listener() walks over ax25_list. and may not find the ax25 control block which actually listens.ax25_find_listener() is called by ax25_rcv() in ax25_in.c when an SABM is received.
a linux with a flexnet digipeater as neighbour (which connects quite often for link test probes) gets most oopses this way.
on the other hand, an ax25 socket which is listening for incoming connections, for e.g. by ax25d, gets sometimes
[pid 4924] accept(7, 0xbffff714, [72]) = -1 ECONNABORTED (Software caused connection abort) that should never happen.
73,
- thomas dl9sau
Hi,
Quoted 7 lines
Quoted 3 lines
are NOT destroyed here ! A condition:
(ax25->sk->state == TCP_LISTEN && ax25->sk->dead)is not true because state of those connections are not
as far as i got into the code when i looked for the reason for the segfault problem, former connected sessions come to the state
TCP_LISTENwhen they get disconnected.
I couldnt get TCP_LISTEN state, ax25_rcv in ax25_in.c set that state to TCP_ESTABLISHED, dont see where it can be changed to TCP_LISTEN, debugging messages reports TCP_ESTABLISHED or TCP_CLOSE, depending if remote station disconnected or not.
Quoted 6 lines
Review
Quoted 4 lines
(quoted diff) +4
-0+ skb->sk->state = TCP_LISTEN; + if (!skb->sk->dead) { + skb->sk->state_change(skb->sk); + }by insuring with with your patch should at least do not harm.
yap, but TCP_LISTEN state is not proper state for such connections, so think there should be more reasonable fix for this
also I think that skb->sk->pair should be set to NULL, because from this point the pair (listening socket) does not exist any more and reference to this can cause problem, right ?
Quoted 3 lines
there's a list of active ax25 control blocks (
ax25_cb*ax25_list). this list gets corrupted. while walking through ax25_list->next->....->next, somewhere in this reference is pointing to somewhere where it should not.
...
Quoted 3 lines
i looked several times over the code (routines adding and removing list elements, checking for
cli()andrestore_flags()). everything looks ok for me.
yap, I also spent a lot of hours looking in code that handle this list, but cannot find anything that can hurt that list
Quoted 3 lines
[as i mentioned also, ax25rtd which adds ax25 route lists to the kernel, causes troubles to the kernel. perhaps it's one of those routines which messes up the ax25 cb lists as side effect]
ax25rtd/axparms calls ioctl(SIOCADDRT) on ax.25 socket, I see that ax25_route_list is not protected with cli() stuff, maybe we should protect this list too ? dont know how this can hurt ax25_list, but protecting this list will not hurt anyone...
anyway, I read that Ralf is doing some things in 2.5 kernel tree, spinlock instead cli() protection should be used in future ax.25 kernel, as I read Ralf works on spinlock in ax.25 code... should we move to spinlocks in 2.4 kernels too or we will wait for 2.6 ?
Quoted 11 lines
Quoted 5 lines
Also, some months ago I mention here that regulary I get this AX.25 kernel behavior after few days of running 9A0TCP gateway machine. I noticed that very often ax25d died or had to restart ax25d because it was not handling connections. Think this bind/non-accept kernel problem is very probably the reason.
afik, it's caused by the inkonsistent list:
ax25_find_listener()walks overax25_list. and may not find the ax25 control block which actually listens.ax25_find_listener()is called byax25_rcv()inax25_in.cwhen an SABM is received.
yup, but if ax25d stops accepting connections, you will get ooops, dont know what came first, ax25d problem or kernel problem... but for sure ax25d problem may cause kernel problem...
73 de Tihomir Heidelberg, [email protected]
Hi,
I've got a few odd bits of patches for AX.25 for 2.5 kernels that I'm intending to feed into the kernel shortly. I'll take a look at the problem you've brought up at the same time as I'd like to have that solved. My main interest though is 2.5 rather than fixing up 2.4.
[snip]
Quoted 10 lines
Quoted 4 lines
[as i mentioned also, ax25rtd which adds ax25 route lists to the kernel, causes troubles to the kernel. perhaps it's one of those routines which messes up the ax25 cb lists as side effect]
ax25rtd/axparms calls ioctl(SIOCADDRT) on ax.25 socket, I see that
ax25_route_listis not protected withcli()stuff, maybe we should protect this list too ? dont know how this can hurtax25_list, but protecting this list will not hurt anyone...
Quoted 5 lines
anyway, I read that Ralf is doing some things in 2.5 kernel tree, spinlock instead
cli()protection should be used in future ax.25 kernel, as I read Ralf works on spinlock in ax.25 code... should we move to spinlocks in 2.4 kernels too or we will wait for 2.6 ?
Yes, some of his work has made it in, some more can be found in the linux-hams bkbits tree. If you are looking at structures that are not currently protected in 2.4, then I'd suggest looking at spinlocks right away. You might find the solution in Ralf's work for 2.5 and it would be a good idea to try and keep 2.4 similar to 2.5 where possible.
The last day or two I've been quite busy and I hope to have some time to put out my patches for testing shortly,
Steve.
G7RRM
Quoted 3 lines
also I think that
skb->sk->pairshould be set to NULL, because from this point the pair (listening socket) does not exist any more and reference to this can cause problem, right ?
hmm. the listening socket exists as long the userspace application does listen().
but i do not really understand the sk->pair concept.
also i have problems in understanding the concept in ax25_destroy_socket():skb->sk->dead is set to 1, then ax25_start_heartbeat() is called, which is a timer (which expires in +5s).
if we set it to skb->sk->state = TCP_LISTEN and our state is AX25_STATE_0, then this timer will call ax25_destroy_socket() again. due to the timer it is not a loop (hopefully), but after our first ax25_destroy_socket() goes further down (ax25->sk == NULL), we will call ax25_free_cb(ax25). what will our timer do, which we restarted above and which will work on this ax25 control block?
and: if ax25->sk is != NULL and there are no in/outstanding buffer, there's only an sk_free(ax25->sk). the socket is not destroyed, but ax25->sk is not set to NULL. does it refer now to an unassigned memory segment? should'n in this case ax25_free_cb() called too? here also the problem with a potentially running ax25_std_heartbeat_expiry(), which will use ax25->sk->... we have just free'd.
well, i deeply hope there's someone out there who knows how this code works ;)
Quoted 8 lines
Quoted 3 lines
[as i mentioned also, ax25rtd which adds ax25 route lists to the kernel, causes troubles to the kernel. perhaps it's one of those routines which messes up the ax25 cb lists as side effect]
ax25rtd/axparms calls ioctl(SIOCADDRT) on ax.25 socket, I see that
ax25_route_listis not protected withcli()stuff, maybe we should protect this list too ? dont know how this can hurtax25_list, but protecting this list will not hurt anyone...
just thought the same some weeks ago.
i have done this (interested in?). after the observation that when ax25rtd is running, the oops problem on ax25 cb's (which are independend of the ax25 route list) occorued.
unfotunately, i never found a way to produce the corrupted ax25 cb list by force. thus i only could wait for oopses. and my system where i tried this never oopsed :(
Quoted 4 lines
anyway, I read that Ralf is doing some things in 2.5 kernel tree, spinlock instead
cli()protection should be used in future ax.25 kernel, as I read Ralf works on spinlock in ax.25 code... should we move to spinlocks in 2.4 kernels too or we will wait for 2.6 ?
i'd advice not to hack too much in the current code. we may get more problems or side-effects.
73,
- thomas