linux-kernel - [PATCH 3.16 222/233] net: handle NAPI_GRO_FREE_STOLEN_HEAD case also in napi_frags

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Sat, 09 Sep 2017 22:47:15 +0100
From:   Ben Hutchings <ben@...adent.org.uk>
To:     linux-kernel@...r.kernel.org, stable@...r.kernel.org
CC:     akpm@...ux-foundation.org, "David S. Miller" <davem@...emloft.net>,
        "Michal Kubeček" <mkubecek@...e.cz>
Subject: [PATCH 3.16 222/233] net: handle NAPI_GRO_FREE_STOLEN_HEAD case
 also in napi_frags_finish()

3.16.48-rc1 review patch.  If anyone has any objections, please let me know.

------------------

From: Michal Kubeček <mkubecek@...e.cz>

commit e44699d2c28067f69698ccb68dd3ddeacfebc434 upstream.

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 c21b48cc1bbf ("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: d7e8883cfcf4 ("net: make GRO aware of skb->head_frag")
Signed-off-by: Michal Kubecek <mkubecek@...e.cz>
Signed-off-by: David S. Miller <davem@...emloft.net>
[bwh: Backported to 3.16: The necessary cleanup is just kmem_cache_free(),
 so don't bother adding a function for this.]
Signed-off-by: Ben Hutchings <ben@...adent.org.uk>
---
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -4166,10 +4166,16 @@ static gro_result_t napi_frags_finish(st
 		break;

 	case GRO_DROP:
-	case GRO_MERGED_FREE:
 		napi_reuse_skb(napi, skb);
 		break;

+	case GRO_MERGED_FREE:
+		if (NAPI_GRO_CB(skb)->free == NAPI_GRO_FREE_STOLEN_HEAD)
+			kmem_cache_free(skbuff_head_cache, skb);
+		else
+			napi_reuse_skb(napi, skb);
+		break;
+
 	case GRO_MERGED:
 		break;
 	}