Commit e44699d2 authored by Michal Kubeček's avatar Michal Kubeček Committed by David S. Miller

net: handle NAPI_GRO_FREE_STOLEN_HEAD case also in napi_frags_finish()

Recently I started seeing warnings about pages with refcount -1. The
problem was traced to packets being reused after their head was merged into
a GRO packet by skb_gro_receive(). While bisecting the issue pointed to
commit c21b48cc ("net: adjust skb->truesize in ___pskb_trim()") and
I have never seen it on a kernel with it reverted, I believe the real
problem appeared earlier when the option to merge head frag in GRO was
implemented.

Handling NAPI_GRO_FREE_STOLEN_HEAD state was only added to GRO_MERGED_FREE
branch of napi_skb_finish() so that if the driver uses napi_gro_frags()
and head is merged (which in my case happens after the skb_condense()
call added by the commit mentioned above), the skb is reused including the
head that has been merged. As a result, we release the page reference
twice and eventually end up with negative page refcount.

To fix the problem, handle NAPI_GRO_FREE_STOLEN_HEAD in napi_frags_finish()
the same way it's done in napi_skb_finish().

Fixes: d7e8883c ("net: make GRO aware of skb->head_frag")
Signed-off-by: default avatarMichal Kubecek <mkubecek@suse.cz>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parent 6bdf6abc
...@@ -4767,6 +4767,13 @@ struct packet_offload *gro_find_complete_by_type(__be16 type) ...@@ -4767,6 +4767,13 @@ struct packet_offload *gro_find_complete_by_type(__be16 type)
} }
EXPORT_SYMBOL(gro_find_complete_by_type); EXPORT_SYMBOL(gro_find_complete_by_type);
static void napi_skb_free_stolen_head(struct sk_buff *skb)
{
skb_dst_drop(skb);
secpath_reset(skb);
kmem_cache_free(skbuff_head_cache, skb);
}
static gro_result_t napi_skb_finish(gro_result_t ret, struct sk_buff *skb) static gro_result_t napi_skb_finish(gro_result_t ret, struct sk_buff *skb)
{ {
switch (ret) { switch (ret) {
...@@ -4780,13 +4787,10 @@ static gro_result_t napi_skb_finish(gro_result_t ret, struct sk_buff *skb) ...@@ -4780,13 +4787,10 @@ static gro_result_t napi_skb_finish(gro_result_t ret, struct sk_buff *skb)
break; break;
case GRO_MERGED_FREE: case GRO_MERGED_FREE:
if (NAPI_GRO_CB(skb)->free == NAPI_GRO_FREE_STOLEN_HEAD) { if (NAPI_GRO_CB(skb)->free == NAPI_GRO_FREE_STOLEN_HEAD)
skb_dst_drop(skb); napi_skb_free_stolen_head(skb);
secpath_reset(skb); else
kmem_cache_free(skbuff_head_cache, skb);
} else {
__kfree_skb(skb); __kfree_skb(skb);
}
break; break;
case GRO_HELD: case GRO_HELD:
...@@ -4858,10 +4862,16 @@ static gro_result_t napi_frags_finish(struct napi_struct *napi, ...@@ -4858,10 +4862,16 @@ static gro_result_t napi_frags_finish(struct napi_struct *napi,
break; break;
case GRO_DROP: case GRO_DROP:
case GRO_MERGED_FREE:
napi_reuse_skb(napi, skb); napi_reuse_skb(napi, skb);
break; break;
case GRO_MERGED_FREE:
if (NAPI_GRO_CB(skb)->free == NAPI_GRO_FREE_STOLEN_HEAD)
napi_skb_free_stolen_head(skb);
else
napi_reuse_skb(napi, skb);
break;
case GRO_MERGED: case GRO_MERGED:
case GRO_CONSUMED: case GRO_CONSUMED:
break; break;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment