Re: Fix for thread+network crashes in 2.0/2.1?

Henner Eisen (eis@baty.hanse.de)
28 Feb 1998 02:48:24 +0100


Hello,

Just a Tree <funaho@jurai.org> writes:

>
> >>EIP: c012ba56 <free_wait+2e/44>
> Trace: c012bcca <do_select+1ba/1d4>
> Trace: c012c015 <sys_select+331/4b4>

This oops extremly resembles the kind of oopses I'm suffering
for a long time when using 25 sockets. Thoses oopses occured inside the
inline function remove_wait_queue() which is called on behalf of
free_wait().

BTW: a trick that helped me to see the oops message at least on the console
was to remove the "wake_up_interruptible(&log_wait)" at the very end of
printk().

After inserting lots of printk()'s at various locations I realized
that the socket's inode is freed and cleaned while a select system
call on that socket is not finished yet. When the select system call
finishes the process tries to remove itsself from the wait queue.

However, the socket poll method uses

struct sock{
:
struct wait_queue **sleep;
:
}

and sleep points to the wait of

struct socket{
:
struct wait_queue *wait
:
}

The instance of struct socket is part of the inode

struct inode {
:
union {
:
struct socket socket_i;
:
} u;
}

Now, after the inode storage is reclaimed for other things the sk->sleep
is a dangling pointer, causing a lot of troble when the process tries to
remove itsself from the wait_queue when the select system call finishes.

The crash occurred within the x25-telnetd from the x25-utility package.
This is a hacked version of bsd telnetd where the tcp/ip sockets have
been replaced by x25 sockets. No threads involved (it does fork, however).

A fix that worked was to additionally increment the inode's i_count whenever
a new struct sock refers to a struct socket (and thus, to the inode).
For further details, the patch for af_x25.c is attached. (Well, that won't
fix the tcp/ip oops, but maybe it gives at least some ideas).

Henner

diff -u --recursive --new-file 2.1.85-i4ldev/net/x25/af_x25.c ix25/net/x25/af_x25.c
--- 2.1.85-i4ldev/net/x25/af_x25.c Tue Jan 6 20:21:21 1998
+++ ix25/net/x25/af_x25.c Wed Feb 11 19:30:02 1998
@@ -1,4 +1,20 @@
/*
+ Work around that fixes a problem which has shown up when using X.25 on
+ top of isdn. Before returning from a select system call (from x25 telnetd)
+ the kernel dereferences a null pointer (causing total lock up) when it
+ tries to remove the process (by calling free_wait()) from the the
+ socket's wait queue sk->sleep.
+ sk->sleep refers to storage inside the socket's inode. It turned out that
+ the socket's inode is freed and cleaned before the select system call is
+ finished. It's not yet known which code is resposible for that bug,
+ but incrementing the inode's use count helps.
+
+ Try if you can reproduce this bug when deactivating the fix by #undef'ining
+ this. I'm interested in your observations!
+ Please report them to eis@baty.hanse.de.
+ */
+#define X25_ISDN_FIXES
+/*
* X.25 Packet Layer release 002
*
* This is ALPHA test software. This code may break your machine, randomly fail to work with new
@@ -334,6 +350,10 @@
sk->timer.data = (unsigned long)sk;
add_timer(&sk->timer);
} else {
+#ifdef X25_ISDN_FIXES
+ /* freed sk no longer refers the socket inode's wait queue*/
+ iput(sk->socket->inode);
+#endif
sk_free(sk);
MOD_DEC_USE_COUNT;
}
@@ -463,6 +483,12 @@

sock_init_data(sock, sk);

+#ifdef X25_ISDN_FIXES
+ /* Hack to prevent socket beeing freed while sk->sleep refers to
+ sock->wait. The socket's sock->wait is stored inside the inode,
+ thus increment use count. */
+ sock->inode->i_count++;
+#endif
init_timer(&x25->timer);

sock->ops = &x25_proto_ops;
@@ -519,6 +545,12 @@

x25->qbitincl = osk->protinfo.x25->qbitincl;

+#ifdef X25_ISDN_FIXES
+ /* Hack to prevent socket beeing freed while sk->sleep refers to
+ sock->wait. The socket's sock->wait is stored inside the inode,
+ thus increment use count. */
+ sk->socket->inode->i_count++;
+#endif
init_timer(&x25->timer);

return sk;
@@ -557,7 +589,12 @@
}

sock->sk = NULL;
+#ifndef X25_ISDN_FIXES
sk->socket = NULL; /* Not used, but we should do this */
+#else
+ /* still used because we need to decrement the socket's inode
+ use count later when sk is destroyed */
+#endif

return 0;
}

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.rutgers.edu