Eric Dumazet <eric.duma...@gmail.com> writes:

> On Thu, 2015-11-19 at 21:20 +0100, Bjørn Mork wrote:
>> Eric Dumazet <eric.duma...@gmail.com> writes:
>> 
>> > How many times should we crash before napi_frags_skb() returns NULL ?
>> ..
>> >                         return NULL;
>> 
>> Huh?  Now I'm lost here, too.
>
>
> Well, Ethernet drivers should not feed GRO with frames with less than 14 
> bytes.
>
> ( eth_type_trans() would crash the same )
>
> Lets fix buggy drivers instead of adding defensive code all over the stack.
>
> napi_gro_frags() is used by exactly 10 drivers, and I am pretty sure
> they are OK.
>

Would the following be an appropriate change in addition to the one
you've posted, then? If so I can repost as a formal patch, if you'd
like. At present, there's only one user of napi_frags_skb(), and your
patch removes the NULL check. If this can really only be the result of
buggy driver, then perhaps we should just call out the bug?

diff --git a/net/core/dev.c b/net/core/dev.c
index 41cef3e..b71c8b4 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -4440,10 +4440,7 @@ static struct sk_buff *napi_frags_skb(struct napi_struct 
*napi)
        eth = skb_gro_header_fast(skb, 0);
        if (unlikely(skb_gro_header_hard(skb, hlen))) {
                eth = skb_gro_header_slow(skb, hlen, 0);
-               if (unlikely(!eth)) {
-                       napi_reuse_skb(napi, skb);
-                       return NULL;
-               }
+               BUG_ON(!eth);
        } else {
                gro_pull_from_frag0(skb, hlen);
                NAPI_GRO_CB(skb)->frag0 += hlen;
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to