netdev - [net-next PATCH V1 3/3] net: warn on napi_alloc

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:	Mon, 09 May 2016 15:44:39 +0200
From:	Jesper Dangaard Brouer <brouer@...hat.com>
To:	netdev@...r.kernel.org, "David S. Miller" <davem@...emloft.net>
Cc:	saeedm@...lanox.com, gerlitz.or@...il.com, eugenia@...lanox.com,
	Alexander Duyck <alexander.duyck@...il.com>,
	Jesper Dangaard Brouer <brouer@...hat.com>
Subject: [net-next PATCH V1 3/3] net: warn on napi_alloc_skb being called in
 wrong context

It have always been required to call napi_alloc_skb from NAPI/softirq
context, which implies running with local_bh_disable'ed.  Thus, this
code path should already be well tested. But recent SKB bulk changes
introduced will make this more volatile and bugs more subtle, if this
is violated.

To catch any driver violating this add a loud WARN_ON.

Performance wise, I do worry about adding this runtime check code into
the hotpath, of this highly optimized function call.  I've
micro-benchmarked it with both IP-forwarding and local UDP delivery,
and didn't see any regressions.  It does adds extra code size (icache).

add/remove: 0/0 grow/shrink: 1/0 up/down: 43/0 (43)
function                                     old     new   delta
__napi_alloc_skb                             461     504     +43

Signed-off-by: Jesper Dangaard Brouer <brouer@...hat.com>
---
 net/core/skbuff.c |    3 +++
 1 file changed, 3 insertions(+)

diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index e85f1065b263..99addbf66f2e 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -498,6 +498,9 @@ struct sk_buff *__napi_alloc_skb(struct napi_struct *napi, unsigned int len,
 	struct sk_buff *skb;
 	void *data;

+	/* Catch drivers violating, not having local BH disabled */
+	WARN_ON(!in_softirq());
+
 	len += NET_SKB_PAD + NET_IP_ALIGN;

 	if (unlikely((len > SKB_WITH_OVERHEAD(PAGE_SIZE)) ||