Skip to contentSkip to conversation

AX.25 unaccepted socket makes problems

DISCUSSION Tihomir Heidelberg 3 participants 5 messages May 27, 2003 – May 28, 2003

lore
Share & tools
lore
#0 lore raw

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]

#1 lore raw
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

#2 lore raw

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_LISTEN when 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() and restore_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 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.

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]

#3 lore raw

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_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...

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

#4 lore raw
In reply to Tihomir Heidelberg
Quoted 3 lines

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 ?

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_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...

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