Commit ec764bf083a6ff396234351b51fd236f53c903bf
Committed by
David S. Miller
David S. Miller
1 parent
2e4ceec4ed
Exists in
master
and in
4 other branches
net: tracepoint of net_dev_xmit sees freed skb and causes panic
Because there is a possibility that skb is kfree_skb()ed and zero cleared after ndo_start_xmit, we should not see the contents of skb like skb->len and skb->dev->name after ndo_start_xmit. But trace_net_dev_xmit does that and causes panic by NULL pointer dereference. This patch fixes trace_net_dev_xmit not to see the contents of skb directly. If you want to reproduce this panic, 1. Get tracepoint of net_dev_xmit on 2. Create 2 guests on KVM 2. Make 2 guests use virtio_net 4. Execute netperf from one to another for a long time as a network burden 5. host will panic(It takes about 30 minutes) Signed-off-by: Koki Sanagi <sanagi.koki@jp.fujitsu.com> Signed-off-by: David S. Miller <davem@davemloft.net>
Showing 2 changed files with 12 additions and 7 deletions Side-by-side Diff
include/trace/events/net.h
| ... | ... | @@ -12,22 +12,24 @@ |
| 12 | 12 | TRACE_EVENT(net_dev_xmit, |
| 13 | 13 | |
| 14 | 14 | TP_PROTO(struct sk_buff *skb, |
| 15 | - int rc), | |
| 15 | + int rc, | |
| 16 | + struct net_device *dev, | |
| 17 | + unsigned int skb_len), | |
| 16 | 18 | |
| 17 | - TP_ARGS(skb, rc), | |
| 19 | + TP_ARGS(skb, rc, dev, skb_len), | |
| 18 | 20 | |
| 19 | 21 | TP_STRUCT__entry( |
| 20 | 22 | __field( void *, skbaddr ) |
| 21 | 23 | __field( unsigned int, len ) |
| 22 | 24 | __field( int, rc ) |
| 23 | - __string( name, skb->dev->name ) | |
| 25 | + __string( name, dev->name ) | |
| 24 | 26 | ), |
| 25 | 27 | |
| 26 | 28 | TP_fast_assign( |
| 27 | 29 | __entry->skbaddr = skb; |
| 28 | - __entry->len = skb->len; | |
| 30 | + __entry->len = skb_len; | |
| 29 | 31 | __entry->rc = rc; |
| 30 | - __assign_str(name, skb->dev->name); | |
| 32 | + __assign_str(name, dev->name); | |
| 31 | 33 | ), |
| 32 | 34 | |
| 33 | 35 | TP_printk("dev=%s skbaddr=%p len=%u rc=%d", |
net/core/dev.c
| ... | ... | @@ -2096,6 +2096,7 @@ |
| 2096 | 2096 | { |
| 2097 | 2097 | const struct net_device_ops *ops = dev->netdev_ops; |
| 2098 | 2098 | int rc = NETDEV_TX_OK; |
| 2099 | + unsigned int skb_len; | |
| 2099 | 2100 | |
| 2100 | 2101 | if (likely(!skb->next)) { |
| 2101 | 2102 | u32 features; |
| 2102 | 2103 | |
| ... | ... | @@ -2146,8 +2147,9 @@ |
| 2146 | 2147 | } |
| 2147 | 2148 | } |
| 2148 | 2149 | |
| 2150 | + skb_len = skb->len; | |
| 2149 | 2151 | rc = ops->ndo_start_xmit(skb, dev); |
| 2150 | - trace_net_dev_xmit(skb, rc); | |
| 2152 | + trace_net_dev_xmit(skb, rc, dev, skb_len); | |
| 2151 | 2153 | if (rc == NETDEV_TX_OK) |
| 2152 | 2154 | txq_trans_update(txq); |
| 2153 | 2155 | return rc; |
| 2154 | 2156 | |
| ... | ... | @@ -2167,8 +2169,9 @@ |
| 2167 | 2169 | if (dev->priv_flags & IFF_XMIT_DST_RELEASE) |
| 2168 | 2170 | skb_dst_drop(nskb); |
| 2169 | 2171 | |
| 2172 | + skb_len = nskb->len; | |
| 2170 | 2173 | rc = ops->ndo_start_xmit(nskb, dev); |
| 2171 | - trace_net_dev_xmit(nskb, rc); | |
| 2174 | + trace_net_dev_xmit(nskb, rc, dev, skb_len); | |
| 2172 | 2175 | if (unlikely(rc != NETDEV_TX_OK)) { |
| 2173 | 2176 | if (rc & ~NETDEV_TX_MASK) |
| 2174 | 2177 | goto out_kfree_gso_skb; |