Sorry for the repeat email but this bug is also in 2.5.22 and my patch is still
valid, although I'm not entirely sure it is the correct fix for the problem?
-- From Previous email --
2.5.21 and 2.5.22 OOPS at boot on my test machine, the decoded OOPS is attached.
Also attached is a program that triggers the bug, it emulates the behavior
of bind on my test machine and binds to two ports one IPv4 and one IPv6
with the same port number but different IP addresses.
The bug appears to be the IPv6 TCP code, in net/ipv6/tcp_ipv6.c
Line 149:
struct ipv6_pinfo *np2 = inet6_sk(sk2);
if (sk != sk2 &&
sk->bound_dev_if == sk2->bound_dev_if) {
if (!sk_reuse ||
!sk2->reuse ||
sk2->state == TCP_LISTEN) {
/* NOTE: IPv6 tw bucket have different format */
if (!inet_sk(sk2)->rcv_saddr ||
addr_type == IPV6_ADDR_ANY ||
!ipv6_addr_cmp(&np->rcv_saddr,
sk2->state != TCP_TIME_WAIT ?
BUG --> &np2->rcv_saddr :
&((struct tcp_tw_bucket*)sk)->v6_rcv_saddr) ||
(addr_type==IPV6_ADDR_MAPPED && sk2->family==AF_INET &&
inet_sk(sk)->rcv_saddr ==
inet_sk(sk2)->rcv_saddr))
break;
}
}
}
np2 can be NULL if the socket is an IPv4 socket, since IPv6 and IPv4
share the port address space. While the test of !inet_sk(sk2-)->rcv_addr
_should_ prevent this it is not assured in C that the conditions to an if
statement will be evaluated in the order written? At least my gcc
(2.95.4 from compiled from CVS) doesn't think so :-).
I propose a diff similar to the one below to fix the problem maybe?
Many thanks,
Carl Ritson
[email protected]
--- orig/net/ipv6/tcp_ipv6.c Sat Jun 15 19:23:44 2002
+++ linux/net/ipv6/tcp_ipv6.c Sat Jun 15 19:21:46 2002
@@ -156,14 +156,16 @@
/* NOTE: IPv6 tw bucket have different format */
if (!inet_sk(sk2)->rcv_saddr ||
addr_type == IPV6_ADDR_ANY ||
- !ipv6_addr_cmp(&np->rcv_saddr,
- sk2->state != TCP_TIME_WAIT ?
- &np2->rcv_saddr :
- &((struct tcp_tw_bucket*)sk)->v6_rcv_saddr) ||
(addr_type==IPV6_ADDR_MAPPED && sk2->family==AF_INET &&
inet_sk(sk)->rcv_saddr ==
inet_sk(sk2)->rcv_saddr))
break;
+ if (np2 != NULL)
+ if (!ipv6_addr_cmp(&np->rcv_saddr,
+ sk2->state != TCP_TIME_WAIT ?
+ &np2->rcv_saddr :
+ &((struct tcp_tw_bucket*)sk)->v6_rcv_saddr))
+ break;
}
}
}
This is a known bug introduced by the struct sock splitup into
external per-protocol pieces done by Arnaldo de Melo. He is working
on the proper fix, your proposed change will just paper over the real
bug.
Em Mon, Jun 17, 2002 at 02:33:19PM -0700, David S. Miller escreveu:
>
> This is a known bug introduced by the struct sock splitup into
> external per-protocol pieces done by Arnaldo de Melo. He is working
> on the proper fix, your proposed change will just paper over the real
> bug.
Carl,
Can you try this patch?
- Arnaldo
--- orig/net/ipv6/tcp_ipv6.c Sat May 25 23:13:56 2002
+++ linux/net/ipv6/tcp_ipv6.c Fri Jun 14 23:23:07 2002
@@ -1240,6 +1240,7 @@
struct dst_entry *dst)
{
struct ipv6_pinfo *newnp, *np = inet6_sk(sk);
+ struct tcp6_sock *newtcp6sk;
struct flowi fl;
struct inet_opt *newinet;
struct tcp_opt *newtp;
@@ -1256,10 +1257,15 @@
if (newsk == NULL)
return NULL;
+ newtcp6sk = (struct tcp6_sock *)newsk;
+ newtcp6sk->pinet6 = &newtcp6sk->inet6;
+
newinet = inet_sk(newsk);
newnp = inet6_sk(newsk);
newtp = tcp_sk(newsk);
+ memcpy(newnp, np, sizeof(struct ipv6_pinfo));
+
ipv6_addr_set(&newnp->daddr, 0, 0, htonl(0x0000FFFF),
newinet->daddr);
@@ -1336,9 +1342,15 @@
ip6_dst_store(newsk, dst, NULL);
sk->route_caps = dst->dev->features&~NETIF_F_IP_CSUM;
+ newtcp6sk = (struct tcp6_sock *)newsk;
+ newtcp6sk->pinet6 = &newtcp6sk->inet6;
+
newtp = tcp_sk(newsk);
newinet = inet_sk(newsk);
newnp = inet6_sk(newsk);
+
+ memcpy(newnp, np, sizeof(struct ipv6_pinfo));
+
ipv6_addr_copy(&newnp->daddr, &req->af.v6_req.rmt_addr);
ipv6_addr_copy(&newnp->saddr, &req->af.v6_req.loc_addr);
ipv6_addr_copy(&newnp->rcv_saddr, &req->af.v6_req.loc_addr);
--
Russell King ([email protected]) The developer of ARM Linux
http://www.arm.linux.org.uk/personal/aboutme.html
From: Arnaldo Carvalho de Melo <[email protected]>
Date: Mon, 17 Jun 2002 21:57:35 -0300
--- orig/net/ipv6/tcp_ipv6.c Sat May 25 23:13:56 2002
+++ linux/net/ipv6/tcp_ipv6.c Fri Jun 14 23:23:07 2002
I've installed this change into my tree in the meantime.
If we find a better fix, we can just revert this.
Thanks.
Em Mon, Jun 17, 2002 at 07:17:26PM -0700, David S. Miller escreveu:
> From: Arnaldo Carvalho de Melo <[email protected]>
> Date: Mon, 17 Jun 2002 21:57:35 -0300
>
> --- orig/net/ipv6/tcp_ipv6.c Sat May 25 23:13:56 2002
> +++ linux/net/ipv6/tcp_ipv6.c Fri Jun 14 23:23:07 2002
>
> I've installed this change into my tree in the meantime.
> If we find a better fix, we can just revert this.
OK, I found a better fix, I think, that allowed me to kill inet6_sk_generic
in af_inet6.c, using a constructor for the tcpv6, udpv6 and raw6 slab caches,
as suggested by Russel King, I'll be sending RSN.
- Arnaldo
Em Mon, Jun 17, 2002 at 11:49:34PM -0300, Arnaldo C. Melo escreveu:
> Em Mon, Jun 17, 2002 at 07:17:26PM -0700, David S. Miller escreveu:
> > From: Arnaldo Carvalho de Melo <[email protected]>
> > Date: Mon, 17 Jun 2002 21:57:35 -0300
> >
> > --- orig/net/ipv6/tcp_ipv6.c Sat May 25 23:13:56 2002
> > +++ linux/net/ipv6/tcp_ipv6.c Fri Jun 14 23:23:07 2002
> >
> > I've installed this change into my tree in the meantime.
> > If we find a better fix, we can just revert this.
>
> OK, I found a better fix, I think, that allowed me to kill inet6_sk_generic
> in af_inet6.c, using a constructor for the tcpv6, udpv6 and raw6 slab caches,
> as suggested by Russel King, I'll be sending RSN.
Here it is, David, please consider pulling it from:
http://kernel-acme.bkbits.net:8080/tcpv6-pinet6
Best Regards,
- Arnaldo
# This is a BitKeeper generated patch for the following project:
# Project Name: Linux kernel tree
# This patch format is intended for GNU patch command version 2.5 or higher.
# This patch includes the following deltas:
# ChangeSet 1.656 -> 1.657
# net/ipv6/tcp_ipv6.c 1.17 -> 1.18
# net/ipv6/af_inet6.c 1.8 -> 1.9
#
# The following is the BitKeeper ChangeSet Log
# --------------------------------------------
# 02/06/18 [email protected] 1.657
# net/ipv6/af_inet6.c
# net/ipv6/tcp_ipv6.c
#
# - Add constructors for the tcp6_sk_cachep, udp6_sk_cachep and raw6_sk_cachep slab caches,
# to initialize ->pinet6 to the respective &->inet6, this simplifies inet6_create,
# making ipv6_sk_generic not needed and fixing a bug where we create a new tcpv6 socket
# outside inet6_create and we were not initializing ->pinet6 properly, thanks to
# Russell King for pointing out the bug and doing the initial analisys that made the
# fix easy to make.
# --------------------------------------------
#
diff -Nru a/net/ipv6/af_inet6.c b/net/ipv6/af_inet6.c
--- a/net/ipv6/af_inet6.c Mon Mar 11 11:13:05 2002
+++ b/net/ipv6/af_inet6.c Tue Jun 18 00:27:30 2002
@@ -134,23 +134,11 @@
return rc;
}
-static __inline__ struct ipv6_pinfo *inet6_sk_generic(struct sock *sk)
-{
- struct ipv6_pinfo *rc = (&((struct tcp6_sock *)sk)->inet6);
-
- if (sk->protocol == IPPROTO_UDP)
- rc = (&((struct udp6_sock *)sk)->inet6);
- else if (sk->protocol == IPPROTO_RAW)
- rc = (&((struct raw6_sock *)sk)->inet6);
- return rc;
-}
-
static int inet6_create(struct socket *sock, int protocol)
{
struct inet_opt *inet;
struct ipv6_pinfo *np;
struct sock *sk;
- struct tcp6_sock* tcp6sk;
struct list_head *p;
struct inet_protosw *answer;
@@ -212,8 +200,7 @@
sk->backlog_rcv = answer->prot->backlog_rcv;
- tcp6sk = (struct tcp6_sock *)sk;
- tcp6sk->pinet6 = np = inet6_sk_generic(sk);
+ np = inet6_sk(sk);
np->hop_limit = -1;
np->mcast_hops = -1;
np->mc_loop = 1;
@@ -632,6 +619,30 @@
inet_unregister_protosw(p);
}
+static void tcp6_pinet6_init(void *foo, kmem_cache_t *cachep,
+ unsigned long flags)
+{
+ struct tcp6_sock *tcp6sk = (struct tcp6_sock *)foo;
+
+ tcp6sk->pinet6 = &tcp6sk->inet6;
+}
+
+static void udp6_pinet6_init(void *foo, kmem_cache_t *cachep,
+ unsigned long flags)
+{
+ struct udp6_sock *udp6sk = (struct udp6_sock *)foo;
+
+ udp6sk->pinet6 = &udp6sk->inet6;
+}
+
+static void raw6_pinet6_init(void *foo, kmem_cache_t *cachep,
+ unsigned long flags)
+{
+ struct raw6_sock *raw6sk = (struct raw6_sock *)foo;
+
+ raw6sk->pinet6 = &raw6sk->inet6;
+}
+
static int __init inet6_init(void)
{
struct sk_buff *dummy_skb;
@@ -655,13 +666,16 @@
/* allocate our sock slab caches */
tcp6_sk_cachep = kmem_cache_create("tcp6_sock",
sizeof(struct tcp6_sock), 0,
- SLAB_HWCACHE_ALIGN, 0, 0);
+ SLAB_HWCACHE_ALIGN,
+ tcp6_pinet6_init, NULL);
udp6_sk_cachep = kmem_cache_create("udp6_sock",
sizeof(struct udp6_sock), 0,
- SLAB_HWCACHE_ALIGN, 0, 0);
+ SLAB_HWCACHE_ALIGN,
+ udp6_pinet6_init, NULL);
raw6_sk_cachep = kmem_cache_create("raw6_sock",
sizeof(struct raw6_sock), 0,
- SLAB_HWCACHE_ALIGN, 0, 0);
+ SLAB_HWCACHE_ALIGN,
+ raw6_pinet6_init, NULL);
if (!tcp6_sk_cachep || !udp6_sk_cachep || !raw6_sk_cachep)
printk(KERN_CRIT __FUNCTION__
": Can't create protocol sock SLAB caches!\n");
diff -Nru a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
--- a/net/ipv6/tcp_ipv6.c Wed May 22 15:16:37 2002
+++ b/net/ipv6/tcp_ipv6.c Tue Jun 18 00:27:30 2002
@@ -1260,6 +1260,8 @@
newnp = inet6_sk(newsk);
newtp = tcp_sk(newsk);
+ memcpy(newnp, np, sizeof(struct ipv6_pinfo));
+
ipv6_addr_set(&newnp->daddr, 0, 0, htonl(0x0000FFFF),
newinet->daddr);
@@ -1339,6 +1341,7 @@
newtp = tcp_sk(newsk);
newinet = inet_sk(newsk);
newnp = inet6_sk(newsk);
+ memcpy(newnp, np, sizeof(struct ipv6_pinfo));
ipv6_addr_copy(&newnp->daddr, &req->af.v6_req.rmt_addr);
ipv6_addr_copy(&newnp->saddr, &req->af.v6_req.loc_addr);
ipv6_addr_copy(&newnp->rcv_saddr, &req->af.v6_req.loc_addr);
Em Tue, Jun 18, 2002 at 12:58:04AM -0300, Arnaldo C. Melo escreveu:
> Em Mon, Jun 17, 2002 at 11:49:34PM -0300, Arnaldo C. Melo escreveu:
> > Em Mon, Jun 17, 2002 at 07:17:26PM -0700, David S. Miller escreveu:
> > > From: Arnaldo Carvalho de Melo <[email protected]>
> > > Date: Mon, 17 Jun 2002 21:57:35 -0300
> > >
> > > --- orig/net/ipv6/tcp_ipv6.c Sat May 25 23:13:56 2002
> > > +++ linux/net/ipv6/tcp_ipv6.c Fri Jun 14 23:23:07 2002
> > >
> > > I've installed this change into my tree in the meantime.
> > > If we find a better fix, we can just revert this.
> >
> > OK, I found a better fix, I think, that allowed me to kill inet6_sk_generic
> > in af_inet6.c, using a constructor for the tcpv6, udpv6 and raw6 slab caches,
> > as suggested by Russel King, I'll be sending RSN.
>
> Here it is, David, please consider pulling it from:
>
> http://kernel-acme.bkbits.net:8080/tcpv6-pinet6
>
> Best Regards,
>
> - Arnaldo
Oops, brain fart, David, please don't apply, in tcp_create_openreq_child
we overwrite the ->pinet6 field after the constructor inits it to the proper
value :( Please leave the old patch in place for a while... :(
- Arnaldo
From: Arnaldo Carvalho de Melo <[email protected]>
Date: Tue, 18 Jun 2002 01:15:39 -0300
Oops, brain fart, David, please don't apply, in tcp_create_openreq_child
we overwrite the ->pinet6 field after the constructor inits it to the proper
value :( Please leave the old patch in place for a while... :(
No problem.
On Mon, 17 Jun 2002, Arnaldo Carvalho de Melo wrote:
> Em Mon, Jun 17, 2002 at 02:33:19PM -0700, David S. Miller escreveu:
> >
> > This is a known bug introduced by the struct sock splitup into
> > external per-protocol pieces done by Arnaldo de Melo. He is working
> > on the proper fix, your proposed change will just paper over the real
> > bug.
I suspected something like that.
> Carl,
>
> Can you try this patch?
>
> - Arnaldo
>
> --- orig/net/ipv6/tcp_ipv6.c Sat May 25 23:13:56 2002
> +++ linux/net/ipv6/tcp_ipv6.c Fri Jun 14 23:23:07 2002
> <diff snipped>
This patch doesn't help, it produces exactly the same oops when decoded, I
assume the newer patch you mention to Dave is the correct fix?
Thanks,
Carl