On Sun, Oct 16, 2005 at 12:34:57AM -0700, David S. Miller wrote:
>
> Likely someone sets skb->sk in dccp without grabbing a reference
> count to the socket. Socket gets early free'd up, and later
> ICMP does a local response deref'ing the now gone skb->sk.
>
> Just a theory :-)
>
> This is why I don't think we should merge Herbert's fix just
> yet. If the skb->sk handling is still hosed, then it's still
> just a half-fix.
Oh ye of little faith :)
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 <[EMAIL PROTECTED]>
BTW Ian, did you get a "Code" line in the Oops? If you did please
include it in future reports because without it it's difficult to
pin down a crash to a specific spot inside a function.
Thanks,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[EMAIL PROTECTED]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
diff --git a/net/dccp/output.c b/net/dccp/output.c
--- a/net/dccp/output.c
+++ b/net/dccp/output.c
@@ -243,7 +243,8 @@ int dccp_write_xmit(struct sock *sk, str
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;
}
diff --git a/net/dccp/proto.c b/net/dccp/proto.c
--- a/net/dccp/proto.c
+++ b/net/dccp/proto.c
@@ -402,8 +402,6 @@ int dccp_sendmsg(struct kiocb *iocb, str
* This bug was _quickly_ found & fixed by just looking at an OSTRA
* generated callgraph 8) -acme
*/
- if (rc != 0)
- goto out_discard;
out_release:
release_sock(sk);
return rc ? : len;
