From fda0fd6c5b722cc48e904e0daafedca275d332af Mon Sep 17 00:00:00 2001 From: Herbert Xu Date: Fri, 14 Oct 2005 16:38:49 +1000 Subject: [DCCP]: Use skb_set_owner_w in dccp_transmit_skb when skb->sk is NULL David S. Miller wrote: > One thing you can probably do for this bug is to mark data packets > explicitly somehow, perhaps in the SKB control block DCCP already > uses for other data. Put some boolean in there, set it true for > data packets. Then change the test in dccp_transmit_skb() as > appropriate to test the boolean flag instead of "skb_cloned(skb)". I agree. In fact we already have that flag, it's called skb->sk. So here is patch to test that instead of skb_cloned(). Signed-off-by: Herbert Xu Acked-by: Ian McDonald Signed-off-by: Arnaldo Carvalho de Melo --- net/dccp/output.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) (limited to 'net/dccp/output.c') diff --git a/net/dccp/output.c b/net/dccp/output.c index 4786bdc..946ec2d 100644 --- a/net/dccp/output.c +++ b/net/dccp/output.c @@ -62,10 +62,8 @@ int dccp_transmit_skb(struct sock *sk, struct sk_buff *skb) skb->h.raw = skb_push(skb, dccp_header_size); dh = dccp_hdr(skb); - /* - * Data packets are not cloned as they are never retransmitted - */ - if (skb_cloned(skb)) + + if (!skb->sk) skb_set_owner_w(skb, sk); /* Build DCCP header and checksum it. */ -- cgit v1.1 From ffa29347dfbc158d1f47f5925324a6f5713659c1 Mon Sep 17 00:00:00 2001 From: Herbert Xu Date: Sun, 16 Oct 2005 21:08:46 +1000 Subject: [DCCP]: Make dccp_write_xmit always free the packet icmp_send doesn't use skb->sk at all so even if skb->sk has already been freed it can't cause crash there (it would've crashed somewhere else first, e.g., ip_queue_xmit). I found a double-free on an skb that could explain this though. dccp_sendmsg and dccp_write_xmit are a little confused as to what should free the packet when something goes wrong. Sometimes they both go for the ball and end up in each other's way. This patch makes dccp_write_xmit always free the packet no matter what. This makes sense since dccp_transmit_skb which in turn comes from the fact that ip_queue_xmit always frees the packet. Signed-off-by: Herbert Xu Signed-off-by: Arnaldo Carvalho de Melo --- net/dccp/output.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'net/dccp/output.c') diff --git a/net/dccp/output.c b/net/dccp/output.c index 946ec2d..7006549 100644 --- a/net/dccp/output.c +++ b/net/dccp/output.c @@ -241,7 +241,8 @@ int dccp_write_xmit(struct sock *sk, struct sk_buff *skb, long *timeo) err = dccp_transmit_skb(sk, skb); ccid_hc_tx_packet_sent(dp->dccps_hc_tx_ccid, sk, 0, len); - } + } else + kfree_skb(skb); return err; } -- cgit v1.1 From 49c5bfaffe8ae6e6440dc4bf78b03800960d93f5 Mon Sep 17 00:00:00 2001 From: Herbert Xu Date: Tue, 18 Oct 2005 12:03:28 +1000 Subject: [DCCP]: Clear the IPCB area Turns out the problem has nothing to do with use-after-free or double-free. It's just that we're not clearing the CB area and DCCP unlike TCP uses a CB format that's incompatible with IP. Signed-off-by: Herbert Xu Signed-off-by: Ian McDonald Signed-off-by: Arnaldo Carvalho de Melo --- net/dccp/output.c | 1 + 1 file changed, 1 insertion(+) (limited to 'net/dccp/output.c') diff --git a/net/dccp/output.c b/net/dccp/output.c index 7006549..2925074 100644 --- a/net/dccp/output.c +++ b/net/dccp/output.c @@ -100,6 +100,7 @@ int dccp_transmit_skb(struct sock *sk, struct sk_buff *skb) DCCP_INC_STATS(DCCP_MIB_OUTSEGS); + memset(&(IPCB(skb)->opt), 0, sizeof(IPCB(skb)->opt)); err = ip_queue_xmit(skb, 0); if (err <= 0) return err; -- cgit v1.1 From 7d877f3bda870ab5f001bd92528654471d5966b3 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Fri, 21 Oct 2005 03:20:43 -0400 Subject: [PATCH] gfp_t: net/* Signed-off-by: Al Viro Signed-off-by: Linus Torvalds --- net/dccp/output.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'net/dccp/output.c') diff --git a/net/dccp/output.c b/net/dccp/output.c index 2925074..d59f86f 100644 --- a/net/dccp/output.c +++ b/net/dccp/output.c @@ -495,7 +495,7 @@ void dccp_send_close(struct sock *sk, const int active) { struct dccp_sock *dp = dccp_sk(sk); struct sk_buff *skb; - const unsigned int prio = active ? GFP_KERNEL : GFP_ATOMIC; + const gfp_t prio = active ? GFP_KERNEL : GFP_ATOMIC; skb = alloc_skb(sk->sk_prot->max_header, prio); if (skb == NULL) -- cgit v1.1