[PATCH] Fix NULL dereference in tcp_4_send_ack()

[PATCH] Fix NULL dereference in tcp_4_send_ack()

[Vitaliy Gusev]

Fix NULL dereference in tcp_4_send_ack().

As skb->dev is reset to NULL in tcp_v4_rcv() thus OOPS occurs:

BUG: unable to handle kernel NULL pointer dereference at 00000000000004d0
IP: [<ffffffff80498503>] tcp_v4_send_ack+0x203/0x250

Stack:  ffff810005dbb000 ffff810015c8acc0 e77b2c6e5f861600 a01610802e90cb6d
 0a08010100000000 88afffff88afffff 0000000080762be8 0000000115c872e8
 0004122000000000 0000000000000001 ffffffff80762b88 0000000000000020
Call Trace:
 <IRQ>  [<ffffffff80499c33>] tcp_v4_reqsk_send_ack+0x20/0x22
 [<ffffffff8049bce5>] tcp_check_req+0x108/0x14c
 [<ffffffff8047aaf7>] ? rt_intern_hash+0x322/0x33c
 [<ffffffff80499846>] tcp_v4_do_rcv+0x399/0x4ec
 [<ffffffff8045ce4b>] ? skb_checksum+0x4f/0x272
 [<ffffffff80485b74>] ? __inet_lookup_listener+0x14a/0x15c
 [<ffffffff8049babc>] tcp_v4_rcv+0x6a1/0x701
 [<ffffffff8047e739>] ip_local_deliver_finish+0x157/0x24a
 [<ffffffff8047ec9a>] ip_local_deliver+0x72/0x7c
 [<ffffffff8047e5bd>] ip_rcv_finish+0x38d/0x3b2
 [<ffffffff803d3548>] ? scsi_io_completion+0x19d/0x39e
 [<ffffffff8047ebe5>] ip_rcv+0x2a2/0x2e5
 [<ffffffff80462faa>] netif_receive_skb+0x293/0x303
 [<ffffffff80465a9b>] process_backlog+0x80/0xd0
 [<ffffffff802630b4>] ? __rcu_process_callbacks+0x125/0x1b4
 [<ffffffff8046560e>] net_rx_action+0xb9/0x17f
 [<ffffffff80234cc5>] __do_softirq+0xa3/0x164
 [<ffffffff8020c52c>] call_softirq+0x1c/0x28
 <EOI>  [<ffffffff8020de1c>] do_softirq+0x34/0x72
 [<ffffffff80234b8e>] local_bh_enable_ip+0x3f/0x50
 [<ffffffff804d43ca>] _spin_unlock_bh+0x12/0x14
 [<ffffffff804599cd>] release_sock+0xb8/0xc1
 [<ffffffff804a6f9a>] inet_stream_connect+0x146/0x25c
 [<ffffffff80243078>] ? autoremove_wake_function+0x0/0x38
 [<ffffffff8045751f>] sys_connect+0x68/0x8e
 [<ffffffff80291818>] ? fd_install+0x5f/0x68
 [<ffffffff80457784>] ? sock_map_fd+0x55/0x62
 [<ffffffff8020b39b>] system_call_after_swapgs+0x7b/0x80

Code: 41 10 11 d0 83 d0 00 4d 85 ed 89 45 c0 c7 45 c4 08 00 00 00 74 07 41 8b 45 04 89 45 c8 48 8b 43 20 8b 4d b8 48 8d 55 b0 48 89 de <48> 8b 80 d0 04 00 00 48 8b b8 60 01 00 00 e8 20 ae fe ff 65 48
RIP  [<ffffffff80498503>] tcp_v4_send_ack+0x203/0x250
 RSP <ffffffff80762b78>
CR2: 00000000000004d0


Signed-off-by: Vitaliy Gusev <vgusev@openvz.org>

---
diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index 1b4fee2..011478e 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -618,7 +618,7 @@ static void tcp_v4_send_ack(struct sk_buff *skb, u32 seq, u32 ack,
 			];
 	} rep;
 	struct ip_reply_arg arg;
-	struct net *net = dev_net(skb->dev);
+	struct net *net = dev_net(skb->dst->dev);
 
 	memset(&rep.th, 0, sizeof(struct tcphdr));
 	memset(&arg, 0, sizeof(arg));

[PATCH net-2.6] ip: NULL pointer dereferrence in tcp_v(4|6)_send_ack

[Denis V. Lunev]

The following actions are possible:
tcp_v4_rcv
  skb->dev = NULL;
  tcp_v4_do_rcv
    tcp_v4_hnd_req
      tcp_check_req
        req->rsk_ops->send_ack == tcp_v4_send_ack

So, skb->dev can be NULL in tcp_v4_send_ack. We must obtain namespace
from dst entry. IPv6 codepath is similar.

Thanks to Vitaliy Gusev <vgusev@openvz.org> for initial oops decoding.

Signed-off-by: Denis V. Lunev <den@openvz.org>
---
 net/ipv4/tcp_ipv4.c |    2 +-
 net/ipv6/tcp_ipv6.c |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index 1b4fee2..011478e 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -618,7 +618,7 @@ static void tcp_v4_send_ack(struct sk_buff *skb, u32 seq, u32 ack,
 			];
 	} rep;
 	struct ip_reply_arg arg;
-	struct net *net = dev_net(skb->dev);
+	struct net *net = dev_net(skb->dst->dev);
 
 	memset(&rep.th, 0, sizeof(struct tcphdr));
 	memset(&arg, 0, sizeof(arg));
diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
index b585c85..10e22fd 100644
--- a/net/ipv6/tcp_ipv6.c
+++ b/net/ipv6/tcp_ipv6.c
@@ -1050,7 +1050,7 @@ static void tcp_v6_send_ack(struct sk_buff *skb, u32 seq, u32 ack, u32 win, u32
 	struct tcphdr *th = tcp_hdr(skb), *t1;
 	struct sk_buff *buff;
 	struct flowi fl;
-	struct net *net = dev_net(skb->dev);
+	struct net *net = dev_net(skb->dst->dev);
 	struct sock *ctl_sk = net->ipv6.tcp_sk;
 	unsigned int tot_len = sizeof(struct tcphdr);
 	__be32 *topt;
-- 
1.5.6.4

Re: [PATCH net-2.6] ip: NULL pointer dereferrence in tcp_v(4|6)_send_ack

[David Miller]

From: "Denis V. Lunev" <den@openvz.org>
Date: Tue, 30 Sep 2008 20:29:11 +0400

> The following actions are possible:
> tcp_v4_rcv
>   skb->dev = NULL;
>   tcp_v4_do_rcv
>     tcp_v4_hnd_req
>       tcp_check_req
>         req->rsk_ops->send_ack == tcp_v4_send_ack
> 
> So, skb->dev can be NULL in tcp_v4_send_ack. We must obtain namespace
> from dst entry. IPv6 codepath is similar.
> 
> Thanks to Vitaliy Gusev <vgusev@openvz.org> for initial oops decoding.
> 
> Signed-off-by: Denis V. Lunev <den@openvz.org>

Vitaliy sent the same patch first, so I applied his copy :-)

Thanks everyone.

Re: [PATCH net-2.6] ip: NULL pointer dereferrence in tcp_v(4|6)_send_ack

[David Miller]

From: Vitaliy Gusev <vgusev@openvz.org>
Date: Wed, 1 Oct 2008 13:06:15 +0400

> On 1 October 2008 12:52:46 David Miller wrote:
> > Vitaliy sent the same patch first, so I applied his copy :-)
> 
> Den's patch is not the same. My patch didn't fix IPv6 code.

Whoops... Denis can you resubmit just the ipv6 side
fix then?  I already pushed Vitaliy's commit out
to net-2.6

Thanks.

Re: [PATCH net-2.6] ip: NULL pointer dereferrence in tcp_v(4|6)_send_ack

[Vitaliy Gusev]

On 1 October 2008 12:52:46 David Miller wrote:
> From: "Denis V. Lunev" <den@openvz.org>
> Date: Tue, 30 Sep 2008 20:29:11 +0400
> 
> > The following actions are possible:
> > tcp_v4_rcv
> >   skb->dev = NULL;
> >   tcp_v4_do_rcv
> >     tcp_v4_hnd_req
> >       tcp_check_req
> >         req->rsk_ops->send_ack == tcp_v4_send_ack
> > 
> > So, skb->dev can be NULL in tcp_v4_send_ack. We must obtain namespace
> > from dst entry. IPv6 codepath is similar.
> > 
> > Thanks to Vitaliy Gusev <vgusev@openvz.org> for initial oops decoding.
> > 
> > Signed-off-by: Denis V. Lunev <den@openvz.org>
> 
> Vitaliy sent the same patch first, so I applied his copy :-)

Den's patch is not the same. My patch didn't fix IPv6 code.

> 
> Thanks everyone.
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 



-- 
Thank,
Vitaliy Gusev

[PATCH net-2.6] ipv6: NULL pointer dereferrence in tcp_v6_send_ack

[Denis V. Lunev]

The following actions are possible:
tcp_v6_rcv
  skb->dev = NULL;
  tcp_v6_do_rcv
    tcp_v6_hnd_req
      tcp_check_req
        req->rsk_ops->send_ack == tcp_v6_send_ack

So, skb->dev can be NULL in tcp_v6_send_ack. We must obtain namespace
from dst entry.

Thanks to Vitaliy Gusev <vgusev@openvz.org> for initial problem finding
in IPv4 code.

Signed-off-by: Denis V. Lunev <den@openvz.org>
---
 net/ipv4/tcp_ipv4.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
index b585c85..10e22fd 100644
--- a/net/ipv6/tcp_ipv6.c
+++ b/net/ipv6/tcp_ipv6.c
@@ -1050,7 +1050,7 @@ static void tcp_v6_send_ack(struct sk_buff *skb, u32 seq, u32 ack, u32 win, u32
 	struct tcphdr *th = tcp_hdr(skb), *t1;
 	struct sk_buff *buff;
 	struct flowi fl;
-	struct net *net = dev_net(skb->dev);
+	struct net *net = dev_net(skb->dst->dev);
 	struct sock *ctl_sk = net->ipv6.tcp_sk;
 	unsigned int tot_len = sizeof(struct tcphdr);
 	__be32 *topt;
-- 
1.5.6.4

Re: [PATCH net-2.6] ipv6: NULL pointer dereferrence in tcp_v6_send_ack

[David Miller]

From: "Denis V. Lunev" <den@openvz.org>
Date: Wed,  1 Oct 2008 13:11:57 +0400

> The following actions are possible:
> tcp_v6_rcv
>   skb->dev = NULL;
>   tcp_v6_do_rcv
>     tcp_v6_hnd_req
>       tcp_check_req
>         req->rsk_ops->send_ack == tcp_v6_send_ack
> 
> So, skb->dev can be NULL in tcp_v6_send_ack. We must obtain namespace
> from dst entry.
> 
> Thanks to Vitaliy Gusev <vgusev@openvz.org> for initial problem finding
> in IPv4 code.
> 
> Signed-off-by: Denis V. Lunev <den@openvz.org>

Applied, thanks for resending this part for me.

Re: [PATCH net-2.6] ipv6: NULL pointer dereferrence in tcp_v6_send_ack

[Arnaldo Carvalho de Melo]

Em Wed, Oct 01, 2008 at 01:11:57PM +0400, Denis V. Lunev escreveu:
> The following actions are possible:
> tcp_v6_rcv
>   skb->dev = NULL;
>   tcp_v6_do_rcv
>     tcp_v6_hnd_req
>       tcp_check_req
>         req->rsk_ops->send_ack == tcp_v6_send_ack
> 
> So, skb->dev can be NULL in tcp_v6_send_ack. We must obtain namespace
> from dst entry.
> 
> Thanks to Vitaliy Gusev <vgusev@openvz.org> for initial problem finding
> in IPv4 code.
> 
> Signed-off-by: Denis V. Lunev <den@openvz.org>
> ---
>  net/ipv4/tcp_ipv4.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)

Forgot to update the diffstat? :-) Nah, just nitpicking :-P

> diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
> index b585c85..10e22fd 100644
> --- a/net/ipv6/tcp_ipv6.c
> +++ b/net/ipv6/tcp_ipv6.c
> @@ -1050,7 +1050,7 @@ static void tcp_v6_send_ack(struct sk_buff *skb, u32 seq, u32 ack, u32 win, u32
>  	struct tcphdr *th = tcp_hdr(skb), *t1;
>  	struct sk_buff *buff;
>  	struct flowi fl;
> -	struct net *net = dev_net(skb->dev);
> +	struct net *net = dev_net(skb->dst->dev);
>  	struct sock *ctl_sk = net->ipv6.tcp_sk;
>  	unsigned int tot_len = sizeof(struct tcphdr);
>  	__be32 *topt;
> -- 
> 1.5.6.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH net-2.6] ipv6: NULL pointer dereferrence in tcp_v6_send_ack

[David Miller]

From: Arnaldo Carvalho de Melo <acme@redhat.com>
Date: Wed, 1 Oct 2008 10:34:09 -0300

> Em Wed, Oct 01, 2008 at 01:11:57PM +0400, Denis V. Lunev escreveu:
> > Signed-off-by: Denis V. Lunev <den@openvz.org>
> > ---
> >  net/ipv4/tcp_ipv4.c |    2 +-
> >  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> Forgot to update the diffstat? :-) Nah, just nitpicking :-P

Now I don't feel so bad about not noticing this :)